Aklakan commented on code in PR #3464:
URL: https://github.com/apache/jena/pull/3464#discussion_r2382448850
##########
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:
> 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.
Yes, that's the issue.
I think your suggestion to wrap an exception with the already existing
unchecked `HttpException` is the clean way forward. Consolidating this
exception handling in `AsyncHttpRDF.getOrElseThrow` seems reasonable.
A variant of `getOrElseThrow` with an additional `HttpRequest` argument
could be used to add the request URIs to the exceptions:
```java
throw new HttpException(httpRequest.method()+"
"+httpRequest.uri().toString(), cause);
```
--
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]
