-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Invalid response object from API #1213
Comments
@apflieger Thanks for reaching out! Would you be able to dump a full/complete stacktrace without the frames omitted? I tried to repro on my end but I can de-serialize this exact raw JSON with the latest version of stripe-java so I'm not yet sure what could be causing this. |
Apparently, "frames omitted" means that these frames were already printed in the stack trace, there's no more to print. It might be a thread safety issue because we get these errors since we started processing payment in parallel |
@apflieger Sorry for the delay, we haven't been able to reproduce this yet unfortunately. We do think it's a thread safety issue with GSON that is likely related to this issue: google/gson#764 (comment) We're going to keep this one in mind and see if we can make improvements to the library internally to protect from this further in the future. |
It hasn't crossed my mind that this could happen on successful payment too so I did a bit more investigations and yes, it actually does happen on successful payments which is a bit problematic since our payment system do retries... We end up billing multiple times our customers. The issue on Gson is still opened google/gson#764 (comment) A simple way to fix it would be to not share Gson instance between threads. I ran some performance tests on that (I guess performance is the only concern) @Test
public void gsonCreationPerformance() {
var durations = new ArrayList<>();
for (int i = 0; i < 1000; i++) {
var gson = ApiResource.createGson();
Instant at1 = Instant.now();
gson.fromJson(JSON, PaymentIntent.class);
durations.add(Duration.between(at1, Instant.now()).toMillis());
}
System.out.println(durations.stream().mapToLong(d -> (long) d).average());
durations.clear();
var gson = ApiResource.createGson();
for (int i = 0; i < 1000; i++) {
Instant at2 = Instant.now();
gson.fromJson(JSON, PaymentIntent.class);
durations.add(Duration.between(at2, Instant.now()).toMillis());
}
System.out.println(durations.stream().mapToLong(d -> (long) d).average());
} Where JSON is a hardcoded response. I made ApiResource.createGson public On my machine, it gives
4-5 milliseconds process time would be added to a request. This looks negligeable to me. |
Thanks for the very detailed report @apflieger - it's really appreciated! We'll be looking at that this quarter to see how we can fix this internally. |
The bug seems to be fixed in Gson 2.9.0. I upgraded 2 months ago and we got no error since. |
Great - thank you for the report back @apflieger ! Gson 2.9.0 is the version we include in our Gradle file as of #1322. I'll close this issue for now but please re-open if you experience this issue again. |
Hi, We still get the error sometimes unfortunately. We reduced the amount of charges that we do so we see it less often but still. This one is a successful payment that has been marked as failed in our system. As I said previously, this is kinda problematic because we retry these payments so we end up charging customers multiple times.
|
Apparently, this might be fixed in the next release of Gson (2.11 probably) by this PR google/gson#1832 |
The fix has been released in gson 2.10.1 🎉
We've just deployed the upgrade. Let's monitor 👀 |
Thank you @apflieger! |
In https://github.com/stripe/stripe-java/releases/tag/v22.5.1 we bumped stripe-java's dependency to the newest version of GSON. |
Version of stripe-java: 20.50.0
Version of the API: 2020-08-27
java -version
I experience these errors since 30th of April.
I can't figure out what is causing the error since gson doesn't say much about the error. It seems that the json schema changed somewhere and the library became incompatible.
Note that this concerns only a few payments.
(I anonymized personnal data in the json)
The text was updated successfully, but these errors were encountered: