fix: retry interceptor now closes previous response #332
Conversation
|
|
||
| final AtomicBoolean successful = new AtomicBoolean(false); | ||
| final CountDownLatch requestLatch = new CountDownLatch(1); | ||
| okHttpClient.newCall(request).enqueue(new Callback() { |
There was a problem hiding this comment.
I think we have the Await utility in this testcodebase? It would simplify cleanup this CountDownLatch/AtomicBoolean stuff.
(Same for the next test, too.)
There was a problem hiding this comment.
I started looking at that and unfortunately the Await utility is in the integration tests package. I was tinkering with moving it under the this package, but turned out to be more of a cluster than what it was worth. I'll leave it as is for now if that's OK
There was a problem hiding this comment.
You can always just copy-paste it. It is unlikely to change so the code duplication is probably not worth optimizing for.
There was a problem hiding this comment.
Clipboard inheritance never fails me 🤣 The chain of callbacks made it a little 🤯 , but I think it looks a little cleaner.
2 line fix. 142 lines of unit tests. The CS gods are happy 😄
No...the test package has sourceCompatibility set to 1.8 but the rest of the codebase doesn't |
Ah bummer! |
Issue #, if available: #305
Description of changes:
The
RetryInterceptorwas not properly closing the response from a previous attempt before before re-trying the request by invokingchain.proceed.This PR attempts to address the issue by closing the response for scenarios when the request will be retried. Added a couple of unit tests I used to reproduce the bug.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.