Skip to content

fix(http2): make StreamException a StreamTransportException and forward ALPN errors to onError - #2009

Merged
mosuem merged 1 commit into
masterfrom
mosum/http2-5-frame-reader-cancel-and-goaway-exception
Oct 30, 2026
Merged

fix(http2): make StreamException a StreamTransportException and forward ALPN errors to onError#2009
mosuem merged 1 commit into
masterfrom
mosum/http2-5-frame-reader-cancel-and-goaway-exception

Conversation

@mosuem

@mosuem mosuem commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

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:

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:

00:00 +1: All tests passed!

@mosuem
mosuem added this pull request to stack #2011 October 8, 2026 08:32
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from 166ae6a to 9300221 Compare October 8, 2026 10:36
@mosuem mosuem changed the title fix(http2): cancel byte stream on FrameReader cancel and make StreamException extend StreamTransportException fix(http2): make StreamException a StreamTransportException and forward ALPN errors to onError Oct 8, 2026
@mosuem
mosuem removed this pull request from stack #2011 October 8, 2026 10:38
@mosuem
mosuem changed the base branch from mosum/http2-4-control-frame-queue-and-rst-order to mosum/http2-4b-no-hol-blocking-on-exhausted-window October 8, 2026 10:38
@mosuem
mosuem added this pull request to stack #2016 October 8, 2026 10:42
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from 9300221 to 6ddca40 Compare October 8, 2026 12:04
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from 6ddca40 to a288db5 Compare October 9, 2026 07:54
@mosuem
mosuem removed this pull request from stack #2016 October 9, 2026 07:55
@mosuem
mosuem added this pull request to stack #2020 October 9, 2026 07:55
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from a288db5 to 29db67f Compare October 9, 2026 08:18
@github-actions

github-actions Bot commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown

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 (for example, for non-user-visible changes when the package version is already at -wip).

@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from 29db67f to e2cda6d Compare October 9, 2026 08:48
@mosuem
mosuem removed this pull request from stack #2020 October 22, 2026 06:44
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from e2cda6d to 43e4932 Compare October 22, 2026 06:44
@mosuem
mosuem added this pull request to stack #2030 October 22, 2026 06:47
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from 43e4932 to 17b48b1 Compare October 22, 2026 07:38
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch 2 times, most recently from 2a9aef0 to 72553dd Compare October 22, 2026 08:01
@mosuem
mosuem marked this pull request as ready for review October 22, 2026 08:03
@mosuem
mosuem requested a review from brianquinlan October 22, 2026 08:13
@mosuem
mosuem removed this pull request from stack #2030 October 22, 2026 08:19
@mosuem
mosuem added this pull request to stack #2032 October 22, 2026 08:19
Base automatically changed from mosum/http2-4b-no-hol-blocking-on-exhausted-window to master October 23, 2026 05:31
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from 72553dd to d301bc3 Compare October 23, 2026 05:31
@mosuem
fix(http2): make StreamException a StreamTransportException and forwa…
e0a04ec 
…rd ALPN errors to onError

> [!NOTE]
> This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem.

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.

