fix(transport): add HTTP/2 to HTTP/1.1 automatic fallback for connect…#1340
Open
Mrjyw wants to merge 1 commit into
Open
fix(transport): add HTTP/2 to HTTP/1.1 automatic fallback for connect…#1340Mrjyw wants to merge 1 commit into
Mrjyw wants to merge 1 commit into
Conversation
…ion errors When HTTP/2 connections are dropped by cloud infrastructure (e.g. Alibaba Cloud SLB GOAWAY, RST_STREAM due to rate limiting),the transport now automatically retries the request with HTTP/1.1 instead of failing immediately. Previously, such connection-level errors propagated directly to the caller with no protocol fallback. Fixes agentscope-ai#1287 1.0.12-SNAPSHOT **Background:** Alibaba Cloud SLB/ALB enforces a default limit of concurrent streams per HTTP/2 connection. When this limit is exceeded, the load balancer sends GOAWAY or RST_STREAM frames, causing the JDK HttpClient to throw IOExceptions without an HTTP status code. Other cloud providers (Tencent Cloud CLB, Huawei Cloud ELB) and enterprise proxies exhibit similar behavior. JDK HttpClient's built-in ALPN negotiation cannot handle these mid-connection failures since the HTTP/2 connection was already successfully negotiated. **Changes:** - `JdkHttpTransport`: Added `http1_1Client` field, built eagerly in all three constructors via `buildClient(config, HttpVersion.HTTP_1_1)`. - `execute()`: Catches `HttpTransportException` with no status code when HTTP/2 is configured, then retries via `doSend(http1_1Client, jdkRequest)`. If the fallback also fails, the original error is thrown to preserve the root cause. - `stream()`: Added `onErrorResume` in the reactive chain that catches connection- level errors (IOException or HttpTransportException without status code) and retries with the HTTP/1.1 client via `sendAsyncStream()`. - Extracted `doSend()` and `sendAsyncStream()` helpers that accept a `HttpClient` parameter, making both protocol paths share the same execution logic. - `buildClient()` overloaded to accept an explicit `HttpVersion` parameter. - Added 7 tests in `JdkHttpTransportTest` covering the full decision tree: HTTP/2 fallback success (execute + stream), HTTP/1.1 normal success (execute + stream), HTTP/1.1 no fallback, HTTP/2 no fallback for HTTP errors, and fallback failure propagating original error. - [x] Code has been formatted with `mvn spotless:apply` - [x] All tests are passing (`mvn test`) - [x] Javadoc comments are complete and follow project conventions - [ ] Related documentation has been updated - [x] Code is ready for review
c5027a8 to
f567fd8
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When HTTP/2 connections are dropped by cloud infrastructure
(e.g. Alibaba Cloud SLB GOAWAY, RST_STREAM due to rate limiting),the transport now automatically retries the request with HTTP/1.1
instead of failing immediately. Previously, such connection-level
errors propagated directly to the caller with no protocol fallback.
Fixes #1287
1.0.12-SNAPSHOT
Background: Alibaba Cloud SLB/ALB enforces a default limit of concurrent
streams per HTTP/2 connection. When this limit is exceeded, the load balancer sends
GOAWAY or RST_STREAM frames, causing the JDK HttpClient to throw IOExceptions without
an HTTP status code. Other cloud providers (Tencent Cloud CLB, Huawei Cloud ELB) and
enterprise proxies exhibit similar behavior. JDK HttpClient's built-in ALPN
negotiation cannot handle these mid-connection failures since the HTTP/2 connection
was already successfully negotiated.
Changes:
JdkHttpTransport: Addedhttp1_1Clientfield, built eagerly in all threeconstructors via
buildClient(config, HttpVersion.HTTP_1_1).execute(): CatchesHttpTransportExceptionwith no status code when HTTP/2 isconfigured, then retries via
doSend(http1_1Client, jdkRequest). If the fallbackalso fails, the original error is thrown to preserve the root cause.
stream(): AddedonErrorResumein the reactive chain that catches connection-level errors (IOException or HttpTransportException without status code) and
retries with the HTTP/1.1 client via
sendAsyncStream().Extracted
doSend()andsendAsyncStream()helpers that accept aHttpClientparameter, making both protocol paths share the same execution logic.
buildClient()overloaded to accept an explicitHttpVersionparameter.Added 7 tests in
JdkHttpTransportTestcovering the full decision tree:HTTP/2 fallback success (execute + stream), HTTP/1.1 normal success (execute +
stream), HTTP/1.1 no fallback, HTTP/2 no fallback for HTTP errors, and fallback
failure propagating original error.
Code has been formatted with
mvn spotless:applyAll tests are passing (
mvn test)Javadoc comments are complete and follow project conventions
Related documentation has been updated
Code is ready for review