StreamingResponseHandler is incompatible with Airlift's expecations #208
Description
StreamingResponseHandler behaves a bit strangely in that we don't really care about what it returns, as its only purpose is to transfer data from one stream (what we get back from the S3 endpoint) to another (the outbound stream back to aws-proxy's client) - bypassing the entire flow where Airlift's HTTP client actually deals with responses.
It is currently implemented as a ResponseHandler<Void, RuntimeException> - highlighting that point, we don't care about its return value.
However, Airlift internally asserts (much in keeping with the rest of the codebase) that nothing it ever deals with is null. This only seems to happen for cases where handleException is called:
This means that in cases where we need to handle any exception, the flow looks like the below:
2025-12-30T18:17:02.066Z DEBUG io.trino.aws.proxy.server.rest.TrinoS3ProxyClient Streaming request started
2025-12-30T18:17:02.073Z INFO io.trino.aws.proxy.server.rest.RequestLoggerController RequestEnd: {request.http.method=PUT, request.errors={exception.type=java.io.EOFException, exception.message=[...], request.number=0 [...]}, request.type=SigningServiceType[serviceName=s3, signingTraits=[S3V4_SIGNER, STREAM_CONTENT]], request.uri=http://127.0.0.1:63217/api/v1/s3Proxy/s3/test-remote-eof-bucket/foo, request.http.entity=true, request.id=71098595-3dd9-4b32-a0bc-c5d945f74e59, request.elapsed.ms=90, request.eventId=0000019b707a4d1d.0000000000000000.1}
2025-12-30T18:17:09.136Z DEBUG io.trino.aws.proxy.server.rest.StreamingResponseHandler Resuming streaming response handler: java.io.EOFException: [...]
2025-12-30T18:17:23.862Z DEBUG io.trino.aws.proxy.server.rest.StreamingResponseHandler Resuming streaming response handler: OutboundJaxrsResponse{status=500, reason=[... reference to the above exception ...], hasEntity=false, closed=false, buffered=false}
2025-12-30T18:17:31.626Z DEBUG io.trino.aws.proxy.server.rest.TrinoS3ProxyClient streaming failed with error message: exceptionResponse is null
2025-12-30T18:17:57.361Z DEBUG io.trino.aws.proxy.server.rest.StreamingResponseHandler Resuming streaming response handler: java.lang.RuntimeException: java.lang.NullPointerException: exceptionResponse is null
2025-12-30T18:18:03.097Z DEBUG io.trino.aws.proxy.server.rest.StreamingResponseHandler Resuming streaming response handler: OutboundJaxrsResponse{status=500, reason=java.lang.NullPointerException: exceptionResponse is null, hasEntity=false, closed=false, buffered=false}
The first handleException call dealt with the real exception that took place (an EOF triggered by the client, while I experimented for #207 ). Then, as handleException returned null (the only thing it can return with its current signature), Airlift threw and started a new cycle of exception handling.
Since the response had been delivered by the first exception, this doesn't materially impact much other than being noisy.
There are a few options, but I'd like to get some feedback if possible as this appears to be an intentional choice in Airlift: we could just make this return an empty optional instead, or an empty string - the return value of the response handler is irrelevant.
However, is there a better & more idiomatic way?
CC @Randgalt