- **`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.

`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!
```
@mosuem
mosuem force-pushed the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch from d301bc3 to e0a04ec Compare October 23, 2026 05:32
@mosuem
mosuem merged commit 05d0b18 into master Oct 30, 2026
51 checks passed
@mosuem
mosuem deleted the mosum/http2-5-frame-reader-cancel-and-goaway-exception branch October 30, 2026 06:08
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Nov 3, 2026
@natebosch dart-scoped@luci-project-accounts.iam.gserviceaccount.com
[deps] rev core, ecosystem, http, i18n, shelf, test, tools
c48ebb8 
Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/d7bd45c..24e09c4):
  24e09c4c  Sun Nov 1 21:12:19 2026 +0000  dependabot[bot]  Bump the github-actions group with 4 updates (dart-lang/core#1037)
  87ac1b4c  Fri Oct 30 16:02:43 2026 +0100  Michael Thomsen  [os_detect] Deprecate package:os_detect in favor of package:platform (dart-lang/core#1003)
  8debf335  Fri Oct 30 01:57:36 2026 +0530  Sahaja Pallapothula  Clarify Characters equality and Unicode normalization (dart-lang/core#1031)

ecosystem (https://github.com/dart-lang/ecosystem/compare/24b9ba8..f8faabf):
  f8faabf  Sun Nov 1 18:16:20 2026 +0000  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/ecosystem#481)
  27c2fc8  Sat Oct 31 12:36:53 2026 +0100  Moritz  Fix publish workflow pins, add tag_prefix to health, and update docs (dart-lang/ecosystem#480)
  05641a9  Sat Oct 31 09:56:42 2026 +0100  Moritz  Restore deprecated publish.yaml workflow and firehose CLI entrypoint (dart-lang/ecosystem#479)
  78abf72  Fri Oct 30 08:24:07 2026 -0700  Kevin Moore  fix(repo_manage): use pub_semver for changelog -wip transitions (dart-lang/ecosystem#475)
  0bfa654  Fri Oct 30 09:33:57 2026 +0100  Moritz  Move publish check to health workflow and use individual package publishing workflows from `setup-dart` (dart-lang/ecosystem#454)
  adf7979  Thu Oct 29 23:46:49 2026 -0700  Kevin Moore  test(firehose): use hasLength, containsPair, and isFalse (dart-lang/ecosystem#477)

http (https://github.com/dart-lang/http/compare/c76dfb3..8c0d66a):
  8c0d66a  Fri Oct 30 09:54:41 2026 -0700  Brian Quinlan  chore(cupertino_http): check ffigen generation (dart-lang/http#2044)
  c8737d1  Fri Oct 30 09:11:46 2026 +0100  Moritz  chore(http): run Firefox tests with -headless instead of xvfb-run (dart-lang/http#2033)
  05d0b18  Fri Oct 30 07:08:19 2026 +0100  Moritz  fix(http2): make StreamException a StreamTransportException and forward ALPN errors to onError (dart-lang/http#2009)

i18n (https://github.com/dart-lang/i18n/compare/4c7b4ad..7475ce6):
  7475ce64  Sat Oct 31 02:49:28 2026 +0100  Moritz  [intl4x] Fix NumberFormat.percent ignoring digits (dart-lang/i18n#1115)

shelf (https://github.com/dart-lang/shelf/compare/62d3e4e..9946de5):
  9946de5  Sun Nov 1 12:27:59 2026 +0000  dependabot[bot]  build(deps): bump the github-actions group with 3 updates (dart-lang/shelf#554)

test (https://github.com/dart-lang/test/compare/6d74720..2a9e017):
  2a9e0175  Sun Nov 1 03:47:24 2026 +0000  dependabot[bot]  Bump the github-actions group with 6 updates (dart-lang/test#2813)
  55cd5cd5  Fri Oct 30 13:43:47 2026 -0700  Nate Bosch  Migrate publishing workflows from firehose to setup-dart (dart-lang/test#2812)
  10dc78c0  Fri Oct 30 11:21:53 2026 -0700  Nate Bosch  [checks] Describe what the stream did in failures (dart-lang/test#2811)
  c11ad06d  Fri Oct 30 08:59:57 2026 -0700  Nate Bosch  [checks] Add CheckFailure.describeNested (dart-lang/test#2793)
  0137b43e  Fri Oct 30 05:40:43 2026 +0530  Shriraj Jadhav  [checks] Use the Iterable.last member for the last expectation (dart-lang/test#2807)
  50c9f454  Thu Oct 29 15:11:39 2026 -0700  Nate Bosch  [checks] Collapse clauses for described conditions (dart-lang/test#2794)

tools (https://github.com/dart-lang/tools/compare/3980dca..1579f8d):
  1579f8d69  Mon Nov 2 02:44:58 2026 +0530  Joshua  fix(coverage): reject package: and dart: URIs outside known roots in Resolver (dart-lang/tools#2764)
  f869fdca4  Sun Nov 1 10:26:34 2026 +0100  dart-ecosystem-bot[bot]  [boring] Update BoringSSL to ed9209cf (dart-lang/tools#2774)
  d10e2b87f  Sun Nov 1 10:26:11 2026 +0100  dart-ecosystem-bot[bot]  [boring] Pin the prebuilt binaries of boring-binaries-20261101-33e9e69722af (dart-lang/tools#2777)
  33e9e6972  Sun Nov 1 05:56:30 2026 +0000  dependabot[bot]  build(deps): bump the github-actions group with 4 updates (dart-lang/tools#2775)
  f84d0d0d2  Sun Nov 1 05:36:12 2026 +0000  dependabot[bot]  build(deps): bump ffigen from 22.0.0 to 23.0.0 in /pkgs/boring (dart-lang/tools#2776)
  14ad5e515  Fri Oct 30 10:48:58 2026 -0700  Kevin Moore  [mime] Harden BoundMultipartStream error routing, headers, and epilogue (dart-lang/tools#2629)
  7a780d48c  Fri Oct 30 12:57:03 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid rescanning failed plain-text runs in InlineParser (dart-lang/tools#2757)
  3671d33a9  Fri Oct 30 12:56:48 2026 +0100  Jonas Finnemann Jensen  [markdown] Index footnote definitions and labels on Document for O(1) lookup (dart-lang/tools#2760)
  6f38e9fda  Fri Oct 30 12:56:09 2026 +0100  Jonas Finnemann Jensen  [markdown] Bound the domain of extended URL autolinks to DNS limits (dart-lang/tools#2759)
  aa0c38544  Fri Oct 30 12:20:40 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid quadratic removeRange in _combineAdjacentText (dart-lang/tools#2758)
  f425baf46  Fri Oct 30 11:16:28 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid rescanning failed space runs in LineBreakSyntax (dart-lang/tools#2756)
  c7eafbec3  Fri Oct 30 11:09:45 2026 +0100  Jonas Finnemann Jensen  [markdown] Reject '<' in AutolinkSyntax (dart-lang/tools#2755)
  86ed0f902  Fri Oct 30 11:08:25 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid quadratic backtracking in headerPattern (dart-lang/tools#2754)
  8ea07517f  Fri Oct 30 07:45:42 2026 +0100  Moritz  [boring] Regenerate the x509-limbo expected failures in the refresh job (dart-lang/tools#2712)
  40235c365  Thu Oct 29 14:46:17 2026 +0100  Jonas Finnemann Jensen  [markdown] Make HtmlBlockSyntax.parseChildLines iterative (dart-lang/tools#2749)
  0ff03f643  Thu Oct 29 14:41:33 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid re-escaping nested image alt text in ImageSyntax.createNode (dart-lang/tools#2752)
  784654d01  Thu Oct 29 14:37:11 2026 +0100  Jonas Finnemann Jensen  [markdown] Add test cases covering parser and renderer edge cases (dart-lang/tools#2753)
  4f2718ad8  Thu Oct 29 14:36:52 2026 +0100  Jonas Finnemann Jensen  [markdown] Make Document._parseInlineContent iterative (dart-lang/tools#2750)
  cac1421d5  Thu Oct 29 14:17:06 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid quadratic duplicate heading ID probing in HtmlRenderer (dart-lang/tools#2748)
  66880bcd3  Thu Oct 29 14:11:27 2026 +0100  Jonas Finnemann Jensen  [markdown] Check footnote label and empty map before scanning footnote keys (dart-lang/tools#2751)
  7f8696523  Thu Oct 29 14:07:06 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid quadratic blank-line lookahead in CodeBlockSyntax (dart-lang/tools#2747)
  9da6bb291  Thu Oct 29 14:03:04 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid exponential and quadratic backtracking in tablePattern (dart-lang/tools#2746)

dartdoc is held back at db94437, before dart-lang/dartdoc#4312, because its
mock SDK version changes conflict in the google3 3-way merge with the hotfix
in cl/1005963642.

native is held back at e0ca7c6, before dart-lang/native#3742, because
deprecating `MacOSCodeConfig(targetVersion: ...)` causes a fatal
`deprecated_member_use` diagnostic in `pkg/dartdev/lib/src/native_assets.dart`.

web is held back at 4e60e61, before dart-lang/web#596, because it changes
`package:web` signatures (e.g. `innerHTML=`, `parseFromString`, `Worker()`)
from `JSAny` to `String` in ways that break callers in google3.

Change-Id: I53d2cbde741c45b8fd33bca42742549819dd332b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/572780
Reviewed-by: Brian Quinlan <[email protected]>
Commit-Queue: Nate Bosch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brianquinlan brianquinlan brianquinlan approved these changes

Assignees

No one assigned

Labels

package:http2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mosuem @brianquinlan