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

dgram: don't hide implicit bind errors #31958

Merged
merged 2 commits into from
Feb 28, 2020
Merged

dgram: don't hide implicit bind errors #31958

merged 2 commits into from
Feb 28, 2020

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Feb 26, 2020

When dgram socket implicit binding fails, an attempt is made to
clean up the send queue. This was originally implemented using
an 'error' handler that performed cleanup and then emitted a
fake error, which concealed the original error. This was done
to prevent cases where the same error was emitted twice. Now
that the errorMonitor event is available, use that to perform
the cleanup without impacting the actual error handling.

The second commit removes the ERR_SOCKET_CANNOT_SEND error, which is now unused.

Refs: nodejs/help#2484
cc: @bnoordhuis

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@cjihrig cjihrig added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 26, 2020
@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Feb 26, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 28, 2020

@cjihrig cjihrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 28, 2020
When dgram socket implicit binding fails, an attempt is made to
clean up the send queue. This was originally implemented using
an 'error' handler that performed cleanup and then emitted a
fake error, which concealed the original error. This was done
to prevent cases where the same error was emitted twice. Now
that the errorMonitor event is available, use that to perform
the cleanup without impacting the actual error handling.

PR-URL: nodejs#31958
Refs: nodejs/help#2484
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This error is no longer used within core. This commit removes it.

PR-URL: nodejs#31958
Refs: nodejs/help#2484
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig merged commit 331d636 into nodejs:master Feb 28, 2020
@cjihrig
Copy link
Contributor Author

cjihrig commented Feb 28, 2020

Landed in fb26b13...331d636.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dgram Issues and PRs related to the dgram subsystem / UDP. errors Issues and PRs related to JavaScript errors originated in Node.js core. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants