JCL-402: Low-level client throws enriched exceptions - #1157
NSeydoux merged 25 commits into
Conversation
Sorry, something went wrong.
| headers.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); | ||
|
|
||
| final Request request = builder.build(); | ||
| return client.send(request, Response.BodyHandlers.ofByteArray()) |
There was a problem hiding this comment.
This looks like a large change, but essentially the error branch of the initial if has been moved to the wrapping throwOnError
Sorry, something went wrong.
| )// FIXME I don't understand why the following is required. | ||
| .thenAccept(o -> { /* no-op */ }); |
There was a problem hiding this comment.
I don't know that this is the correct approach
Sorry, something went wrong.
There was a problem hiding this comment.
It's because the method signature is (async) void.
Previously (when error handling was here) the response from the body handler was mapped (thenApply) to nothing.
Sorry, something went wrong.
|
You'll notice that unlike what we discussed yesterday, I didn't end up using the exceptionally clause of the CompletionStage chain, and instead changed the throwOnError BodyHandler so that it accepts a custom exception mapper. This prevents from having to add a mapping from generic ClientHttpException to module-specific SolidClientException, which didn't provide much value. |
Sorry, something went wrong.
| * @param statusCode the HTTP response status code | ||
| * @return true if the status code is in the success range, namely [200, 299]. | ||
| */ | ||
| static boolean isSuccess(final int statusCode) { |
There was a problem hiding this comment.
Do we really need this method?
Sorry, something went wrong.
There was a problem hiding this comment.
It avoids some duplication, and my impression was that it is a quite common pattern to have this affordance on HTTP response object: JS Response has a .ok getter, and OkHttp has isSuccessful. I'm not strongly opposed to removing this, but I find it reads slightly better than checking the status range each time.
Sorry, something went wrong.
`SolidClient` now parses HTTP error responses if they are compliant with RFC9457 in order to throw a structured exception. In case when no JSON parser is available in the classpath, the default behavior is to build a ProblemDetails object that only relies on the data available in the HTTP response status code.
It will be used in Jena and RDF4J modules too
HTTP body handlers are applied by the low-level HTTP client, so they should only ever map to an HTTP response. The processing of that response is then the responsibility of the caller.
This is based on #1160, which should be reviewed first.
The SolidClient now uses the throwOnError body mapper to handle error responses, and provides a custom exception mapper to throw the appropriate specialized exception.