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: do not call callback if socket is closed #52829

Merged

Conversation

theanarkh
Copy link
Contributor

Do not call callback if socket is closed.

  • 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

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run. labels May 4, 2024
@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label May 4, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 4, 2024
Copy link
Contributor

github-actions bot commented May 4, 2024

Failed to start CI
   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/8949880553

lib/dgram.js Outdated Show resolved Hide resolved
@mcollina mcollina changed the title lib: do not call callback if socket is closed dgram: do not call callback if socket is closed May 15, 2024
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm so we can start CI

@mcollina mcollina added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels May 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 15, 2024
@theanarkh theanarkh force-pushed the dont-emit-error-after-close-handle branch from 9aeee94 to d022b6c Compare May 15, 2024 11:48
@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label May 19, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 19, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/52829
✔  Done loading data for nodejs/node/pull/52829
----------------------------------- PR info ------------------------------------
Title      dgram: do not call callback if socket is closed (#52829)
Author     theanarkh  (@theanarkh)
Branch     theanarkh:dont-emit-error-after-close-handle -> nodejs:main
Labels     dgram, needs-ci
Commits    1
 - lib: do not call callback if socket is closed
Committers 1
 - theanarkh 
PR-URL: https://github.com/nodejs/node/pull/52829
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52829
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - lib: do not call callback if socket is closed
   ℹ  This PR was created on Sat, 04 May 2024 07:58:48 GMT
   ✔  Approvals: 1
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/52829#pullrequestreview-2057177270
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-05-18T07:21:46Z: https://ci.nodejs.org/job/node-test-pull-request/59282/
- Querying data for job/node-test-pull-request/59282/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9146455101

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 19, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@theanarkh theanarkh added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 20, 2024
@aduh95 aduh95 requested a review from mcollina May 23, 2024 15:15
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label May 23, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 23, 2024
@nodejs-github-bot nodejs-github-bot merged commit f05baff into nodejs:main May 23, 2024
68 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in f05baff

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. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. 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.

None yet

4 participants