swagger-codegen icon indicating copy to clipboard operation
swagger-codegen copied to clipboard

[Java][okhttp-gson] Fix incorrect use of OkHttp interceptors

Open ngaya-ll opened this issue 7 years ago • 3 comments

PR checklist

  • [x] Read the contribution guidelines.
  • [x] Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • [x] Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • [x] Copied the technical committee to review the pull request if your PR is targeting a particular programming language. @bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger

Description of the PR

Currently, the generated Java okhttp-gson client adds an interceptor to the underlying OkHttpClient for each async call. The purpose of the interceptor is to wrap the response body to track download progress. This implementation doesn't work correctly, for multiple reasons:

  • The interceptor intercepts all requests to the client, not just the one it's trying to track.
  • Interceptors are never removed from the client, so each async request adds another layer of interception for all subsequent requests. Since the interceptor chain is invoked recursively, at some point all requests will start failing with a StackOverflowError.

With this code change, the client uses a single interceptor to decorate all async requests and send updates to the relevant listener only.

I also added a test case to demonstrate the issue, which fails on the current master and passes on this branch.

Fixes https://github.com/swagger-api/swagger-codegen/issues/7876, https://github.com/swagger-api/swagger-codegen/issues/8915

ngaya-ll avatar Apr 21 '18 09:04 ngaya-ll

@bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger Any update on this? This fixes a critical bug for users making async calls using okhttp-gson generated clients.

ngaya-ll avatar May 18 '18 21:05 ngaya-ll

It would be lovely to have this merged to fix #7876 as the current state renders it impossible to use any of the generated "async" methods in any sort of long-lived production server.

mattnworb avatar Jan 11 '19 16:01 mattnworb

It would be lovely to have this merged to fix #7876 as the current state renders it impossible to use any of the generated "async" methods in any sort of long-lived production server.

+1 Any update on this ?

mtnourji avatar Feb 18 '21 16:02 mtnourji