Skip to content

Conversation

@lpinca
Copy link
Member

@lpincalpinca commented Apr 1, 2021

The socket might be destroyed by the other peer while data is still
being written. Add the missing error handler.

Fixes: #37291

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 1, 2021
client.on('error',function(err){
// The socket might be destroyed by the other peer while data is still
// being written.
assert.strictEqual(err.code,'ECONNRESET');
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think err.code can also be 'EPIPE' or 'ECONNABORTED' but I was not able to reproduce. Should we handle that in advance or do it if/when it will occur?

Copy link
Member

@TrottTrottApr 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say let's do it when it occurs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can add a comment right now that says "EPIPE might be valid too but we haven't seen it yet" or something like that?

@nodejs-github-bot
Copy link
Collaborator

The socket might be destroyed by the other peer while data is still being written. Add the missing error handler. Fixes: nodejs#37291
@nodejs-github-bot
Copy link
Collaborator

lpinca added a commit that referenced this pull request Apr 8, 2021
The socket might be destroyed by the other peer while data is still being written. Add the missing error handler. PR-URL: #38018Fixes: #37291 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@lpinca
Copy link
MemberAuthor

Landed in cc4ee6c.

@lpincalpinca closed this Apr 8, 2021
@lpincalpinca deleted the fix/issue-37291 branch April 8, 2021 17:30
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

investigate flaky test/parallel/test-http-many-ended-pipelines.js

5 participants

@lpinca@nodejs-github-bot@Trott@cjihrig@targos