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

test: fix flaky timeout in test-pipe-file-to-http #53380

Closed
wants to merge 1 commit into from

Conversation

jakecastelli
Copy link
Contributor

@jakecastelli jakecastelli commented Jun 7, 2024

Fixes: #52963

Reproduced with 1000 runs:

./tools/test.py --repeat 1000 test/parallel/test-pipe-file-to-http.js
=== release test-pipe-file-to-http ===                   
Path: parallel/test-pipe-file-to-http
Command: ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---


[06:16|% 100|+ 999|-   1]: Done                          

Failed tests:
./node/test/parallel/test-pipe-file-to-http.js

After apply the fix seems stable:

✗ ./tools/test.py --repeat 1000 test/parallel/test-pipe-file-to-http.js
[02:48|% 100|+ 1000|-   0]: Done                         

All tests passed.

./tools/test.py --repeat 1000 test/parallel/test-pipe-file-to-http.js
[02:44|% 100|+ 1000|-   0]: Done                         

All tests passed.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 7, 2024
@jakecastelli jakecastelli changed the title test: fix flaky timeout test: fix flaky timeout in test-pipe-file-to-http Jun 7, 2024
@lpinca
Copy link
Member

lpinca commented Jun 7, 2024

It doesn't make much sense. Why resuming the stream a bit later fixes it?

@jakecastelli
Copy link
Contributor Author

It doesn't make much sense. Why resuming the stream a bit later fixes it?

Thanks for potting it out @lpinca! I think you are absolutely right. I tried to dig it but without stack trace or meaning log I really couldn't get any direction.

I am wondering if the timeout generally makes the test flaky, shall we change it to setImmediate in this case?

@lpinca
Copy link
Member

lpinca commented Jun 7, 2024

See #52959 (comment). I think the issue here is the same.

@jakecastelli
Copy link
Contributor Author

Thanks again! I have read about that comment and now it rings the bell!

@lpinca
Copy link
Member

lpinca commented Jun 7, 2024

Can you try if the --jitless flag fixes the issue? I can't reproduce this test failure on my machine.

@jakecastelli
Copy link
Contributor Author

jakecastelli commented Jun 7, 2024

Sure, I will try it now. Can I ask two questions here:

  1. I am occasionally getting this error now:
./tools/test.py --repeat 10000 test/parallel/test-pipe-file-to-http.js
=== release test-pipe-file-to-http ===                   
Path: parallel/test-pipe-file-to-http
node:events:498
      throw er; // Unhandled 'error' event
      ^

Error: write EPIPE
    at afterWriteDispatched (node:internal/stream_base_commons:161:15)
    at writevGeneric (node:internal/stream_base_commons:144:3)
    at Socket._writeGeneric (node:net:952:11)
    at Socket._writev (node:net:961:8)
    at doWrite (node:internal/streams/writable:588:12)
    at clearBuffer (node:internal/streams/writable:767:5)
    at Writable.uncork (node:internal/streams/writable:523:7)
    at connectionCorkNT (node:_http_outgoing:1031:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:81:21)
Emitted 'error' event on ClientRequest instance at:
    at ClientRequest.onerror (node:internal/streams/readable:1026:14)
    at ClientRequest.emit (node:events:520:28)
    at Socket.socketErrorListener (node:_http_client:502:9)
    at Socket.emit (node:events:520:28)
    at emitErrorNT (node:internal/streams/destroy:170:8)
    at emitErrorCloseNT (node:internal/streams/destroy:129:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -32,
  code: 'EPIPE',
  syscall: 'write'
}

Node.js v23.0.0-pre

# OR

=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
node:events:498
      throw er; // Unhandled 'error' event
      ^

Error: write EPIPE
    at WriteWrap.onWriteComplete [as oncomplete] (node:internal/stream_base_commons:95:16)
Emitted 'error' event on ClientRequest instance at:
    at ClientRequest.onerror (node:internal/streams/readable:1026:14)
    at ClientRequest.emit (node:events:520:28)
    at Socket.socketErrorListener (node:_http_client:502:9)
    at Socket.emit (node:events:520:28)
    at emitErrorNT (node:internal/streams/destroy:170:8)
    at emitErrorCloseNT (node:internal/streams/destroy:129:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -32,
  code: 'EPIPE',
  syscall: 'write'
}

Node.js v23.0.0-pre

Do you have any idea why I am getting it 👀 any potentially misconfiguration or something else?

  1. If I remember correctly --jitless is a cli flag, how can I pass it into ./tools/test.py?

@jakecastelli
Copy link
Contributor Author

jakecastelli commented Jun 7, 2024

I can't reproduce this test failure on my machine.

Ummm I can still reproduce this on main on my machine - running 10,000 times and its around 30% atm got 2 timeout related failures:

=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Command: out/Release/node ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---


=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Command: out/Release/node ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---

@lpinca
Copy link
Member

lpinca commented Jun 7, 2024

  1. That error occurs when a peer closes the connection while the other peer writes data. Try to use the host option in the http.request() and set it to 127.0.0.1 to bypass "Happy Eyeballs" and see if it makes any difference.
  2. add a comment like this // Flags: --jitless ad the beginning of the file after 'use strict';.

@jakecastelli
Copy link
Contributor Author

Did another run with adding host: '127.0.0.1 option into http.request along with //Flags: --jitless

still 3 timeouts, still got 1 potentially peer writes when other peer closes
./tools/test.py --repeat 10000 test/parallel/test-pipe-file-to-http.js
=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Warning: disabling flag --expose_wasm due to conflicting flags
Command: out/Release/node --jitless ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---


=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Warning: disabling flag --expose_wasm due to conflicting flags
Command: out/Release/node --jitless ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---


=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Warning: disabling flag --expose_wasm due to conflicting flags
node:events:498
      throw er; // Unhandled 'error' event
      ^

Error: write EPIPE
    at afterWriteDispatched (node:internal/stream_base_commons:161:15)
    at writevGeneric (node:internal/stream_base_commons:144:3)
    at Socket._writeGeneric (node:net:952:11)
    at Socket._writev (node:net:961:8)
    at doWrite (node:internal/streams/writable:588:12)
    at clearBuffer (node:internal/streams/writable:767:5)
    at Writable.uncork (node:internal/streams/writable:523:7)
    at connectionCorkNT (node:_http_outgoing:1031:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:81:21)
Emitted 'error' event on ClientRequest instance at:
    at ClientRequest.onerror (node:internal/streams/readable:1026:14)
    at ClientRequest.emit (node:events:520:28)
    at Socket.socketErrorListener (node:_http_client:502:9)
    at Socket.emit (node:events:520:28)
    at emitErrorNT (node:internal/streams/destroy:170:8)
    at emitErrorCloseNT (node:internal/streams/destroy:129:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -32,
  code: 'EPIPE',
  syscall: 'write'
}

Node.js v23.0.0-pre
Command: out/Release/node --jitless ./node/test/parallel/test-pipe-file-to-http.js


=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Warning: disabling flag --expose_wasm due to conflicting flags
Command: out/Release/node --jitless ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test-pipe-file-to-http is flaky (timeout)
4 participants