FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix flaky timeout in test-pipe-file-to-http by jakecastelli · Pull Request #53380 · nodejs/node · GitHub

/ node Public

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

Closed
jakecastelli wants to merge 1 commit into
nodejs:mainfrom
jakecastelli:fix-52963
ConversationCommits (1)ChecksFiles changed
Closed

test: fix flaky timeout in test-pipe-file-to-http#53380
jakecastelli wants to merge 1 commit into
nodejs:mainfrom
jakecastelli:fix-52963

Conversation

jakecastelli commented Jun 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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 added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 7, 2024
jakecastelli changed the title test: fix flaky timeout test: fix flaky timeout in test-pipe-file-to-http Jun 7, 2024

lpinca commented Jun 7, 2024

Copy link
Copy Markdown
Member

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

jakecastelli commented Jun 7, 2024

Copy link
Copy Markdown
Member 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 commented Jun 7, 2024

Copy link
Copy Markdown
Member

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

jakecastelli commented Jun 7, 2024

Copy link
Copy Markdown
Member Author

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

lpinca commented Jun 7, 2024

Copy link
Copy Markdown
Member

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

jakecastelli commented Jun 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 commented Jun 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 commented Jun 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member
  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 commented Jun 8, 2024

Copy link
Copy Markdown
Member 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 ---

jakecastelli mentioned this pull request Jun 26, 2024
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

richardlau richardlau approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

4 participants

Back | FazBrowse Home | New Git URL