Skip to content

Commit 3cb894c

Browse files
divybotlittledivy
andauthored
fix(ext/node): enable test-http2-server-shutdown-redundant (#33793)
## Summary Enables `test-http2-server-shutdown-redundant` in node_compat suite. ## Test plan - [x] `cargo test --test node_compat -- test-http2-server-shutdown-redundant` --------- Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 60b234f commit 3cb894c

2 files changed

Lines changed: 24 additions & 2 deletions

File tree

ext/node/polyfills/http2.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,15 @@ function submitGoaway(code, lastStreamID, opaqueData) {
494494
}
495495
debugSessionObj(this, "submitting goaway");
496496
this[kUpdateTimer]();
497+
// Track that this side initiated a GOAWAY with a non-zero error code.
498+
// The peer will react by tearing down its streams with that same code,
499+
// which arrives at us as RST_STREAM(<code>); nghttp2 then reports stream
500+
// close to JS as a non-NO_ERROR rstCode. Without this flag, _destroy
501+
// would synthesize an unhandled ERR_HTTP2_STREAM_ERROR for what is just
502+
// the peer's expected response to our locally-initiated shutdown.
503+
if (code !== NGHTTP2_NO_ERROR && this[kState].sentGoawayCode == null) {
504+
this[kState].sentGoawayCode = code;
505+
}
497506
this[kHandle].goaway(code, lastStreamID, opaqueData);
498507
scheduleSendPending(this);
499508
}
@@ -2106,8 +2115,19 @@ class Http2Stream extends Duplex {
21062115

21072116
// RST code 8 not emitted as an error as its used by clients to signify
21082117
// abort and is already covered by aborted event, also allows more
2109-
// seamless compatibility with http1
2110-
if (err == null && code !== NGHTTP2_NO_ERROR && code !== NGHTTP2_CANCEL) {
2118+
// seamless compatibility with http1.
2119+
//
2120+
// Also skip when this session locally initiated the shutdown via a
2121+
// non-NO_ERROR goaway (sentGoawayCode): the peer's RST_STREAM with
2122+
// that same code is the expected reaction to our own goaway, not a
2123+
// stream-level error to surface (see test-http2-server-shutdown-
2124+
// redundant.js).
2125+
if (
2126+
err == null &&
2127+
code !== NGHTTP2_NO_ERROR &&
2128+
code !== NGHTTP2_CANCEL &&
2129+
sessionState.sentGoawayCode == null
2130+
) {
21112131
err = new ERR_HTTP2_STREAM_ERROR(nameForErrorCode[code] || code);
21122132
}
21132133

@@ -3410,6 +3430,7 @@ class Http2Session extends EventEmitter {
34103430
flags: SESSION_FLAGS_PENDING,
34113431
goawayCode: null,
34123432
goawayLastStreamID: null,
3433+
sentGoawayCode: null,
34133434
streams: new SafeMap(),
34143435
pendingStreams: new SafeSet(),
34153436
pendingAck: 0,

tests/node_compat/config.jsonc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,6 +1836,7 @@
18361836
"parallel/test-http2-server-setLocalWindowSize.js": {},
18371837
"parallel/test-http2-server-shutdown-before-respond.js": {},
18381838
"parallel/test-http2-server-shutdown-options-errors.js": {},
1839+
"parallel/test-http2-server-shutdown-redundant.js": {},
18391840
"parallel/test-http2-server-startup.js": {},
18401841
"parallel/test-http2-server-timeout.js": {},
18411842
"parallel/test-http2-session-graceful-close.js": {},

0 commit comments

Comments
 (0)