| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Signed-off-by: Tim Perry <pimterry@gmail.com>
Previously, stream errors were completely swallowed and never exposed. With this change, they're exposed only if there is a listener for them - this is the exact same pattern used in HTTP/1 itself, so hopefully a good fit for the compat API! This also changes the compat API to only report 'finish' when the writable has actually finished - previously all closes were reporting with finish, even when the writable was aborted part way through.
|
Review requested:
|
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 98.06452% with 3 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## main #63249 +/- ##
========================================
Coverage 90.02% 90.03%
========================================
Files 713 714 +1
Lines 224950 225330 +380
Branches 42530 42615 +85
========================================
+ Hits 202513 202875 +362
- Misses 14220 14236 +16
- Partials 8217 8219 +2
... and 37 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was a problem hiding this comment.
lgtm
Sorry, something went wrong.
|
Pushed updates to the deprecation docs, tightened the tests a little, and covered an extra case: it turns out respondWithFD was also broken in very similar ways (e.g. finish fired when you didn't actually finish writing) and that wasn't previously fixed with the rest. Now updated and fixed to match the same behaviour there too. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGMT !
Sorry, something went wrong.
|
It looks like this is ready to land. |
Sorry, something went wrong.
|
In theory yes, but it needs re-review (ping @nodejs/http2 & @nodejs/http) and 2x @nodejs/tsc approval for the breaking change |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
… with Code.Unavailable When a peer (typically a proxy such as Envoy) resets a stream with RST_STREAM code NO_ERROR before the response completed, Node's http2 client (pre nodejs/node#63249) surfaces the truncation as a clean, error-free end-of-stream. The protocol layer then fails parsing the truncated body with misleading errors: "protocol error: incomplete envelope" (invalid_argument) or "protocol error: missing status" (internal). Track whether a request that declared "TE: trailers" (as gRPC requests always do) received its terminating trailers or a trailers-only response, and reject with Code.Unavailable from the close handler when the stream ended with rstCode 0 without either. Verified: package tests 113/113 (5 new failure-mode tests fail without the fix, 4 new non-regression tests), conformance 6258/6258 client and 4718/4718 server. Signed-off-by: Robin Wieruch <hello@rwieruch.com>
…285244) ## Human summary Improves the detection of completed requests by additionally checking for `res.writableEnded`. This works around a quirk in Node.js when running in http/2, which is resolved in 26+: nodejs/node#63249. Checking for `res.writableEnded` does not change anything when running in http/1, as this is always true when `res.writableFinished`. This changes behavior for http/2 by properly detecting aborted requests. This should result in downstream consumers having a more reliable signal for when a client request is aborted, allowing them to cancel jobs such as in-flight ES requests regardless of http protocol. We detected this while writing scout tests for #285153 ## AI description When an HTTP/2 client destroys a stream mid-request (RST_STREAM / NGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation), Node's Http2ServerResponse emits 'close' with writableFinished === true even though nothing was written. isCompleted() relied on writableFinished alone, so the !isCompleted filter swallowed the event and request.events.aborted$ never fired — consumers (route handlers, auth providers) could not observe HTTP/2 client aborts. HTTP/1 was unaffected because writableFinished stays false there. Treat a request as completed only when writableFinished && writableEnded: writableEnded only becomes true once the server actually ended the response, and is truthful on both protocols and for abrupt TCP-level disconnects. Adds integration coverage in http2_protocol.test.ts driving a real HTTP/2 TLS session that resets the stream while the handler is pending, plus a control test asserting completed$ (and not aborted$) on normal completion. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…uests (#285244) (#285382) # Backport This will backport the following commits from `main` to `9.4`: - [[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)](#285244) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Larry Gregory","email":"larry.gregory@elastic.co"},"sourceCommit":{"committedDate":"2026-08-17T11:55:38Z","message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289","branchLabelMapping":{"^v9.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","backport:all-open","v9.6.0"],"title":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests","number":285244,"url":"https://github.com/elastic/kibana/pull/285244","mergeCommit":{"message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.6.0","branchLabelMappingKey":"^v9.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/285244","number":285244,"mergeCommit":{"message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289"}}]}] BACKPORT--> Co-authored-by: Larry Gregory <larry.gregory@elastic.co> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…uests (#285244) (#285383) # Backport This will backport the following commits from `main` to `9.5`: - [[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)](#285244) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Larry Gregory","email":"larry.gregory@elastic.co"},"sourceCommit":{"committedDate":"2026-08-17T11:55:38Z","message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289","branchLabelMapping":{"^v9.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","backport:all-open","v9.6.0"],"title":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests","number":285244,"url":"https://github.com/elastic/kibana/pull/285244","mergeCommit":{"message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.6.0","branchLabelMappingKey":"^v9.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/285244","number":285244,"mergeCommit":{"message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289"}}]}] BACKPORT--> Co-authored-by: Larry Gregory <larry.gregory@elastic.co> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…lastic#285244) ## Human summary Improves the detection of completed requests by additionally checking for `res.writableEnded`. This works around a quirk in Node.js when running in http/2, which is resolved in 26+: nodejs/node#63249. Checking for `res.writableEnded` does not change anything when running in http/1, as this is always true when `res.writableFinished`. This changes behavior for http/2 by properly detecting aborted requests. This should result in downstream consumers having a more reliable signal for when a client request is aborted, allowing them to cancel jobs such as in-flight ES requests regardless of http protocol. We detected this while writing scout tests for elastic#285153 ## AI description When an HTTP/2 client destroys a stream mid-request (RST_STREAM / NGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation), Node's Http2ServerResponse emits 'close' with writableFinished === true even though nothing was written. isCompleted() relied on writableFinished alone, so the !isCompleted filter swallowed the event and request.events.aborted$ never fired — consumers (route handlers, auth providers) could not observe HTTP/2 client aborts. HTTP/1 was unaffected because writableFinished stays false there. Treat a request as completed only when writableFinished && writableEnded: writableEnded only becomes true once the server actually ended the response, and is truthful on both protocols and for abrupt TCP-level disconnects. Adds integration coverage in http2_protocol.test.ts driving a real HTTP/2 TLS session that resets the stream while the handler is pending, plus a control test asserting completed$ (and not aborted$) on normal completion.
…quests (#285244) (#285403) # Backport This will backport the following commits from `main` to `8.19`: - [[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)](#285244) <!--- Backport version: 11.0.2 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Larry Gregory","email":"larry.gregory@elastic.co"},"sourceCommit":{"committedDate":"2026-08-17T11:55:38Z","message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289","branchLabelMapping":{"^v9.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","backport:all-open","v9.6.0","v9.4.6","v9.5.2"],"title":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests","number":285244,"url":"https://github.com/elastic/kibana/pull/285244","mergeCommit":{"message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.6.0","branchLabelMappingKey":"^v9.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/285244","number":285244,"mergeCommit":{"message":"[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244)\n\n## Human summary\n\nImproves the detection of completed requests by additionally checking\nfor `res.writableEnded`. This works around a quirk in Node.js when\nrunning in http/2, which is resolved in 26+:\nhttps://github.com/nodejs/node/pull/63249.\n\nChecking for `res.writableEnded` does not change anything when running\nin http/1, as this is always true when `res.writableFinished`. This\nchanges behavior for http/2 by properly detecting aborted requests.\n\nThis should result in downstream consumers having a more reliable signal\nfor when a client request is aborted, allowing them to cancel jobs such\nas in-flight ES requests regardless of http protocol.\n\nWe detected this while writing scout tests for\nhttps://github.com//pull/285153\n\n## AI description\nWhen an HTTP/2 client destroys a stream mid-request (RST_STREAM /\nNGHTTP2_CANCEL, e.g. an AbortController cancel or browser navigation),\nNode's Http2ServerResponse emits 'close' with writableFinished === true\neven though nothing was written. isCompleted() relied on\nwritableFinished alone, so the !isCompleted filter swallowed the event\nand request.events.aborted$ never fired — consumers (route handlers,\nauth providers) could not observe HTTP/2 client aborts. HTTP/1 was\nunaffected because writableFinished stays false there.\n\nTreat a request as completed only when writableFinished &&\nwritableEnded: writableEnded only becomes true once the server actually\nended the response, and is truthful on both protocols and for abrupt\nTCP-level disconnects.\n\nAdds integration coverage in http2_protocol.test.ts driving a real\nHTTP/2 TLS session that resets the stream while the handler is pending,\nplus a control test asserting completed$ (and not aborted$) on normal\ncompletion.\n\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>","sha":"a19e27a5dc885a40a31998f728c450d8aee27289"}},{"branch":"9.4","label":"v9.4.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/285382","number":285382,"state":"MERGED","mergeCommit":{"sha":"8cb7c7f6e2b21af321412223579d0eab0f0b8134","message":"[9.4] [Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244) (#285382)\n\n# Backport\n\nThis will backport the following commits from `main` to `9.4`:\n- [[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests\n(#285244)](https://github.com/elastic/kibana/pull/285244)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\nCo-authored-by: Larry Gregory <larry.gregory@elastic.co>\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>"}},{"branch":"9.5","label":"v9.5.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/285383","number":285383,"state":"MERGED","mergeCommit":{"sha":"aeff16c0cbf64223297cf496f698e03a793c24bf","message":"[9.5] [Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests (#285244) (#285383)\n\n# Backport\n\nThis will backport the following commits from `main` to `9.5`:\n- [[Core] Fix KibanaRequest aborted$ never emitting for HTTP/2 requests\n(#285244)](https://github.com/elastic/kibana/pull/285244)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\nCo-authored-by: Larry Gregory <larry.gregory@elastic.co>\nCo-authored-by: Claude Fable 5 <noreply@anthropic.com>"}}]}] BACKPORT-->
| Back | FazBrowse Home | New Git URL |
This PR replaces #62923, which aims to fix #56627. The goal here is to make hard-shutdown (RST_STREAM messages) for HTTP/2 streams behave predictably and remove a bunch of footguns. This became a bit of a rabbit hole, but I think I have a good understanding of the problem and a much better behaviour that should be workable, although is definitely requires some breaking changes (though ~all affected code is broken in some ways today anyway).
To implement this, this PR does a few related things:
This fixes a few major existing issues. Most notably, without this PR, if you have a stream where you're finished writing but you're only part way through reading (e.g. normal client request flow) and you get a non-error RST_STREAM (a remote stream cancellation) then we close the readable cleanly, with no errors. In effect, we currently truncate cancelled responses with no warning whatsoever. The existing 'aborted' event is only emitted for incomplete writes, not reads.
This resolves that issue for reads. For writes, it stays close to existing behaviour but relaxes one key case: if you've read everything, and receive a RST_STREAM cancellation while writing (most commonly: a client aborts a request while a server is responding) then the writable closes immediately but doesn't emit an 'error' event. Effectively, we treat this as a quiet shutdown. This roughly matches HTTP/1 behaviour, and avoid spurious errors server-side. Even though there's no 'error' event, it's still easy to detect this: if you get 'close' without 'finish' (without writableFinished being true) while writing then the client has cancelled the request.
We could make aborted match the error behaviour here, but I've left it as is to avoid churn there, since I think we should deprecate it regardless so there's no point making unnecessary breaking changes in the meantime.
In the compat API, there's two other related existing problems this fixes:
As a happy side bonus, as part of this work I hunted down the underlying issue for #58252: buggy cleanup of reset streams. This includes a reliable repro for the underlying issue (test/parallel/test-http2-server-stream-destroy-after-reset.js) and fixes that test and the flaky one as well, through with better cleanup behaviour in onStreamClose.
I suspect there'll be some debate around this, and it is definitely a non-trivial breaking change, but on the flip side this does show some quite substantial major issues in the current APIs, including silent read & write data loss everywhere, so I think it's worthwhile.