FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

dgram: don't swallow bind errors when callback is provided by armanmikoyan · Pull Request #62602 · nodejs/node · GitHub

/ node Public

dgram: don't swallow bind errors when callback is provided - #62602

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
armanmikoyan:fix/dgram-bind-silent-error
Aug 23, 2026
Merged

dgram: don't swallow bind errors when callback is provided#62602
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
armanmikoyan:fix/dgram-bind-silent-error

Conversation

armanmikoyan commented Apr 5, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

dgram: don't swallow bind errors when callback is provided

When Socket.prototype.bind() is called with a callback, the internal
cleanup listener is registered on 'error', which counts as an error
handler and silently swallows bind errors (e.g. EADDRINUSE) when no
user error handler is attached.

This switches to EventEmitter.errorMonitor for the cleanup listener,
matching the pattern already used in the enqueue() function
(lib/dgram.js:571). This ensures bind errors properly propagate as
unhandled errors while still performing listener cleanup.

Setup — a socket already bound to port 5000

import dgram from 'dgram';

const receiver = dgram.createSocket('udp4');

receiver.bind({ port: 5000, address: '127.0.0.1' });

No callback, no listening event — error surfaces (correct)

import dgram from 'dgram';

const socket = dgram.createSocket('udp4');

socket.bind({ port: 5000, address: '127.0.0.1' }); 
// Throws: Error: bind EADDRINUSE 127.0.0.1:5000

Using listening event — error surfaces (correct)

import dgram from 'dgram';

const socket = dgram.createSocket('udp4');

socket.on('listening', () => {
  console.log('bound');
});

socket.bind({ port: 5000, address: '127.0.0.1' });
// Throws: Error: bind EADDRINUSE 127.0.0.1:5000

Using callback — error silently swallowed (bug)

import dgram from 'dgram';

const socket = dgram.createSocket('udp4');

socket.bind(
  { port: 5000, address: '127.0.0.1' },
  () => { console.log('bound'); },
);
// No error, no output — process hangs

All three examples do the same thing — bind to a port already in use.
The first two properly throw, but the third silently swallows the error
just because a callback was passed to bind().

After this fix, all three cases properly surface the error.

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run. labels Apr 5, 2026
armanmikoyan force-pushed the fix/dgram-bind-silent-error branch 2 times, most recently from 60c4c3a to 6624c6f Compare April 5, 2026 17:12
Signed-off-by: armanmikoyan <arman.mikoyan1@gmail.com>
armanmikoyan force-pushed the fix/dgram-bind-silent-error branch from 6624c6f to 4b5f804 Compare April 5, 2026 17:15

Ethan-Arrowood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This seems appropriate to me. Even if a fix, this is a notable change in behavior. I believe notable-change is warranted at least. I'm not sure about semver. I don't want to tag all of the TSC, but I'll see if http/net folks have any input as the dgram subsystem is relatively close to those.

Copy link
Copy Markdown
Contributor

cc @nodejs/net @nodejs/http

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

mcollina added semver-major PRs that contain breaking changes and should be released in the next major version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Jul 15, 2026

Copy link
Copy Markdown
Member

No, I think this is just a bug fix

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2026
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Contributor Author

@mcollina I can't access the CI logs. Could someone let me know if the failures are related to my changes or just flaky? Happy to fix them if needed, or a re-run would be appreciated if they're unrelated.

Ethan-Arrowood added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 23, 2026

Copy link
Copy Markdown
Contributor

CI failure looks unrelated to me; its a sqlite-backup flake and some rebase infra issue.

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 23, 2026

This comment was marked as outdated.

This comment was marked as outdated.

trivikr added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 22, 2026

Copy link
Copy Markdown
Collaborator

trivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 23, 2026
nodejs-github-bot merged commit 97c7e32 into nodejs:main Aug 23, 2026
53 checks passed

Copy link
Copy Markdown
Collaborator

Landed in 97c7e32

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 23, 2026
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
Signed-off-by: armanmikoyan <arman.mikoyan1@gmail.com>
PR-URL: #62602
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
Signed-off-by: armanmikoyan <arman.mikoyan1@gmail.com>
PR-URL: #62602
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, and a CI started. dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL