Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions pkgs/http2/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
## 3.0.1-wip

- Gracefully handle receiving headers on a stream that the client has canceled. (#1799)
- **BREAKING (per #1913)**: distinguish graceful peer-initiated
shutdown (GOAWAY with `NO_ERROR` followed by transport close) from
forceful termination. Under the graceful path, pending operations
error with `TransportConnectionException(errorCode: NO_ERROR,
message: "Connection gracefully closed by peer.")` instead of the
prior conflated `"Connection is being forcefully terminated."` text.
Consumers that previously matched the forceful-termination text to
detect any peer-side shutdown should switch to observing
`TransportConnectionException.errorCode == ErrorCode.NO_ERROR` (or
the new message text) to distinguish graceful from forceful close.
(#1913)

## 3.0.0

Expand Down
38 changes: 31 additions & 7 deletions pkgs/http2/lib/src/connection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,20 @@ abstract class Connection {
}) {
// TODO: When do we complete here?
if (_state.state != ConnectionState.Terminated) {
// Detect graceful-shutdown path per issue #1913 shape (b): the peer
// has already sent a GOAWAY with NO_ERROR (transitioning us to
// passive-finishing state) and the transport then closed as part of
// that graceful flow. The transport-close that triggered this
// _terminate() call is expected, not an error condition — so we
// complete sub-components without an exception rather than surfacing
// `TransportConnectionException("Connection is being forcefully
// terminated.")`. BREAKING vs prior behavior where this path always
// raised the forceful-termination exception.
final isGracefulShutdown =
causedByTransportError &&
_state.isFinishing &&
(_state.finishingState & ConnectionState.FinishingPassive) != 0;

_state.state = ConnectionState.Terminated;

var cancelFuture = Future.sync(_frameReaderSubscription.cancel);
Expand All @@ -469,13 +483,23 @@ abstract class Connection {
// We ignore any errors after writing to [GoawayFrame]
});

// Close all lower level handlers with an error message.
// (e.g. if there is a pending connection.ping(), it's returned
// Future will complete with this error).
var exception = TransportConnectionException(
errorCode,
'Connection is being forcefully terminated.',
);
// Close all lower level handlers. For graceful peer-initiated
// shutdown (per #1913 shape (b)), use NO_ERROR + a distinct
// "gracefully closed" message so callers can distinguish from
// forceful termination. Pending operations (e.g. pending
// `connection.ping()` calls or pending settings ACKs) still
// complete with an error — they cannot succeed across a peer
// shutdown — but the error itself carries the graceful signal.
var exception =
isGracefulShutdown
? TransportConnectionException(
ErrorCode.NO_ERROR,
'Connection gracefully closed by peer.',
)
: TransportConnectionException(
errorCode,
'Connection is being forcefully terminated.',
);

// Close all streams & stream queues
_streams.terminate(exception);
Expand Down
37 changes: 37 additions & 0 deletions pkgs/http2/test/transport_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,43 @@ void main() {
clientSettings: const ClientSettings(streamWindowSize: 8096),
);
});

// Regression test for issue #1913: when a peer sends GOAWAY with
// NO_ERROR and then closes the underlying transport, pending
// operations on the local connection should error with a
// distinguishable graceful-shutdown signal — not with the
// "Connection is being forcefully terminated." message reserved for
// forceful termination paths. Per issue body shape (b) +
// brianquinlan's vote: the graceful-shutdown signal is
// breaking-cleaner than the prior conflated message.
transportTest('graceful-server-finish-distinct-shutdown-signal', (
TransportConnection client,
TransportConnection server,
) async {
// Race a pending client.ping() with server-side graceful shutdown.
// Under the prior behavior the ping errored with
// "Connection is being forcefully terminated."; per shape (b) the
// ping errors with the distinct graceful-shutdown signal.
var clientPingError = client.ping().catchError(
expectAsync2((Object e, StackTrace s) {
expect(e, isA<TransportConnectionException>());
// Graceful-shutdown signal: errorCode == NO_ERROR + distinct
// message. Either condition is sufficient for callers to
// distinguish from the forceful-termination path; we assert
// both for regression safety.
final tce = e as TransportConnectionException;
expect(tce.errorCode, 0); // ErrorCode.NO_ERROR
expect(
tce.toString(),
isNot(contains('forcefully terminated')),
reason:
'Graceful close must not surface forceful-termination text.',
);
}),
);
await server.finish();
await clientPingError;
});
});
}

Expand Down