search

molnarg / node-http2

An HTTP/2 client and server implementation for node.js
MIT License
1.79k stars 185 forks source link

v3.3.1 breaks firefox unit tests #167

Closed nwgh closed 8 years ago

nwgh commented 8 years ago

Since I'd like to keep things up to date appropriately in mozilla-central, I should track down which of my changes in v3.3.1 breaks Firefox unit tests. I suspect I have an off-by-one https://github.com/molnarg/node-http2/commit/e32f7363afa69133b49152b6e963e52da0a42f78 somewhere that our tests don't exercise, but no bisect to back that up (yet).

This should be fixed before continuing on (and this is what I get for not running Firefox unit tests before committing and releasing v3.3.1).

nwgh commented 8 years ago

I was wrong. git bisect reports 148eccf3fe215d491ab165992d0026656076fafd as the first bad revision (which fixed #146).

nwgh commented 8 years ago

Ah-ha! It's the "HEADERS is the only thing that can create a stream" bit of that patch (I had a bad feeling about that when I wrote it, but went ahead with it anyway). While it's true that HEADERS is the only thing that creates a stream, Firefox sends a bunch of PRIORITY frames with never-used stream IDs to create a priority tree, which in node-http2 takes the same path. The appropriate fix is to remove the HEADERS-only check when creating a stream.