From e2cda6dd9b2af76268a9a4896f0561d60d340c8f Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 8 Oct 2026 10:13:03 +0000 Subject: [PATCH] fix(http2): make StreamException a StreamTransportException and forward 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: Actual: StreamException: Which: is not an instance of 'StreamTransportException' ``` **After fix:** ```text 00:00 +1: All tests passed! ``` --- pkgs/http2/CHANGELOG.md | 4 ++++ pkgs/http2/lib/multiprotocol_server.dart | 9 ++++++++- pkgs/http2/lib/src/sync_errors.dart | 20 ++++++++++++-------- pkgs/http2/test/client_test.dart | 3 ++- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/pkgs/http2/CHANGELOG.md b/pkgs/http2/CHANGELOG.md index da5f98c628..ed5aa1eb30 100644 --- a/pkgs/http2/CHANGELOG.md +++ b/pkgs/http2/CHANGELOG.md @@ -23,6 +23,10 @@ stream when a stream is cancelled; drop the stream's queued `DATA` instead. - Stop head-of-line blocking `HEADERS`, `RST_STREAM`, and `GOAWAY` behind `DATA` that is waiting for connection-level flow-control credit. +- `StreamException` (a stream reset because of a `GOAWAY` or oversized headers) + now extends `StreamTransportException`, as documented for `TransportException` + subclasses. `MultiProtocolHttpServer.startServing` forwards an unexpected ALPN + protocol to `onError` instead of throwing inside the socket listener. ## 3.1.0 diff --git a/pkgs/http2/lib/multiprotocol_server.dart b/pkgs/http2/lib/multiprotocol_server.dart index 0f837c6030..abac03466e 100644 --- a/pkgs/http2/lib/multiprotocol_server.dart +++ b/pkgs/http2/lib/multiprotocol_server.dart @@ -91,7 +91,14 @@ class MultiProtocolHttpServer { ); } else { socket.destroy(); - throw Exception('Unexpected negotiated ALPN protocol: $protocol.'); + final error = Exception( + 'Unexpected negotiated ALPN protocol: $protocol.', + ); + if (onError != null) { + onError(error, StackTrace.current); + } else { + throw error; + } } }, onError: onError); diff --git a/pkgs/http2/lib/src/sync_errors.dart b/pkgs/http2/lib/src/sync_errors.dart index 16259817fd..fb08a0e8d0 100644 --- a/pkgs/http2/lib/src/sync_errors.dart +++ b/pkgs/http2/lib/src/sync_errors.dart @@ -2,6 +2,8 @@ // 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. +import '../transport.dart'; + class ProtocolException implements Exception { final String _message; @@ -44,27 +46,29 @@ class TerminatedException implements Exception { String toString() => 'TerminatedException: The object has been terminated.'; } -class StreamException implements Exception { - final String _message; +class StreamException extends StreamTransportException { final int streamId; + final String _details; - StreamException(this.streamId, this._message); + StreamException(this.streamId, String details) + : _details = details, + super(details); @override - String toString() => 'StreamException(stream id: $streamId): $_message'; + String toString() => 'StreamException(stream id: $streamId): $_details'; } class StreamClosedException extends StreamException { - StreamClosedException(super.streamId, [super.message = '']); + StreamClosedException(super.streamId, [super.details = '']); @override - String toString() => 'StreamClosedException(stream id: $streamId): $_message'; + String toString() => 'StreamClosedException(stream id: $streamId): $_details'; } class StreamRefusedException extends StreamException { - StreamRefusedException(super.streamId, [super.message = '']); + StreamRefusedException(super.streamId, [super.details = '']); @override String toString() => - 'StreamRefusedException(stream id: $streamId): $_message'; + 'StreamRefusedException(stream id: $streamId): $_details'; } diff --git a/pkgs/http2/test/client_test.dart b/pkgs/http2/test/client_test.dart index 662476fdb6..556ae9ea00 100644 --- a/pkgs/http2/test/client_test.dart +++ b/pkgs/http2/test/client_test.dart @@ -1321,7 +1321,8 @@ void main() { // Make sure we don't get messages/pushes on the terminated stream. unawaited( stream.incomingMessages.toList().catchError( - expectAsync1((e) { + expectAsync1((Object e) { + expect(e, isA()); expect( '$e', contains(