Skip to content

Conversation

@LiviaMedeiros
Copy link
Member

Before this change, the following functions had either empty string or wrappedFn as function name:

  • http.OutgoingMessage.prototype.cork
  • http.OutgoingMessage.prototype.uncork
  • http.Server.prototype.close
  • http.Server.prototype.closeAllConnections
  • http.Server.prototype.closeIdleConnections
  • http.Server.prototype[Symbol.asyncDispose]
  • http.Server.prototype[nodejs.rejection]
  • http.validateHeaderName
  • http.validateHeaderValue
  • https.Server.prototype.closeAllConnections
  • https.Server.prototype.closeIdleConnections
  • https.Server.prototype.close

Affected functions: - http.OutgoingMessage.prototype.cork - http.OutgoingMessage.prototype.uncork - http.Server.prototype.close - http.Server.prototype.closeAllConnections - http.Server.prototype.closeIdleConnections - http.Server.prototype[Symbol.asyncDispose] - http.Server.prototype[nodejs.rejection] - http.validateHeaderName - http.validateHeaderValue - https.Server.prototype.closeAllConnections - https.Server.prototype.closeIdleConnections - https.Server.prototype.close
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues or PRs related to the http subsystem. https Issues or PRs related to the https subsystem. needs-ci PRs that need a full CI run. labels May 5, 2025
@LiviaMedeiros
Copy link
MemberAuthor

Comment on lines +622 to +623
Server.prototype[EE.captureRejectionSymbol]=
assignFunctionName(EE.captureRejectionSymbol,function(err,event, ...args){
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Linter seems to be happy with this but better suggestions would be appreciated.

@codecov
Copy link

codecovbot commented May 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.15%. Comparing base (a44ccac) to head (27ff9d0).
Report is 72 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #58180 +/- ## ========================================== - Coverage 90.17% 90.15% -0.03%  ========================================== Files 630 630 Lines 186503 186759 +256 Branches 36618 36654 +36 ========================================== + Hits 168188 168365 +177 - Misses 11120 11192 +72 - Partials 7195 7202 +7 
Files with missing linesCoverage Δ
lib/_http_outgoing.js95.76% <100.00%> (+<0.01%)⬆️
lib/_http_server.js97.07% <100.00%> (+0.08%)⬆️
lib/https.js99.30% <100.00%> (ø)

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@bjohansebasbjohansebas left a comment

Choose a reason for hiding this comment

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

LGTM

@LiviaMedeirosLiviaMedeiros added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels May 12, 2025
@nodejs-github-botnodejs-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 12, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/58180 ✔ Done loading data for nodejs/node/pull/58180 ----------------------------------- PR info ------------------------------------ Title http,https: give names to anonymous or misnamed functions (#58180) Author Livia Medeiros <[email protected]> (@LiviaMedeiros) Branch LiviaMedeiros:http-name-functions -> nodejs:main Labels http, https, author ready, needs-ci Commits 1 - http,https: give names to anonymous or misnamed functions Committers 1 - LiviaMedeiros <[email protected]> PR-URL: https://github.com/nodejs/node/pull/58180 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/58180 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 05 May 2025 12:39:34 GMT ✔ Approvals: 3 ✔ - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/58180#pullrequestreview-2815427791 ✔ - Ethan Arrowood (@Ethan-Arrowood): https://github.com/nodejs/node/pull/58180#pullrequestreview-2825471155 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/58180#pullrequestreview-2831904689 ✔ Last GitHub CI successful ✘ No full Jenkins CI runs detected ℹ Last Benchmark CI on 2025-05-05T12:43:16Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1716/ - Querying data for job/node-test-pull-request/1716/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/14965261342

@LiviaMedeirosLiviaMedeiros added request-ci Add this label to start a Jenkins CI on a PR. and removed 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. labels May 12, 2025
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 12, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@richardlaurichardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label May 12, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 12, 2025
@nodejs-github-botnodejs-github-bot merged commit cfd2021 into nodejs:mainMay 12, 2025
85 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in cfd2021

targos pushed a commit that referenced this pull request May 16, 2025
Affected functions: - http.OutgoingMessage.prototype.cork - http.OutgoingMessage.prototype.uncork - http.Server.prototype.close - http.Server.prototype.closeAllConnections - http.Server.prototype.closeIdleConnections - http.Server.prototype[Symbol.asyncDispose] - http.Server.prototype[nodejs.rejection] - http.validateHeaderName - http.validateHeaderValue - https.Server.prototype.closeAllConnections - https.Server.prototype.closeIdleConnections - https.Server.prototype.close PR-URL: #58180 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]>
aduh95 pushed a commit that referenced this pull request Jun 10, 2025
Affected functions: - http.OutgoingMessage.prototype.cork - http.OutgoingMessage.prototype.uncork - http.Server.prototype.close - http.Server.prototype.closeAllConnections - http.Server.prototype.closeIdleConnections - http.Server.prototype[Symbol.asyncDispose] - http.Server.prototype[nodejs.rejection] - http.validateHeaderName - http.validateHeaderValue - https.Server.prototype.closeAllConnections - https.Server.prototype.closeIdleConnections - https.Server.prototype.close PR-URL: #58180 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues or PRs related to the http subsystem.httpsIssues or PRs related to the https subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@LiviaMedeiros@nodejs-github-bot@ShogunPanda@jasnell@Ethan-Arrowood@bjohansebas@richardlau