afs commented on code in PR #3464:
URL: https://github.com/apache/jena/pull/3464#discussion_r2381929614
##########
jena-arq/src/main/java/org/apache/jena/http/HttpLib.java:
##########
@@ -640,25 +650,52 @@ public static HttpResponse<InputStream>
executeJDK(HttpClient httpClient, HttpRe
* @return HttpResponse
*/
public static <T> HttpResponse<T> executeJDK(HttpClient httpClient,
HttpRequest httpRequest, BodyHandler<T> bodyHandler) {
- try {
- // This is the one place all HTTP requests go through.
- logRequest(httpRequest);
- HttpResponse<T> httpResponse = httpClient.send(httpRequest,
bodyHandler);
- logResponse(httpResponse);
- return httpResponse;
- //} catch (HttpTimeoutException ex) {
- } catch (IOException | InterruptedException ex) {
- if ( ex.getMessage() != null ) {
- // This is silly.
- // Rather than an HTTP exception, bad authentication becomes
IOException("too many authentication attempts");
- // or IOException("No credentials provided") if the
authenticator decides to return null.
- if ( ex.getMessage().contains("too many authentication
attempts") ||
- ex.getMessage().contains("No credentials provided") ) {
- throw new HttpException(401, HttpSC.getMessage(401));
+ return AsyncHttpRDF.getOrElseThrow(executeJDKAsync(httpClient,
httpRequest, bodyHandler));
+ }
+
+ /**
+ * Execute request and return a {@code HttpResponse<InputStream>} response.
+ * Status codes have not been handled. The response can be passed to
+ * {@link #handleResponseInputStream(HttpResponse)} which will convert
non-2xx
+ * status code to {@link HttpException HttpExceptions}.
+ *
+ * @param httpClient
+ * @param httpRequest
+ * @return HttpResponse
+ */
+ public static CompletableFuture<HttpResponse<InputStream>>
executeJDKAsync(HttpClient httpClient, HttpRequest httpRequest) {
+ return executeAsync(httpClient, httpRequest,
BodyHandlers.ofInputStream());
+ }
+
+ public static <T> CompletableFuture<HttpResponse<T>>
executeJDKAsync(HttpClient httpClient, HttpRequest httpRequest, BodyHandler<T>
bodyHandler) {
+ // This is the one place all HTTP requests go through.
+ logRequest(httpRequest);
+ CompletableFuture<HttpResponse<T>> future =
httpClient.sendAsync(httpRequest, bodyHandler)
+ .thenApply(httpResponse -> {
+ logResponse(httpResponse);
+ return httpResponse;
+ })
+ .exceptionally(ex -> {
+ Throwable cause = ex instanceof CompletionException ?
ex.getCause() : ex;
+ if (cause instanceof IOException) {
+ if ( ex.getMessage() != null ) {
+ // This is silly.
+ // Rather than an HTTP exception, bad authentication
becomes IOException("too many authentication attempts");
+ // or IOException("No credentials provided") if the
authenticator decides to return null.
+ if ( ex.getMessage().contains("too many authentication
attempts") ||
+ ex.getMessage().contains("No credentials provided")
) {
+ throw new HttpException(401,
HttpSC.getMessage(401));
+ }
+ }
+ // Note: Can't reuse AsyncHttpRDF.handleRuntimeException
because of this HttpException.
+ throw new HttpException(httpRequest.method()+"
"+httpRequest.uri().toString(), cause);
+ } else if (cause instanceof RuntimeException re) {
Review Comment:
This looks like it is a change to exception handling in the sync case. (I
hope I read the code correctly.) Does this matter?
`AsyncHttpRDF.getOrElseThrow` passes up runtime exceptions untouched. That
means the app gets the exception from the async thread and a stacktrace does
not indicate where in the application code their synchronous call was made.
`HttpClientImpl.send` builds an exception with the other threads exception
as the cause. The app gets an exception with the app stack. When printed, the
stacktrace identifies the app code that makes the sync call.
The `HttpClientImpl.send` exception rebuild is a bit yuk - it has to
enumerate the possible classes.
We could have a `HttpException`, even `HttpAsyncException` <
`HttpException`, as a carrier to the app call stack making it robust against
any future exception introduced by the JDK library. The change would be here,
although in `AsyncHttpRDF.getOrElseThrow` is a possibility as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
