Enable error handling in Java WebClient library, fixes #1243

[lukstei]

PR checklist

• Read the contribution guidelines.
• 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\.
• Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
• Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Enable error handling in Java WebClient library, fixes #1243.
Also now the generated Exception class ApiException is used.

Custom error handling behaviour can be defined in an overwriting class e.g.:

public class MyApiClient extends ApiClient {
  @Override
  protected <T> Mono<T> handleUnsuccessfulResponse(ClientResponse response) {
    return super.<T>handleUnsuccessfulResponse(response)
      .onErrorMap(errorcode(404), x -> new MyNotFoundException("Not found"));
  }

  private static Predicate<Throwable> errorcode(int errorCode) {
    return x -> x instanceof ApiException && ((ApiException) x).getCode() == errorCode;
  }
}

@bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger

[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

[wing328]

cc @daonomic who's added the WebClient support.

[lukstei]

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

Thanks for the info - fixed it

[cbornet]

Spring has a built-in ResponseStatusException WebClientResponseException which should be used instead of ApiException

[cbornet]

Actually I think this all should be simplified to:

return requestBuilder.retrieve().bodyToMono(returnType);

which should throw a WebClientResponseException if the status code is 4xx or 5xx

[lukstei]

Actually I think this all should be simplified to:

return requestBuilder.retrieve().bodyToMono(returnType);

which should throw a WebClientResponseException if the status code is 4xx or 5xx

@cbornet yes you are right it is handled when calling retrieve() instead of exchange() (see DefaultWebClient:374). So this custom error handling can be removed altogether. I will do this.

Two further questions arised:

1. Should we wrap the WebClientResponseException in an ApiException?
2. ApiClient:495: This return Mono.empty() is not optimal because it does not consume the body (see note in ClientResponse).
   This is fixed when using your suggested solution because it then calls the drainBody() method (see DefaultWebClient.java:448) which consumes the body.
   The question for me is, if we can safely remove this custom handling logic altogether. I think for the second case yes (returnType == null), because this is already handled in the BodyExtractor, but I'm not sure about the response.statusCode() == HttpStatus.NO_CONTENT case, what do you think?

[cbornet]

IMO we should remove all the logic and not use ApiException as it doesn't add any value. Also I think WebClient users would expect WebClientResponseException if something went wrong.

[lukstei]

I removed all the logic, which makes it much more cleaner now and also solves the initial issue #1243.

The question whether we should use the ApiException is for me still not clear, because I think it would never add any value regardless of the underlying library, and I suppose every library has its own underlying Exception, but this should be discussed on a more general level and not in this issue I think.

@cbornet thank you for the input.

[cbornet]

LGTM 👍

[cbornet]

As for ApiException it could be used to replace the RuntimeException that are thrown at some places of the code.