Skip to content

Commit 2aba6f5

Browse files
committed
Bug 1989900 - Make NodeServer hide ERR_HTTP2_INVALID_STREAM errors instead of throwing them r=necko-reviewers,jesup
The issue here is that: 1. The client cancels a request mid-flight 2. This destroys the HTTP/2 stream on the Node.js side 3. But the server code still tries to call stream.respond() on the destroyed stream 4. This causes ERR_HTTP2_INVALID_STREAM errors to be logged Differential Revision: https://phabricator.services.mozilla.com/D265749
1 parent 991d7c7 commit 2aba6f5

File tree

1 file changed

+67
-10
lines changed

1 file changed

+67
-10
lines changed

netwerk/test/httpserver/NodeServer.sys.mjs

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,10 @@ class HTTP2ProxyCode {
641641
);
642642
} catch (e) {
643643
// The channel may have been closed already.
644-
if (e.message != "The stream has been destroyed") {
644+
if (
645+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
646+
!e.message.includes("The stream has been destroyed")
647+
) {
645648
throw e;
646649
}
647650
}
@@ -673,17 +676,35 @@ class HTTP2ProxyCode {
673676
}
674677
if (headers[":method"] !== "CONNECT") {
675678
// Only accept CONNECT requests
676-
stream.respond({ ":status": 405 });
679+
try {
680+
stream.respond({ ":status": 405 });
681+
} catch (e) {
682+
if (
683+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
684+
!e.message.includes("The stream has been destroyed")
685+
) {
686+
throw e;
687+
}
688+
}
677689
stream.end();
678690
return;
679691
}
680692

681693
const authorization_token = headers["proxy-authorization"];
682694
if (auth && !authorization_token) {
683-
stream.respond({
684-
":status": 407,
685-
"proxy-authenticate": "Basic realm='foo'",
686-
});
695+
try {
696+
stream.respond({
697+
":status": 407,
698+
"proxy-authenticate": "Basic realm='foo'",
699+
});
700+
} catch (e) {
701+
if (
702+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
703+
!e.message.includes("The stream has been destroyed")
704+
) {
705+
throw e;
706+
}
707+
}
687708
stream.end();
688709
return;
689710
}
@@ -695,7 +716,16 @@ class HTTP2ProxyCode {
695716
try {
696717
global.socketCounts[socket.remotePort] =
697718
(global.socketCounts[socket.remotePort] || 0) + 1;
698-
stream.respond({ ":status": 200 });
719+
try {
720+
stream.respond({ ":status": 200 });
721+
} catch (e) {
722+
if (
723+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
724+
!e.message.includes("The stream has been destroyed")
725+
) {
726+
throw e;
727+
}
728+
}
699729
socket.pipe(stream);
700730
stream.pipe(socket);
701731
} catch (exception) {
@@ -710,7 +740,16 @@ class HTTP2ProxyCode {
710740
// If we already sent headers when the socket connected
711741
// then sending the status again would throw.
712742
if (!stream.sentHeaders) {
713-
stream.respond({ ":status": status });
743+
try {
744+
stream.respond({ ":status": status });
745+
} catch (e) {
746+
if (
747+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
748+
!e.message.includes("The stream has been destroyed")
749+
) {
750+
throw e;
751+
}
752+
}
714753
}
715754
stream.end();
716755
} catch (exception) {
@@ -858,7 +897,16 @@ class NodeWebSocketHttp2ServerCode extends BaseNodeHTTPServerCode {
858897

859898
global.h2Server.on("stream", (stream, headers) => {
860899
if (headers[":method"] === "CONNECT") {
861-
stream.respond();
900+
try {
901+
stream.respond();
902+
} catch (e) {
903+
if (
904+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
905+
!e.message.includes("The stream has been destroyed")
906+
) {
907+
throw e;
908+
}
909+
}
862910

863911
const ws = new WS(null);
864912
stream.setNoDelay = () => {};
@@ -873,7 +921,16 @@ class NodeWebSocketHttp2ServerCode extends BaseNodeHTTPServerCode {
873921
ws.send("test");
874922
});
875923
} else {
876-
stream.respond();
924+
try {
925+
stream.respond();
926+
} catch (e) {
927+
if (
928+
e.code !== "ERR_HTTP2_INVALID_STREAM" &&
929+
!e.message.includes("The stream has been destroyed")
930+
) {
931+
throw e;
932+
}
933+
}
877934
stream.end("ok");
878935
}
879936
});

0 commit comments

Comments
 (0)