Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing autobahn tests #3252

Closed
9 tasks done
KhafraDev opened this issue May 13, 2024 · 6 comments
Closed
9 tasks done

Failing autobahn tests #3252

KhafraDev opened this issue May 13, 2024 · 6 comments
Labels
bug Something isn't working websocket Pull requests or issues related to websocket and its standard

Comments

@KhafraDev
Copy link
Member

KhafraDev commented May 13, 2024

  • Case 2.6
  • Case 2.11
  • Case 5.5
  • Case 5.8
  • Case 9.3.1
  • Case 9.4.1
  • Case 9.3.2 -> Case 9.3.8
  • Case 9.4.2 -> Case 9.4.8
  • Case 10.1.1

Total tests failing: 15

All of these tests relate to fragmentation but I can't reproduce locally using ws. Seems to be something about receiving chunked data (could it be sending?).

@KhafraDev KhafraDev added bug Something isn't working websocket Pull requests or issues related to websocket and its standard labels May 13, 2024
@KhafraDev
Copy link
Member Author

Interestingly, at a certain threshold the fragmentation tests start to pass, such as 9.3.9 & 9.4.9. Sending out smaller chunks = tests fail for some reason.

@Uzlopak
Copy link
Contributor

Uzlopak commented May 13, 2024

What I am currently looking is 2.6. it seems that a frame containing 125 times 0xfe is not properly processed. I assume that we should actually fast forward the 125 times 0xfe but we dont, we do a consume(2) and not consume(payloadLength).

Undici:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/undici$ npm run test:websocket:autobahn
npm warn cli npm v10.7.0 does not support Node.js v23.0.0-nightly20240507be8d64ec14. This version of npm supports the following node versions: `^18.17.0 || >=20.5.0`. You can find the latest version at https://nodejs.org/.

> undici@6.16.1 test:websocket:autobahn
> node test/autobahn/client.js

(node:95584) [UNDICI-WS] Warning: WebSockets are experimental, expect them to change at any time.
(Use `node --trace-warnings ...` to show where the warning was created)
<Buffer 81 01>
<Buffer 88 00>
Running test case 1/1
<Buffer 89 7d>
<Buffer fe fe>
Error: Invalid opcode received
    at failWebsocketConnection (/home/aras/workspace/undici/lib/web/websocket/util.js:207:14)
    at ByteParser.run (/home/aras/workspace/undici/lib/web/websocket/receiver.js:73:11)
    at ByteParser._write (/home/aras/workspace/undici/lib/web/websocket/receiver.js:49:10)
    at writeOrBuffer (node:internal/streams/writable:564:12)
    at _write (node:internal/streams/writable:493:10)
    at Writable.write (node:internal/streams/writable:502:10)
    at Socket.onSocketData (/home/aras/workspace/undici/lib/web/websocket/connection.js:283:29)
    at Socket.emit (node:events:520:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
<Buffer 88 00>

ws:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/ws$ node test/autobahn
<Buffer 81 01>
<Buffer 88 00>
Running test case 1/1
<Buffer 89 7d>
<Buffer 88 02>
<Buffer 88 00>

@Uzlopak
Copy link
Contributor

Uzlopak commented May 13, 2024

The state at the beginning of the loop:

undici:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/undici$ npm run test:websocket:autobahn
npm warn cli npm v10.7.0 does not support Node.js v23.0.0-nightly20240507be8d64ec14. This version of npm supports the following node versions: `^18.17.0 || >=20.5.0`. You can find the latest version at https://nodejs.org/.

> undici@6.16.1 test:websocket:autobahn
> node test/autobahn/client.js

(node:96897) [UNDICI-WS] Warning: WebSockets are experimental, expect them to change at any time.
(Use `node --trace-warnings ...` to show where the warning was created)
{ state: 0 }
<Buffer 81 01>
{ state: 4 }
{ state: 0 }
<Buffer 88 00>
Running test case 1/1
{ state: 0 }
<Buffer 89 7d>
{ state: 0 }
<Buffer fe fe>
Error: Invalid opcode received
    at failWebsocketConnection (/home/aras/workspace/undici/lib/web/websocket/util.js:207:14)
    at ByteParser.run (/home/aras/workspace/undici/lib/web/websocket/receiver.js:74:11)
    at ByteParser._write (/home/aras/workspace/undici/lib/web/websocket/receiver.js:49:10)
    at writeOrBuffer (node:internal/streams/writable:564:12)
    at _write (node:internal/streams/writable:493:10)
    at Writable.write (node:internal/streams/writable:502:10)
    at Socket.onSocketData (/home/aras/workspace/undici/lib/web/websocket/connection.js:283:29)
    at Socket.emit (node:events:520:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
{ state: 0 }
<Buffer 88 00>

ws:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/ws$ node test/autobahn
{ state: 0 }
<Buffer 81 01>
{ state: 4 }
{ state: 0 }
<Buffer 88 00>
{ state: 4 }
Running test case 1/1
{ state: 0 }
<Buffer 89 7d>
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 0 }
{ state: 0 }
<Buffer 88 02>
{ state: 4 }
{ state: 0 }
<Buffer 88 00>
{ state: 4 }

@KhafraDev
Copy link
Member Author

It's literally a single bug and the rest of the tests will start passing. That's super annoying.

@Uzlopak
Copy link
Contributor

Uzlopak commented May 13, 2024

Its super annoying that the autobahn workflow did not post atleast once a comment on the PR. I would like to see it.
Anyhow, we should set then the env variable FAIL_ON_ERROR to true in the workflow to fail if somebody adds a PR which breaks the autobahn.

Also i wonder, there are some tests reporting being not strict. We should review them. Maybe we beed to make them strict?

@KhafraDev
Copy link
Member Author

non-strict is passing, no reason to touch them until there's nothing else to do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working websocket Pull requests or issues related to websocket and its standard
Projects
None yet
Development

No branches or pull requests

2 participants