Skip to content

Conversation

@Y1D7NG
Copy link
Contributor

@Y1D7NGY1D7NG commented May 8, 2022

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
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
Copy link
Contributor

@ShogunPandaShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@ShogunPanda
Copy link
Contributor

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

Copy link
Member

@mcollinamcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Y1D7NG
Copy link
ContributorAuthor

can this be merged?

@ShogunPanda
Copy link
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
Collaborator

Copy link
Member

@mcollinamcollina left a comment

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
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@ShogunPanda
Copy link
Contributor

Landed in b88045f

@juanarbol
Copy link
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