[kotlin-client] OkHttp call is now non-blocking

[sheepdreamofandroids]

All functions in the kotlin-client are marked suspend but the final call to OkHttp was blocking the thread. This is no longer the case.

Note that this can have impact on integration tests that verify backend calls because now it can happen that one call fails and therefore cancels the whole hierarchy of coroutines which possible cancels other backend calls before they even started. That IS desired behaviour though.

This code is a subset of a few patches that are running in production.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (5.3.0), 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@jimschubert, @dr4ke616, @karismann, @Zomzog, @andrewemery, @4brunu, @yutaka0m

[4brunu]

Hey, thanks for this contribution, this is indeed a good idea.
I left some comments with improvements.
Also all those changes should only be applied when the user uses kotlin coroutines, so they should be inside the following if.

{{#useCoroutines}}
changes here for the projects that use kotlin coroutines
{{/useCoroutines}}
{{^useCoroutines}}
changes here for the projects that don't use kotlin coroutines
{{/useCoroutines}}
{{/doNotUseRxAndCoroutines}}

[4brunu]

@sheepdreamofandroids I think this should only be called if the call is not canceled.

https://github.com/coil-kt/coil/blob/a17284794764ed5d0680330bfd8bca722a36bb5e/coil-base/src/main/java/coil/util/ContinuationCallback.kt

    override fun onFailure(call: Call, e: IOException) {
        if (!call.isCanceled) {
            continuation.resumeWithException(e)
        }
    }

[sheepdreamofandroids]

If the call is cancelled because the coroutine is cancelled as per your next comment, the resumeWithException shouldn't do anything.
On the other hand, if the call is ever cancelled for another reason, you do want to report the exception and resume, or else the coroutine will never terminate.
See https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/suspend-cancellable-coroutine.html

[4brunu]

Thanks for the explanation, looks good to me 👍

[4brunu]

@sheepdreamofandroids I think here we should use a suspendCancellableCoroutine instead of suspendCoroutine.

https://github.com/coil-kt/coil/blob/0af5fe016971ba54518a24c709feea3a1fc075eb/coil-base/src/main/java/coil/util/Extensions.kt#L45-L51

[sheepdreamofandroids]

I did this except for the check on call.isCancelled.

I added checks around the coroutine imports. The other imports may not be necessary all the time but they come from the same very common dependencies as other unconditional imports. I prefer to keep the template simple.

[4brunu]

Looks good to me.
By the way, I would like to have your option regarding this change from suspendCoroutine to suspendCancellableCoroutine?
Do you agree with it?

[sheepdreamofandroids]

The additional value is that an ongoing http call will be cancelled in case the result is not needed anymore. In itself that is valuable.

It seems to me that the time window where you can cancel an ongoing http call is very short, unless there is a huge request body so it will rarely make a difference.

The additional complexity is quite low though so my gut feeling says it's worth it.

[4brunu]

Thanks 👍

[wing328]

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

[4brunu]

For me this PR is ready to merge 👍