revert: "websocket: pre-calculated length" #3290
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
mcollina
left a comment
There was a problem hiding this comment.
Can you add a test to avoid further regressions?
|
The tests catch it already, it's just that the Autobahn workflow doesn't quite work as expected and it uses a previous commit rather than the current one. Once the issue linked is resolved it'll be fixed. |
|
The tests in #3283 are failing because of this, or you can see the first comment that the bot made before the edit. There is a lot broken right now I'd appreciate if we could land this in the meantime. |
|
@mcollina |
This reverts 13523fd which broke permessage-deflate support. See nodejs#3289 for why the tests did not catch this.
7c4908c to
93c653e
Compare
|
@KhafraDev |
|
Yes, you have to understand that these tests send GB of frames over that need to parsed and mirrored to the server. The longer the tests, the more likely the tests are to succeed because the server will instantly fail/close the connection if it receives something unexpected. |
|
Ah, ok. ;) |
the autobahn workflow runs tests that detected this regression originally
|
cc @tsctx I'm pretty positive that the issue came with the custom Buffer.concat. |
This reverts 13523fd which broke permessage-deflate support.
See #3289 for why the tests did not catch this.