Support connection reset on cancellation for generated REST clients

[calebkiage]

Description

In PR #41710, cancellation support was added to the reactive REST services. However, the generated REST clients were never updated to support the scenario. What this means is that if the REST clients are used to call reactive endpoints, then the server will never be notified of the cancellation and will keep running the request to completion.

I have a sample project showing this issue attached below.

In the project, calling:

curl -X GET --location "http://localhost:9000/call/cancel1"

will reset the connection and the service will stop processing, but calling:

curl -X GET --location "http://localhost:9000/call/cancel2"

will not.

Sample project

Implementation ideas

The code below adds cancellation for a Vert.x HttpClient:

withTimeoutOrNull(delayLong) {
    suspendCancellableCoroutine { cont->
        val options = HttpClientOptions().setDefaultHost("localhost").setDefaultPort(9000)
        val client = ctx.vertx().createHttpClient(options)
        client.request(io.vertx.core.http.HttpMethod.GET, "/long/a").onComplete { conn->
            if (conn.succeeded()) {
                cont.invokeOnCancellation {
                    // Trigger a connection reset on cancellation
                    conn.result().reset()
                }
                val request = conn.result()
                request.send().onComplete { resp->
                    if (resp.succeeded()) {
                        val body = resp.result().body()
                        body.onComplete { bodyState->
                            if (bodyState.succeeded()) {
                                cont.resume(bodyState.result().toString(Charsets.UTF_8))
                            } else {
                                cont.resumeWithException(bodyState.cause())
                            }
                        }
                    } else {
                        cont.resumeWithException(resp.cause())
                    }
                }
            } else {
                cont.resumeWithException(conn.cause())
            }
        }
    }
}

[quarkus-bot]

/cc @cescoffier (rest-client), @geoand (kotlin,rest-client)

[geoand]

Thanks, I'll have a look next week

[geoand]

@cescoffier @vietj what is the proper way to close / cancel an in flight request from the Vertx HTTP Client?

[cescoffier]

You cannot cancel a request, you must close the connection. Be very careful with that, as it breaks keep-alive, and if http/2 with streams is used, you close all of them.

Reset is also a possibility, but consequences are almost similar (I think you save the handshake, I. Red to double check)

[cescoffier]

Resetting seems to be at the stream level. Wondering what happens for http/1

[cescoffier]

Ok,

So for http/1 reset closes the connection (and thus may require another connection handshake later).

For http/2 it reset the stream (using the infamous RST_STREAM frame). The server will receive the info and close the stream eventually (which means that it does not guarantee that the server will stop the work immediately or even that you won't receive the answer). The connection is still established and the other streams are not impacted.

[geoand]

Thanks for the detailed response!

So it sounds to me like we should probably shy away from reacting to the cancelation of a Uni and not try to cancel the underlying HTTP client request - so do nothing for this :)

[calebkiage]

Is there a way to find out if a stream has no subscribers? In that situation, I think the right thing to do is close the connection in http/1 and reset the stream in http/2. This should match many other clients including OkHttp and the JDK clients when using:

// future is a CompletableFuture<T> from: client.sendAsync()...
future.cancel(true);

I believe .NET's HTTP client does the same thing as well. So does JavaScript's fetch.

[calebkiage]

I'd imagine this change would go in the ClientSendRequestHandler's handle method:

quarkus/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java

Lines 89 to 90 in 1f69d1d

	@Override
	public void handle(RestClientRequestContext requestContext) {

However, I'm unsure of how to listen to the Mutiny reactive stream subscriber disconnecting from the stream...other than that, the change would be trivial for me to contribute.

[calebkiage]

Maybe adding a cancel handler runnable to the request context would work? Then when Quarkus hands the user a Uni, it will have configured the cancellation.

Given the following client:

@Path("/long")
@RegisterRestClient
public interface HelloClient {
    @GET
    @Path("a")
    @Produces(MediaType.TEXT_PLAIN)
    Uni<String> longRunning();
}

Maybe it's possible to have the returned uni hooked up to run:

// uni is what would have originally been returned when calling longRunning()
uni.onCancellation().invoke(() -> {
    // call previously set cancel handler runnable to reset the connection
});

[geoand]

Yeah, that would be the way to go, but we need to determine whether actually trying to close the connection is the right thing to do.

[calebkiage]

Can it be made into an option like quarkus.rest-client.*.cancellable=true|false? It can default to false.

[geoand]

Likely yeah

[geoand]

@calebkiage can you give #41990 a quick shot?