Repository navigation
fix(http2_adapter): apply receiveTimeout to response body streaming - #2632
Open
itszain-dev wants to merge 2 commits into
Open
itszain-dev wants to merge 2 commits into
itszain-dev wants to merge 2 commits into
Conversation
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.
Fixes #2608, Part 1 only. Part 2, the GOAWAY retry, is intentionally not included here.
Motivation: in Http2Adapter, receiveTimeout only wraps the future that completes when headers arrive. Once headers are received, a server that sends a few body bytes and then stalls forever will hang the download forever, even with a receiveTimeout set. That matches the documented meaning of receiveTimeout as time between data chunks, and it matches how the other adapter behaves.
Change: an inactivity timer is started on headers and reset on every data message. If no body data arrives within receiveTimeout, the timer cancels the subscription and sends DioException.receiveTimeout into the body stream, then closes it. The timer is cancelled on body end, on error, on redirect, and when the response body is closed, so it does not fire after a normal finish. The default behaviour is unchanged when receiveTimeout is not set.
Tests: a new regression test in plugins/http2_adapter/test/http2_test.dart uses a local HTTP/2 server that sends headers and a few bytes, then stalls forever. With receiveTimeout set to 200 milliseconds, reading the body throws DioException.receiveTimeout. The test has a 2 second guard so it fails fast instead of hanging if the fix regresses.
Verification, stated honestly: dart analyze on the changed files reports no issues, and dart format is clean after a formatting commit. The new test passes locally, and it fails without the fix, throwing a timeout instead of receiveTimeout. The full http2_test.dart suite locally shows 9 passed, 1 skipped, and 5 failures, all in network dependent tests that reach external hosts and fail in this environment with handshake and connection errors. The same network dependent tests also fail on unmodified code here, so those failures are not introduced by this change. The remaining network dependent checks, and CI, are unverified locally for that reason.
Changelog: an entry under Unreleased in plugins/http2_adapter/CHANGELOG.md is included.
AI attribution: implementation and tests for this pull request were done with Muse, an AI assistant, and the human submitter owns every line and will respond to review feedback.
Note on scope: this is a timeout and connection handling change, so it is called out explicitly here as a sensitive area change.