Skip to content

dgram: don't hide implicit bind errors - #31958

Merged
cjihrig merged 2 commits into
nodejs:masterfrom
cjihrig:dgram
Feb 28, 2020
Merged

cjihrig merged 2 commits into
nodejs:masterfrom
cjihrig:dgram

Conversation

@cjihrig

@cjihrig cjihrig commented Feb 26, 2020

Copy link
Copy Markdown
Contributor

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 UDP and the dgram module. errors Issues and PRs related to JavaScript errors originating in Node.js core. labels Feb 26, 2020
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

nodejs-github-bot commented Feb 28, 2020 •

Copy link
Copy Markdown
Collaborator

@cjihrig cjihrig added the author ready PRs with CI started, the required approvals, and no outstanding review comments. 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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This error is no longer used within core. This commit removes it.

PR-URL: nodejs#31958
Refs: nodejs/help#2484
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@cjihrig
cjihrig merged commit 331d636 into nodejs:master Feb 28, 2020
@cjihrig

cjihrig commented Feb 28, 2020

Copy link
Copy Markdown
Contributor Author

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 with CI started, the required approvals, and no outstanding review comments. dgram Issues and PRs related to UDP and the dgram module. errors Issues and PRs related to JavaScript errors originating 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