light-my-request
light-my-request copied to clipboard
fastify
Support HTTP/2 (including Cleartext)
Prerequisites
- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the feature has not already been requested
🚀 Feature Proposal
Add options to inject() to send HTTP/2 requests instead of HTTP/1.1. My proposal would be app.inject({ http2: true })... or app.inject({ httpVersion: 2 })... but automatic detection could work too.
Motivation
When I add http2: true to Fastify options, all requests made with inject() receive 505 status code. For secure servers, adding https: { allowHttp1: true } works, but
- the user may not want to allow HTTP/1.1;
- the server may behave differently depending on the protocol version and it's desired to test HTTP/2 in particular.
As a workaround for 1, you could have https: { allowHttp1: process.env.JEST_WORKER_ID !== undefined }, but I don't like server code knowing about tests like that.
And of course, if the server wants to use H2C, there's no allowHttp1 option.
Example
No response
Very interesting. I don't there should be any different between HTTP/2 and HTTP/1.1 for this module, as there is no actual "data" flowing on the wire. I think there is just some internal API compatibility somewhere that we should handle.
There is https://github.com/fastify/light-my-request/blob/b8201068c3b07d5eaa5b7afbf71ad351dfa47dd2/lib/request.js#L103-L105
They don't seem to be used in light-my-request directly, but maybe something a Request is passed to uses them?
I suspect https://github.com/fastify/fastify/blob/c86750e02bcad8651bac63b717c26a2d46c5c440/lib/route.js#L435. Should I post the issue in https://github.com/fastify/fastify then instead of here?
On the other hand, if this is the issue, it's easy to fix by adding options as suggested.
I suspect https://github.com/fastify/fastify/blob/c86750e02bcad8651bac63b717c26a2d46c5c440/lib/route.js#L435. Should I post the issue in https://github.com/fastify/fastify then instead of here?
It should be changed in here, the detection in fastify is correct.
We should provide Connection: close only if the request is not HTTP/2.
We might consider to provide option override the http version.
Would it be OK to also release a 5.15.0 with this fix, so it can be used with Fastify 4 without additional resolutions? I see 5.14.0 was released after 6.0.0, for example.
Would it be OK to also release a 5.15.0 with this fix, so it can be used with Fastify 4 without additional resolutions?
Yes, we can definitely back-port the change and release a new version on old release line.