gnodet-bot commented on code in PR #26515:
URL: https://github.com/apache/camel/pull/26515#discussion_r4052412007
##########
components/camel-platform-http-vertx/src/main/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpConsumer.java:
##########
@@ -373,10 +374,24 @@ private void handleExchangeComplete(Exchange exchange) {
}
private void handleFailure(Exchange exchange, RoutingContext ctx,
Throwable failure) {
- getExceptionHandler().handleException(
- "Failed handling platform-http endpoint " +
getEndpoint().getPath(),
- failure);
- ctx.fail(failure);
+ if (ctx.response().closed()) {
+ LOGGER.debug("Client closed the connection of platform-http
endpoint {} before the response completed",
+ getEndpoint().getPath(), failure);
+ } else {
+ getExceptionHandler().handleException(
+ "Failed handling platform-http endpoint " +
getEndpoint().getPath(),
+ failure);
+ if (!ctx.response().headWritten()) {
+ ctx.fail(failure);
+ } else if (!ctx.response().ended() && !ctx.response().closed()) {
+ // The response has already started, so there is no error
status left to send, and failing the
+ // routing context would only log the failure a second time as
an unhandled router exception.
+ // Reset it directly instead: the client still has to be told
that the response it is reading is
+ // truncated, rather than being left waiting for a body that
will never be completed.
+ ctx.response().reset(HTTP2_INTERNAL_ERROR);
+ }
+ }
Review Comment:
⚠️ **Race: `ctx.response().closed()` may lag behind a client abort.**
Vert.x updates `closed` asynchronously on the network thread. When `pipe()`
fails with a `WriteException` (client disconnected mid-stream), the
`onComplete` callback fires on the event loop before the closed flag is set.
This method will then fall into the `else` branch, call
`getExceptionHandler().handleException(...)` (WARN-level stack trace), and call
`ctx.response().reset(2)` on an already-dead connection — for every client that
disconnects while downloading.
Consider distinguishing write failures caused by connection loss from
genuine handler errors:
```suggestion
private void handleFailure(Exchange exchange, RoutingContext ctx,
Throwable failure) {
if (ctx.response().closed() || isConnectionLost(failure)) {
LOGGER.debug("Client closed the connection of platform-http
endpoint {} before the response completed",
getEndpoint().getPath(), failure);
} else {
getExceptionHandler().handleException(
"Failed handling platform-http endpoint " +
getEndpoint().getPath(),
failure);
if (!ctx.response().headWritten()) {
ctx.fail(failure);
} else if (!ctx.response().ended() && !ctx.response().closed()) {
ctx.response().reset(HTTP2_INTERNAL_ERROR);
}
}
```
where `isConnectionLost` checks for `io.vertx.core.impl.WriteException`,
`java.io.IOException` with a connection-reset/broken-pipe message, or similar.
The exact types depend on the Vert.x version in use.
--
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]
