Repository navigation
fix(http2): make StreamException a StreamTransportException and forward ALPN errors to onError - #2009
Conversation
166ae6a to
9300221
Compare
9300221 to
6ddca40
Compare
6ddca40 to
a288db5
Compare
a288db5 to
29db67f
Compare
PR Health
Coverage
|
| File | Coverage |
|---|---|
| pkgs/http2/lib/multiprotocol_server.dart | 💔 85 % ⬇️ 5 % |
| pkgs/http2/lib/src/sync_errors.dart | 💚 74 % ⬆️ 2 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
License Headers ✔️
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
| Files |
|---|
| no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/http_multi_server/test/cert.dart |
This check can be disabled by tagging the PR with skip-license-check.
Breaking changes ✔️
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| http2 | Non-Breaking | 3.1.0 | 3.2.0-wip | 3.2.0-wip | ✔️ |
This check can be disabled by tagging the PR with skip-breaking-check.
Unused Dependencies ✔️
| Package | Status |
|---|---|
| http2 | ✔️ All dependencies utilized correctly. |
For details on how to fix these, see dependency_validator.
This check can be disabled by tagging the PR with skip-unused-dependencies-check.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
| Package | Leaked API symbol | Leaking sources |
|---|
This check can be disabled by tagging the PR with skip-leaking-check.
Changelog Entry ✔️
| Package | Changed Files |
|---|
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check.
…rd ALPN errors to onError > [!NOTE] > This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem. ### Summary Two small error-handling fixes: 1. **`StreamException` (`lib/src/sync_errors.dart`)**: used by `StreamHandler.processGoawayFrame` when aborting streams with `id > lastStreamId` ("this stream was not processed and can therefore be retried") and by `_rejectOversizedHeaders`. It implemented `Exception` directly instead of extending `StreamTransportException`, so callers that catch `TransportException` / `StreamTransportException` as documented by `package:http2/transport.dart` missed exactly the error that tells them a retry is safe. 2. **`MultiProtocolHttpServer.startServing` (`lib/multiprotocol_server.dart`)**: when a client negotiated an unexpected ALPN protocol, `startServing` threw synchronously inside the `_serverSocket.listen` data callback instead of forwarding the error to `onError` when one was provided. ### Changes - **`lib/src/sync_errors.dart`**: `StreamException extends StreamTransportException`, `toString()` format unchanged. `StreamHandler.processStreamFrame` catches the concrete subtypes (`StreamClosedException`, `StreamRefusedException`, `StreamException`), so its `RST_STREAM` error-code mapping is unaffected. - **`lib/multiprotocol_server.dart`**: forward the unexpected-protocol error to `onError` if provided. ### Test Verification (Fails Before $\rightarrow$ Passes After) `goaway-terminates-nonprocessed-streams` in `test/client_test.dart` now also asserts `e is StreamTransportException`. **Before fix:** ```text 00:00 +0 -1: test/client_test.dart: client-tests client-errors goaway-terminates-nonprocessed-streams [E] Expected: <Instance of 'StreamTransportException'> Actual: StreamException:<StreamException(stream id: 1): Remote end was telling us to stop. This stream was not processed and can therefore be retried (on a new connection).> Which: is not an instance of 'StreamTransportException' ``` **After fix:** ```text 00:00 +1: All tests passed! ```
29db67f to
e2cda6d
Compare
Note
This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem.
Summary
Two small error-handling fixes:
StreamException(lib/src/sync_errors.dart): used byStreamHandler.processGoawayFramewhen aborting streams withid > lastStreamId("this stream was not processed and can therefore be retried") and by_rejectOversizedHeaders. It implementedExceptiondirectly instead of extendingStreamTransportException, so callers that catchTransportException/StreamTransportExceptionas documented bypackage:http2/transport.dartmissed exactly the error that tells them a retry is safe.MultiProtocolHttpServer.startServing(lib/multiprotocol_server.dart): when a client negotiated an unexpected ALPN protocol,startServingthrew synchronously inside the_serverSocket.listendata callback instead of forwarding the error toonErrorwhen one was provided.Changes
lib/src/sync_errors.dart:StreamException extends StreamTransportException,toString()format unchanged.StreamHandler.processStreamFramecatches the concrete subtypes (StreamClosedException,StreamRefusedException,StreamException), so itsRST_STREAMerror-code mapping is unaffected.lib/multiprotocol_server.dart: forward the unexpected-protocol error toonErrorif provided.Test Verification (Fails Before$\rightarrow$ Passes After)
goaway-terminates-nonprocessed-streamsintest/client_test.dartnow also assertse is StreamTransportException.Before fix:
After fix: