creationix/http-parser-js

Broken on node 10.1.0+

richardlay opened this issue · 10 comments

Our tests fail with HPE_INVALID_METHOD when we upgrade to node 10.1.0. Node 10.0.0 still works.
I believe this is related to this change: nodejs/node#20250

Encountering the same issue and Node upgrade is blocked. Any updates?

Not from me. I removed the library and haven't had any response related issues (yet). We're using Node 10.9.0.

I've not had time to look at it - dealing with a PS4 launch, so open source stuff has taken a back burner for a while, but maybe my schedule will open up in a couple weeks ^_^. If anyone wants to take a stab at a fix and PR, I'll happily look at it.

What exactly is the case that is failing? The automated test suite is (mostly) passing on Node 10, except for some edge cases (e.g. calling req.end() a second time behaves differently now, but no one should be doing that anyway). Nothing I tried fails with the error you've mentioned. Are you using this library as a replacement for http parsing as a client or a server? Can you provide a minimal test case that fails?

I upgraded to Node.js 10.12.0, but haven't encountered this specific issue.
I did find a more fundamental issue, which has been fixed in my PR (#58). Can you try my fork to see if it still an issue?

@Jimbly
I write a test code.
https://github.com/blackknifes/http-parser-test
You can pull this repo.

run server: npm run server
run http-parser-test: npm run dev

The source code use electron V3.x. You can see error on webpage or console of devtools.

When I run dev, electron just silently fails, not sure what's up with that. However, looking at the code, it does look like electron is running everything in a child process, so it's quite likely it's the same issue with Node child processes being unable to monkey-patch in HTTP Parser.

When I run dev, electron just silently fails, not sure what's up with that. However, looking at the code, it does look like electron is running everything in a child process, so it's quite likely it's the same issue with Node child processes being unable to monkey-patch in HTTP Parser.

Can fix it at node 10.x ? if can't, I will write http module with socket...

Assuming it's the child process issue, there's currently no way to fix it on our end, a fix would need to be made in Node to allow monkey-patching again. v10.0.0 works, but not v10.1.0+. We have an issue opened with the Node.js repo, but have not had any response yet.

Node v10.15.1 has been released, which includes my PR, which should resolve issues with being unable to monkeypatch in http-parser-js.

There is no way to reliably monkeypatch the http parser in Node versions v10.1 through v10.15.0, so you'll need a newer (or older) Node version. If anyone is still having issues on Node v10.15.1+, please let us know!