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

Investigate flaky test-http-destroyed-socket-write2 #4066

Closed
Trott opened this issue Nov 29, 2015 · 19 comments · Fixed by #36120
Closed

Investigate flaky test-http-destroyed-socket-write2 #4066

Trott opened this issue Nov 29, 2015 · 19 comments · Fixed by #36120
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. http Issues or PRs related to the http subsystem. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.

Comments

@Trott
Copy link
Member

Trott commented Nov 29, 2015

@Trott Trott added http Issues or PRs related to the http subsystem. windows Issues and PRs related to the Windows platform. test Issues and PRs related to the tests. labels Nov 29, 2015
@Trott
Copy link
Member Author

Trott commented Nov 29, 2015

Stress test to see if flakiness is detectable via stress test: https://ci.nodejs.org/job/node-stress-single-test/89/nodes=win2012r2/

@Trott
Copy link
Member Author

Trott commented Nov 29, 2015

Closing #4064, #4065, and #4066. All three tests were flaky in the same CI build/run on just win2012r2, but have never been flaky before. Stress test runs on all three tests did not produce any test failures. Chalking this up to something weird that probably happened inside CI itself just this one time.

/cc @nodejs/build in case it's something they want to investigate (but totally get it if no one really wants to go down that rabbit hole right now or ever).

@Trott Trott closed this as completed Nov 29, 2015
@BridgeAR BridgeAR added macos Issues and PRs related to the macOS platform / OSX. and removed windows Issues and PRs related to the Windows platform. labels May 23, 2020
@BridgeAR
Copy link
Member

I am reopening this one, since it started to fail a couple of times recently.

@ronag could you have a look?

https://ci.nodejs.org/job/node-test-commit-osx/34333/nodes=osx1014/testReport/junit/(root)/test/parallel_test_http_destroyed_socket_write2/

@BridgeAR BridgeAR reopened this May 23, 2020
@BridgeAR
Copy link
Member

@ronag

This comment has been minimized.

@BridgeAR
Copy link
Member

@ronag would you be so kind and open a PR to fix that? That would be great!

@ronag
Copy link
Member

ronag commented May 23, 2020

@BridgeAR Sure. I'll put it on my list.

@ronag
Copy link
Member

ronag commented May 24, 2020

@BridgeAR How do I see in CI what Node version run is against? I can see the commit 77e6691 but not what version that was based on.

I don't think this flakiness should be able to occur on Node 14+. Just wanted to make sure whether that's the case or whether this is for older LTS versions.

@BridgeAR
Copy link
Member

@ronag click on "Parameters". It's v15.0.0-pre.

@ronag
Copy link
Member

ronag commented May 24, 2020

@BridgeAR Did

Stress test: ci.nodejs.org/job/node-stress-single-test/74

reproduce it? Is it reproducible with these stress test single tests?

ronag added a commit to nxtedition/node that referenced this issue May 25, 2020
Make assertions in flaky test stricter to possibly
make it easier to determine the root cause.

Refs: nodejs#4066
@ronag
Copy link
Member

ronag commented May 25, 2020

Made a PR to make the test stricter. If the error occurs again after landing that PR please ping me.

@BridgeAR
Copy link
Member

reproduce it? Is it reproducible with these stress test single tests?

It did not fail. Seems like it's difficult to reproduce that way.

ronag added a commit that referenced this issue May 26, 2020
Make assertions in flaky test stricter to possibly
make it easier to determine the root cause.

Refs: #4066

PR-URL: #33539
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@ronag
Copy link
Member

ronag commented May 26, 2020

I've landed a PR which might make this easier to debug. Please ping me if you encounter this again.

@Trott
Copy link
Member Author

Trott commented Jun 4, 2020

@ronag Just came across it now again in https://pipelines.actions.githubusercontent.com/A76h8lUgAkJrcbixwhN7BUAsUydYrQl8Ux3JbU2S9vu9HqtlrL/_apis/pipelines/1/runs/30920/signedlogcontent/17?urlExpires=2020-06-04T19%3A01%3A31.8124169Z&urlSigningMethod=HMACV1&urlSignature=mTQIwRpEDtvfZ5PIYD%2FC%2F%2F7Ky1nk17UdFua%2BYdvppzw%3D.

