Skip to content

net: remoteAddress always undefined called before connected - #43011

Closed
Y1D7NG wants to merge 1 commit into
nodejs:masterfrom
Y1D7NG:fix-net
Closed

net: remoteAddress always undefined called before connected#43011
Y1D7NG wants to merge 1 commit into
nodejs:masterfrom
Y1D7NG:fix-net

Conversation

@Y1D7NG

@Y1D7NGY1D7NG commented May 8, 2022

Copy link
Copy Markdown
Contributor

fix#43009

  • when connecting and get remote address, family and port, return this._peername || {} directly.
  • fix remoteFamily is IPvundefined when it doesn't exis, return undefined if it does not exist.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels May 8, 2022

@ShogunPandaShogunPanda 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.

LGTM!

@ShogunPanda

Copy link
Copy Markdown
Contributor

Can you link that PR so I can take a look tomorrow?

@mcollinamcollina 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.

lgtm

@Y1D7NG

Copy link
Copy Markdown
ContributorAuthor

can this be merged?

@ShogunPanda

Copy link
Copy Markdown
Contributor

At least two approvals and 48 hours passed. I think we're good to go!

@ShogunPandaShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollinamcollina 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.

lgtm

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ShogunPanda

Copy link
Copy Markdown
Contributor

Landed in b88045f

ShogunPanda pushed a commit that referenced this pull request May 19, 2022
PR-URL: #43011
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Y1D7NG
Y1D7NG deleted the fix-net branch May 19, 2022 14:32
bengl pushed a commit that referenced this pull request May 30, 2022
PR-URL: #43011
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@juanarbol

Copy link
Copy Markdown
Member

This seems to in break v16.x:

make[1]: Leavingdirectory'/home/juanarbol/GitHub/node/test/node-api/test_worker_terminate/build'/usr/bin/python3.10tools/test.py--mode=release \ \ --skip-tests= \ default \ addonsjs-native-apinode-api===releasetest-net-remote-address-port===Path: parallel/test-net-remote-address-port
node:assert:399throwerr;^AssertionError[ERR_ASSERTION]: Theexpressionevaluatedtoafalsy value:
assert.ok(remoteFamilyCandidates.includes(socket.remoteFamily))atServer.<anonymous>(/home/juanarbol/GitHub/node/test/parallel/test-net-remote-address-port.js:38:10)atServer.<anonymous>(/home/juanarbol/GitHub/node/test/common/index.js:417:15)atServer.emit(node:events:527:28)atTCP.onconnection(node:net:1634:8){generatedMessage: true,code: 'ERR_ASSERTION',actual: false,expected: true,operator: '=='}

not really sure why, I will label this one as backport-requested in the meantime

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.netIssues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

socket.remoteAddress is always undefined once called before connect

8 participants

@Y1D7NG@nodejs-github-bot@ShogunPanda@juanarbol@mcollina@lpinca@himself65@targos