Skip to content

test: deflake test-http-dump-req-when-res-ends.js - #30360

Closed
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:deflake/test-http-dump-req-when-res-ends
Closed

test: deflake test-http-dump-req-when-res-ends.js#30360
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:deflake/test-http-dump-req-when-res-ends

Conversation

@lpinca

Copy link
Copy Markdown
Member

On some platforms the 'end' event might not be emitted because the
socket could be destroyed by the other peer while the client is still
sending the data triggering an error. Use the 'close' event instead.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Nov 11, 2019
@lpinca

Copy link
Copy Markdown
MemberAuthor

An alternative, arguably more explicit, fix could be:

diff --git a/test/parallel/test-http-dump-req-when-res-ends.js b/test/parallel/test-http-dump-req-when-res-ends.js
index 718797fae1..7aa02e35ee 100644
--- a/test/parallel/test-http-dump-req-when-res-ends.js+++ b/test/parallel/test-http-dump-req-when-res-ends.js@@ -48,10 +48,12 @@ server.listen(0, mustCall(function() {
res.resume();
- // Wait for the response.- res.on('end', function() {- server.close();+ res.on('error', function(err) {+ console.log(err);
});
++ res.on('aborted', close);+ res.on('end', close);
}));
req.on('error', function() {
@@ -60,3 +62,7 @@ server.listen(0, mustCall(function() {
// this is safe to ignore.
});
}));
++function close() {+ server.close();+}

On some platforms the `'end'` event might not be emitted because the
socket could be destroyed by the other peer while the client is still
sending the data triggering an error. Use the `'close'` event instead.
@lpinca
lpincaforce-pushed the deflake/test-http-dump-req-when-res-ends branch from 7fa136d to bd774d9CompareNovember 11, 2019 16:04
@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 13, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

lpinca added a commit that referenced this pull request Nov 15, 2019
On some platforms the `'end'` event might not be emitted because the
socket could be destroyed by the other peer while the client is still
sending the data triggering an error. Use the `'close'` event instead.
PR-URL: #30360
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@lpinca

Copy link
Copy Markdown
MemberAuthor

Landed in a4a089a.

@lpincalpinca closed this Nov 15, 2019
@lpinca
lpinca deleted the deflake/test-http-dump-req-when-res-ends branch November 15, 2019 15:26
MylesBorins pushed a commit that referenced this pull request Nov 17, 2019
On some platforms the `'end'` event might not be emitted because the
socket could be destroyed by the other peer while the client is still
sending the data triggering an error. Use the `'close'` event instead.
PR-URL: #30360
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeARBridgeAR mentioned this pull request Nov 19, 2019
targos pushed a commit that referenced this pull request Dec 1, 2019
On some platforms the `'end'` event might not be emitted because the
socket could be destroyed by the other peer while the client is still
sending the data triggering an error. Use the `'close'` event instead.
PR-URL: #30360
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BethGriggsBethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
On some platforms the `'end'` event might not be emitted because the
socket could be destroyed by the other peer while the client is still
sending the data triggering an error. Use the `'close'` event instead.
PR-URL: #30360
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BethGriggsBethGriggs mentioned this pull request Dec 23, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@lpinca@nodejs-github-bot@jasnell@addaleax@antsmartian@benjamingr@cjihrig@richardlau@BridgeAR@trivikr