on-headers icon indicating copy to clipboard operation
on-headers copied to clipboard

feat: http2 support

Open bjohansebas opened this issue 11 months ago • 10 comments

Support for HTTP/2 is added as part of supporting HTTP/2 in compression and in the future, in express

bjohansebas avatar Jan 31 '25 01:01 bjohansebas

Currently, there is a test that, although it is not failing, is still showing a warning in Node.js. I will need to investigate the best way to proceed

image

bjohansebas avatar Jan 31 '25 02:01 bjohansebas

@jshttp/express-tc given that writeHead can receive a string as the second parameter, which is not supported in HTTP/2, should we throw an error?

bjohansebas avatar Jan 31 '25 14:01 bjohansebas

I can look through this in more detail, but I wanted to first ask if we should just drop node versions so we can remove the try/catch on require('http2)?

wesleytodd avatar Feb 17 '25 15:02 wesleytodd

I'm fine with removing support for older Node versions in this package, but I don't think that should go in this PR

bjohansebas avatar Feb 17 '25 23:02 bjohansebas

I guess we shouldn't do anything in this case, right? https://github.com/jshttp/on-headers/pull/16#issuecomment-2627449045

bjohansebas avatar Mar 05 '25 20:03 bjohansebas

I mean that is coming from the tests here right? I didn't really dig into that, but I am not sure throwing an error is desirable if node is treating it as a warning.

wesleytodd avatar Mar 05 '25 22:03 wesleytodd

Yep, that's why my philosophical doubt. So for now, no error, maybe it's good that Node.js throws the error.

bjohansebas avatar Mar 06 '25 21:03 bjohansebas

So far, we have been using the HTTP/1 compatibility layer provided by Node.js, but I'm going to look into running tests outside of that compatibility layer

bjohansebas avatar May 18 '25 22:05 bjohansebas

This works great with the compatibility layer, but it doesn't work with http2Session

bjohansebas avatar May 19 '25 00:05 bjohansebas

For compression, we need to modify the headers, which is quite easy with HTTP/1, but with HTTP/2 and without the compatibility layer, it gets complicated, since headers are only sent once. That means we can't modify them afterward, so we'd need to find a way to ensure that if there's a stream.respond, it's the only one sent and includes the headers we plan to send, without using this package

bjohansebas avatar May 19 '25 14:05 bjohansebas