2020-06-04T14:33:18.1314890Z not ok 848 parallel/test-http-destroyed-socket-write2
2020-06-04T14:33:18.1316040Z   ---
2020-06-04T14:33:18.1316740Z   duration_ms: 0.213
2020-06-04T14:33:18.1316980Z   severity: fail
2020-06-04T14:33:18.1317130Z   exitcode: 1
2020-06-04T14:33:18.1317880Z   stack: |-
2020-06-04T14:33:18.1318430Z     assert.js:103
2020-06-04T14:33:18.1319340Z       throw new AssertionError(obj);
2020-06-04T14:33:18.1322280Z       ^
2020-06-04T14:33:18.1322970Z     
2020-06-04T14:33:18.1325630Z     AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
2020-06-04T14:33:18.1326820Z     + actual - expected
2020-06-04T14:33:18.1326950Z     
2020-06-04T14:33:18.1327900Z     + 'ERR_STREAM_DESTROYED'
2020-06-04T14:33:18.1328590Z     - 'ECONNRESET'
2020-06-04T14:33:18.1329810Z         at ClientRequest.<anonymous> (/Users/runner/runners/2.263.0/work/node/node/node-v15.0.0-nightly2020-06-04xxxx/test/parallel/test-http-destroyed-socket-write2.js:67:16)
2020-06-04T14:33:18.1330580Z         at ClientRequest.<anonymous> (/Users/runner/runners/2.263.0/work/node/node/node-v15.0.0-nightly2020-06-04xxxx/test/common/index.js:363:15)
2020-06-04T14:33:18.1331190Z         at ClientRequest.emit (events.js:315:20)
2020-06-04T14:33:18.1331340Z         at emitErrorNt (_http_outgoing.js:666:43)
2020-06-04T14:33:18.1331870Z         at processTicksAndRejections (internal/process/task_queues.js:85:21) {
2020-06-04T14:33:18.1333790Z       generatedMessage: true,
2020-06-04T14:33:18.1335490Z       code: 'ERR_ASSERTION',
2020-06-04T14:33:18.1335950Z       actual: 'ERR_STREAM_DESTROYED',
2020-06-04T14:33:18.1336390Z       expected: 'ECONNRESET',
2020-06-04T14:33:18.1336810Z       operator: 'strictEqual'
2020-06-04T14:33:18.1337280Z     }
2020-06-04T14:33:18.1337590Z   ...

@ronag
Copy link
Member

ronag commented Jun 4, 2020

@Trott: there is a revert PR pending for v14 and follow up PR's pending for master.

@Trott
Copy link
Member Author

Trott commented Jun 5, 2020

@Trott: there is a revert PR pending for v14 and follow up PR's pending for master.

Ah! OK, I was pasting that in there because the last comment said to ping if it was seen again. Awesome. Thanks.

codebytere pushed a commit that referenced this issue Jun 18, 2020
Make assertions in flaky test stricter to possibly
make it easier to determine the root cause.

Refs: #4066

PR-URL: #33539
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
codebytere pushed a commit that referenced this issue Jun 30, 2020
Make assertions in flaky test stricter to possibly
make it easier to determine the root cause.

Refs: #4066

PR-URL: #33539
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
codebytere pushed a commit that referenced this issue Jul 8, 2020
Make assertions in flaky test stricter to possibly
make it easier to determine the root cause.

Refs: #4066

PR-URL: #33539
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott Trott added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Aug 1, 2020
@Trott
Copy link
Member Author

Trott commented Aug 16, 2020

Showed up in node-daily-master last night as a timeout.

https://ci.nodejs.org/job/node-test-commit-osx/35784/nodes=osx1014/consoleText

not ok 1756 parallel/test-http-destroyed-socket-write2
  ---
  duration_ms: 120.117
  severity: fail
  exitcode: -15
  stack: |-
    timeout

I am able to replicate locally on macOS:

$ tools/test.py -j96 --repeat 192 test/parallel/test-http-destroyed-socket-write2.js
=== release test-http-destroyed-socket-write2 ===                   
Path: parallel/test-http-destroyed-socket-write2
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-destroyed-socket-write2.js
--- TIMEOUT ---
[02:00|% 100|+ 191|-   1]: Done    
$

Pinging @ronag again since they may already have ideas.

@Trott
Copy link
Member Author

Trott commented Oct 3, 2020

Showed up again last night as a timeout on macOS:

https://ci.nodejs.org/job/node-test-commit-osx/36476/nodes=osx1015/console

00:46:42 not ok 1805 parallel/test-http-destroyed-socket-write2
00:46:42   ---
00:46:42   duration_ms: 120.112
00:46:42   severity: fail
00:46:42   exitcode: -15
00:46:42   stack: |-
00:46:42     timeout
00:46:42   ...

@Trott
Copy link
Member Author

Trott commented Oct 3, 2020

Able to replicate locally:

$ tools/test.py -j8 --repeat=1024  test/parallel/test-http-destroyed-socket-write2.js
=== release test-http-destroyed-socket-write2 ===                    
Path: parallel/test-http-destroyed-socket-write2
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-destroyed-socket-write2.js
--- TIMEOUT ---
=== release test-http-destroyed-socket-write2 ===                    
Path: parallel/test-http-destroyed-socket-write2
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-destroyed-socket-write2.js
--- TIMEOUT ---
[02:42|% 100|+ 1022|-   2]: Done                                     
$ 

@lpinca lpinca closed this as completed in e682814 Nov 18, 2020
codebytere pushed a commit that referenced this issue Nov 22, 2020
Ensure that the write occurs in the same tick where the socket is
destroyed by the other peer.

PR-URL: #36120
Fixes: #36081
Fixes: #4066
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
BethGriggs pushed a commit that referenced this issue Dec 10, 2020
Ensure that the write occurs in the same tick where the socket is
destroyed by the other peer.

PR-URL: #36120
Fixes: #36081
Fixes: #4066
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
BethGriggs pushed a commit that referenced this issue Dec 10, 2020
Ensure that the write occurs in the same tick where the socket is
destroyed by the other peer.

PR-URL: #36120
Fixes: #36081
Fixes: #4066
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
BethGriggs pushed a commit that referenced this issue Dec 15, 2020
Ensure that the write occurs in the same tick where the socket is
destroyed by the other peer.

PR-URL: #36120
Fixes: #36081
Fixes: #4066
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. http Issues or PRs related to the http subsystem. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants