Skip to content

Conversation

nektro
Copy link
Contributor

@nektro nektro commented Aug 5, 2025

before:

❯ node test/parallel/test-http-server-stale-close.js
node:internal/errors:540
      throw error;
      ^

TypeError [ERR_INVALID_ARG_TYPE]: The "code" argument must be of type number. Received an instance of IncomingMessage
    at process.set [as exitCode] (node:internal/bootstrap/node:119:9)
    at ClientRequest.exit (node:internal/process/per_thread:231:24)
    at Object.onceWrapper (node:events:622:26)
    at ClientRequest.emit (node:events:507:28)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (node:_http_client:716:27)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:117:17)
    at Socket.socketOnData (node:_http_client:558:22)
    at Socket.emit (node:events:507:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Node.js v24.3.0

❯ echo $?
0

after:

❯ node test/parallel/test-http-server-stale-close.js

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 5, 2025
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.86%. Comparing base (dfee0b1) to head (7944416).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59357      +/-   ##
==========================================
- Coverage   89.89%   89.86%   -0.03%     
==========================================
  Files         656      656              
  Lines      193141   193141              
  Branches    37886    37888       +2     
==========================================
- Hits       173623   173568      -55     
- Misses      12051    12099      +48     
- Partials     7467     7474       +7     

see 40 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.

@islandryu
Copy link
Member

@nektro
Could you please rebase the main branch?
I think it will fix the Windows error.

@islandryu islandryu self-requested a review August 9, 2025 03:04
@islandryu islandryu added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 9, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 9, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@gurgunday gurgunday added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 10, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@islandryu islandryu added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 12, 2025
@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 Aug 12, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/59357
✔  Done loading data for nodejs/node/pull/59357
----------------------------------- PR info ------------------------------------
Title      test: fix hidden error in test-http-server-stale-close.js (#59357)
Author     Meghan Denny <[email protected]> (@nektro)
Branch     nektro:patch-9 -> nodejs:main
Labels     test, author ready, needs-ci, commit-queue-squash
Commits    2
 - test: fix hidden error in test-http-server-stale-close.js
 - Merge branch 'main' into patch-9
Committers 1
 - GitHub <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/59357
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ryuhei Shima <[email protected]>
Reviewed-By: Stefan Stojanovic <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/59357
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ryuhei Shima <[email protected]>
Reviewed-By: Stefan Stojanovic <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 05 Aug 2025 04:19:27 GMT
   ✔  Approvals: 3
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/59357#pullrequestreview-3089680106
   ✔  - Ryuhei Shima (@islandryu): https://github.com/nodejs/node/pull/59357#pullrequestreview-3102466424
   ✔  - Stefan Stojanovic (@StefanStojanovic): https://github.com/nodejs/node/pull/59357#pullrequestreview-3104630403
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-08-11T05:37:20Z: https://ci.nodejs.org/job/node-test-pull-request/68552/
- Querying data for job/node-test-pull-request/68552/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 59357
From https://github.com/nodejs/node
 * branch                  refs/pull/59357/merge -> FETCH_HEAD
✔  Fetched commits as fc3f19ef9324..24465b0ec1f8
--------------------------------------------------------------------------------
error: commit 24465b0ec1f8fafa4bd4b72b0335bed602e5203e is a merge but no -m option was given.
fatal: cherry-pick failed
[main 02dc17085b] test: fix hidden error in test-http-server-stale-close.js
 Author: Meghan Denny <[email protected]>
 Date: Mon Aug 4 21:17:59 2025 -0700
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✘  Failed to apply patches
https://github.com/nodejs/node/actions/runs/16895898564

@islandryu
Copy link
Member

@nektro
Sorry, you might have done a merge pull.
Could you remove the merge commit and pull main using rebase instead?
https://github.com/islandryu/node/blob/59112/doc/contributing/pull-requests.md#step-9-discuss-and-update

@aduh95
Copy link
Contributor

aduh95 commented Aug 27, 2025

Could we update the test to make it fail if the forked child process exits with non-zero code?

  const server = http.createServer(common.mustCall((req, res) => {
    res.writeHead(200, { 'Content-Length': '42' });
    req.pipe(res);
    assert.strictEqual(req.destroyed, false);
    req.on('close', common.mustCall(() => {
      assert.strictEqual(req.destroyed, true);
      server.close();
      res.end();
    }));
  }));
  server.listen(0, common.mustCall(function () {
    const cp = fork(__filename, {
      stdio: 'inherit',
      env: { ...process.env, NODE_TEST_FORK_PORT: this.address().port }
    });
    cp.once('exit', common.mustCall((code) => {
      assert.strictEqual(code, 0);
    }))
  }));

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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants