Added HTTP/2 support for onFinished and isFinished functions #100

GroophyLifefor · 2026-01-11T12:53:34Z

Core Approach:

Detected HTTP/2 streams using instanceof http2.Http2Stream
HTTP/2 streams already emit 'close' events naturally - we just needed to listen for them
Used stream.destroyed and stream.closed properties to check if stream is finished

Related to #57, can you review @Phillip9587 ?

bjohansebas

Okay, I haven’t fully reviewed your PR yet, although I’ve also been working on this (see #87). I’m implementing tests using compatibility mode, while yours uses native HTTP/2

GroophyLifefor · 2026-01-11T16:39:41Z

Okay, I haven’t fully reviewed your PR yet, although I’ve also been working on this (see #87). I’m implementing tests using compatibility mode, while yours uses native HTTP/2

Hi @bjohansebas , thanks for the note and for working on this as well. I want to better understand your approach. When you mention compatibility mode, is the main goal to cover how most users run HTTP two in Node, or are there specific lifecycle or edge cases that native Http2Stream based handling might miss. I am wondering if supporting both native HTTP two and compatibility mode would make sense here.

bjohansebas · 2026-01-11T21:15:47Z

I feel we could handle both cases. For Express HTTP/2 support, it will be done through the compatibility layer, which is why I’ve mainly focused on that in my other PR (the reason being that this is exactly why the compatibility mode exists; it’s also how Fastify handles it, and it saves us from having to re-implement many of our middlewares and Express itself).

I think we could support both possible cases and merge our two PRs. That’s how I’m currently thinking about it, although I’m still testing things, since I’ve discovered some specific edge cases that affect both Node.js’s implementation and ours.

Edit: I still haven’t found an edge case, or at least I think so. I was just doing some of the tests incorrectly in #87 .

jonchurch · 2026-01-12T07:36:52Z

index.js

+  if (http2.Http2Stream && msg instanceof http2.Http2Stream) {
+    return Boolean(msg.destroyed || msg.closed)
+  }
+


These added lines will never execute because both Http2ServerResponse and Http2Stream have the writeableEnded property, so we will return early in the first condition which checks for it on line 72

~~I don't think any functional changes are required to master to get http2 support, just tests.~~

Looks like the request abort scenario does need stream.closed check, but it need to come first or else we will return writeablEnded which is not sufficient for all cases in http2

Hi @jonchurch , I did some changes on that with more correct handling, it's now not uses native http2 because not needing, just checks with executive.

Console Test Output

npm run test 01/12/26 18:50:11 PM > on-finished@2.4.1 test > mocha --reporter spec --check-leaks test/ HTTP/2 onFinished(res, listener) with ServerResponse http2 ✔ should fire when response finishes http2 ✔ should include the stream object http2 ✔ should fire when response is destroyed when called after finish http2 http2 ✔ should fire immediately with async local storage http2 http2 ✔ should persist store in callback with async local storage http2 ✔ should persist store in callback with multiple streams http2 http2 ✔ should fire for each stream independently when client closes connection http2 ✔ should fire the callback when stream errors http2 ✔ should fire the callback isFinished(res) http2 ✔ should return false for unfinished stream http2 http2 ✔ should return true for finished stream http2 ✔ should return true for destroyed stream onFinished(res, listener) ✔ should invoke listener given an unknown object ✔ should throw TypeError if listener is not a function when the response finishes http1 ✔ should fire the callback http1 ✔ should include the response object when called after finish http1 http1 ✔ should fire when called after finish when async local storage http1 http1 ✔ should presist store in callback when async local storage http1 ✔ should presist store in callback when using keep-alive http1 http1 ✔ should fire for each response when requests pipelined http1 http1 http1 http1 ✔ should fire for each request when response errors http1 ✔ should fire with error http1 ✔ should include the response object when the response aborts http1 ✔ should execute the callback when calling many times on same response http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 ✔ should not print warnings isFinished(res) ✔ should return undefined for unknown object http1 ✔ should be false before response finishes http1 http1 ✔ should be true after response finishes when requests pipelined http1 http1 http1 http1 ✔ should have correct state when socket shared http1 http1 ✔ should handle aborted requests when response errors http1 http1 ✔ should return true when the response aborts http1 http1 ✔ should return true onFinished(req, listener) ✔ should throw TypeError if listener is not a function when the request finishes http1 ✔ should fire the callback http1 ✔ should include the request object when called after finish http1 http1 ✔ should fire when called after finish when async local storage http1 http1 ✔ should presist store in callback when async local storage http1 ✔ should presist store in callback when using keep-alive http1 http1 ✔ should fire for each request when request errors http1 ✔ should fire with error http1 ✔ should include the request object when requests pipelined http1 http1 ✔ should handle socket errors when the request aborts http1 ✔ should execute the callback when calling many times on same request http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 http1 ✔ should not print warnings when CONNECT method http1 ✔ should fire when request finishes http1 http1 ✔ should fire when called after finish when Upgrade request http1 ✔ should fire when request finishes http1 http1 ✔ should fire when called after finish isFinished(req) ✔ should return undefined for unknown object http1 ✔ should be false before request finishes http1 http1 ✔ should be true after request finishes when request data buffered http1 http1 ✔ should be false before request finishes when request errors http1 http1 ✔ should return true when the request aborts http1 http1 ✔ should return true when CONNECT method http1 ✔ should be true immediately http1 http1 ✔ should be true after request finishes when Upgrade request http1 ✔ should be true immediately http1 http1 ✔ should be true after request finishes 58 passing (410ms)

GroophyLifefor · 2026-01-14T19:25:05Z

Do not merge this PR, moved to #87

Will closed after #87 merged.

feat: add HTTP/2 support for onFinished and isFinished functions

36ad000

GroophyLifefor mentioned this pull request Jan 11, 2026

http2 compat tests #27

Open

bjohansebas reviewed Jan 11, 2026

View reviewed changes

jonchurch requested changes Jan 12, 2026

View reviewed changes

jonchurch mentioned this pull request Jan 12, 2026

chore: add http2 support #87

Open

GroophyLifefor added 3 commits January 12, 2026 18:44

refactor: simplify isFinished function for HTTP/2 support

7fabaf4

refactor: enhance isFinished function for better HTTP/2 handling

a32b0d3

fix: removed console logs

b0c8d74

GroophyLifefor requested a review from jonchurch January 12, 2026 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added HTTP/2 support for onFinished and isFinished functions #100

Added HTTP/2 support for onFinished and isFinished functions #100

GroophyLifefor commented Jan 11, 2026 •

edited

Loading

Uh oh!

bjohansebas left a comment

Uh oh!

GroophyLifefor commented Jan 11, 2026

Uh oh!

bjohansebas commented Jan 11, 2026 •

edited

Loading

Uh oh!

jonchurch Jan 12, 2026 •

edited

Loading

Uh oh!

GroophyLifefor Jan 12, 2026

Uh oh!

GroophyLifefor commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Added HTTP/2 support for onFinished and isFinished functions #100

Are you sure you want to change the base?

Added HTTP/2 support for onFinished and isFinished functions #100

Conversation

GroophyLifefor commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjohansebas left a comment

Choose a reason for hiding this comment

Uh oh!

GroophyLifefor commented Jan 11, 2026

Uh oh!

bjohansebas commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonchurch Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GroophyLifefor Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

GroophyLifefor commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GroophyLifefor commented Jan 11, 2026 •

edited

Loading

bjohansebas commented Jan 11, 2026 •

edited

Loading

jonchurch Jan 12, 2026 •

edited

Loading