HTTP/2 GOAWAY handling limitations and proposed exception

[laeubi]

Key Problem

Different GOAWAY error codes require different handling strategies:

// Current: Cannot distinguish between these scenarios
try {
    client.send(request, BodyHandlers.ofString());
} catch (IOException e) {
    // Was it NO_ERROR (safe retry) or ENHANCE_YOUR_CALM (backoff needed)?
    // Was it PROTOCOL_ERROR (client bug) or REFUSED_STREAM (retry OK)?
}

// Proposed: Explicit error code handling
catch (HttpGoAwayException e) {
    switch (e.getErrorCode()) {
        case NO_ERROR: retryImmediately(); break;
        case ENHANCE_YOUR_CALM: exponentialBackoff(); break;
        case PROTOCOL_ERROR: reportBug(); break;
    }
}

Establishes foundation for future HttpGoAwayException implementation, following precedent of HttpTimeoutException and HttpConnectTimeoutException.

See GOAWAY.md for a more complete elaboration and JavaHttpClientGoawayTest for a testcase

java-http-client/src/test/java/io/github/laeubi/httpclient/JavaHttpClientGoawayTest.java

Lines 35 to 70 in db99114

	@Test
	@DisplayName("HTTP/2 GOAWAY can be handled")
	public void testHttp2GoawayIsHandled() throws Exception {
	logger.info("\n=== Testing HTTP/2 GOAWAY ===");
	HttpClient client = httpsClient();
	HttpRequest request = HttpRequest.newBuilder().uri(getHttpsUrl()).GET().build();
	logger.info("Sending HTTP request to " + getHttpsUrl());
	CompletableFuture<HttpResponse<String>> responseAsync = client.sendAsync(request,
	HttpResponse.BodyHandlers.ofString());
	try {
	responseAsync.join();
	} catch (Exception e) {
	// whatever happens, we just want to make sure the request is complete
	logger.info("Caught exception during join: " + e);
	}
	httpsServer.assertGoaway();
	try {
	HttpResponse<String> response = responseAsync.get();
	logger.info("Response status: {}", response.statusCode());
	logger.info("Response version: {}", response.version());
	logger.info("Response body: {}", response.body());
	logger.info("Response headers: {}", response.headers().map());
	assertEquals(200, response.statusCode(), "Expected 200 OK response");
	assertNotNull(response.body(), "Response body should not be null");
	logger.info("=== HTTP/2 Upgrade test completed successfully ===\n");
	} catch (ExecutionException e) {
	Throwable cause = e.getCause();
	cause.printStackTrace();
	// Here we have no way to know any of the details of the GOAWAY see GOAWAY.md
	// for further rationale.
	assertNotEquals(IOException.class, cause.getClass());
	}
	}

[djelinski]

Hi @laeubi could you present your case on the net-dev mailing list?

Adding a specialized exception wouldn't require much new code, but I'm concerned that it might not be of much help to what you'd like to achieve. In particular:

• errors that can be retried (GOAWAY/NO_ERROR, RESET_STREAM/REFUSED_STREAM) are retried internally by the HttpClient; no error is reported to the application
• I don't think ENHANCE_YOUR_CALM is supposed to be used to tell the client to back off. Are there any servers that use it that way? It looks like a warning signal for misbehaving peers to me.
• We don't currently implement HTTP_1_1_REQUIRED, but if we learn about servers that use it, we will likely implement the retry mechanism in the HttpClient as well.
• If no requests are in progress when a GOAWAY is received, we currently have no way to report it to the user.

I assume the proposed exception would also cover the errors sent in RESET_STREAM frames, and we'd probably need similar exceptions for QUIC and HTTP/3 errors.

I expect to improve the GOAWAY handling situation in JDK-8371903, but as of now I'm planning to use a regular ProtocolException (so, no error code).

[laeubi]

Yes I think JDK-8371903 looks related, the error codes are extracted from the specification and due to currently one only gets "GOAWAY received" its really hard to tell what server really respond in certain situations.

So in general I'm open to different ways it should just be distinguishable e.g. GOAWAY is something that happens while the wire is still active and might be handled in some way while e.g. a broken pipe is fatal on its own and one likely can not do much there.

I also agree that handling as much as possible already internally is great benefit... I have not yet seen any HTTP/3 servers and as previously that's not supported I can not really tell much about that part of the topic :-)

[djelinski]

The "GOAWAY received" error was removed in JDK-8335181. It fixed the graceful shutdown case, but broke the (hopefully infrequent) error case.

Thanks for confirming that you're fine with handling some of the errors internally. I plan to improve the error message, then we can see if the error code is still required.

[laeubi]

The "GOAWAY received" error was removed in JDK-8335181. It fixed the graceful shutdown case, but broke the (hopefully infrequent) error case.

This is a different topic, the bug you mention is indeed fixed but was for the case that a connection was closed during in the "cache". But servers can still send GOAWAY for other reasons and then the current request will receive a generic GOAWAY exception as demonstrated by JavaHttpClientGoawayTest where we let the server respond with GOAWAY during a request happens.