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

Revert the change of network interfaces family from String to Integer #43014

Closed
Apollon77 opened this issue May 9, 2022 · 6 comments
Closed
Labels
dns Issues and PRs related to the dns subsystem. tsc-agenda Issues and PRs to discuss during the meetings of the TSC.

Comments

@Apollon77
Copy link

Version

18.x

Platform

any

Subsystem

dns?

What steps will reproduce the bug?

see #41431 (review)

How often does it reproduce? Is there a required condition?

always

What is the expected behavior?

The family is backward compatible a string and not an integer thatbreaks many modules

What do you see instead?

family is an integer breaking many libraries

Additional information

Please see the comments in the linked PR and consider a "fix" fo this breaking change

@Apollon77 Apollon77 changed the title Revert the change of network interfaces family from Striung to Integer Revert the change of network interfaces family from String to Integer May 9, 2022
@joe-hilling
Copy link

Seeing a lot a pain coming out of this breaking change.

@richardlau
Copy link
Member

cc @nodejs/tsc I think I'd lean towards reverting #41431, even though reverting the change would itself be breaking. Or at least restoring the family attribute on network interface objects to a string.

@Trott Trott added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label May 9, 2022
@Trott
Copy link
Member

Trott commented May 9, 2022

cc @nodejs/tsc I think I'd lean towards reverting #41431, even though reverting the change would itself be breaking. Or at least restoring the family attribute on network interface objects to a string.

I tentatively agree.

@aduh95 @mcollina

@BethGriggs
Copy link
Member

Agree, this change has been more disruptive than I think we'd have liked. I'd be +1 on a revert.

I have seen that some of the ecosystem has already spent time investigating and adjusting to this change. Would a full revert cause additional disruption at this point? I'm curious if there's a way to adequately mitigate the impact of the change without causing further disruption. Would the suggestions in #41431 (comment) be sufficient?

@Apollon77
Copy link
Author

I assume (at least from the changes I have "seen" and because of needed nodejs backward compatibility) most simply added it as alternative to support both variants ... So a revert should (!) for most just lead to a "dead code OR clause"

@VoltrexKeyva VoltrexKeyva added the dns Issues and PRs related to the dns subsystem. label May 9, 2022
aduh95 added a commit to aduh95/node that referenced this issue May 11, 2022
aduh95 added a commit to aduh95/node that referenced this issue May 11, 2022
aduh95 added a commit to aduh95/node that referenced this issue May 11, 2022
aduh95 added a commit to aduh95/node that referenced this issue May 11, 2022
aduh95 added a commit to aduh95/node that referenced this issue May 21, 2022
aduh95 added a commit to aduh95/node that referenced this issue May 21, 2022
@Apollon77
Copy link
Author

Inthe TSC meeting (at least as I understand the minutes) it was decided to revert the "integrater family back to Strings" ... should be in Node.js 18.2. Thank you very much for this!

aduh95 added a commit to aduh95/node that referenced this issue Jun 5, 2022
aduh95 added a commit to aduh95/node that referenced this issue Jun 5, 2022
danielleadams pushed a commit that referenced this issue Jun 16, 2022
Refs: #43014

PR-URL: #43054
Fixes: #43014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
xnuk added a commit to xnuk/souffle that referenced this issue Jul 5, 2022
AdamMajer added a commit to AdamMajer/node that referenced this issue Aug 1, 2022
As a consequence of nodejs#43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: nodejs#44003
AdamMajer added a commit to AdamMajer/node that referenced this issue Aug 1, 2022
As a consequence of nodejs#43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: nodejs#44003
nodejs-github-bot pushed a commit that referenced this issue Aug 3, 2022
As a consequence of #43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: #44003
PR-URL: #44083
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Aug 16, 2022
As a consequence of #43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: #44003
PR-URL: #44083
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
ruyadorno pushed a commit that referenced this issue Aug 23, 2022
As a consequence of #43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: #44003
PR-URL: #44083
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
As a consequence of nodejs#43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: nodejs#44003
PR-URL: nodejs#44083
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
juanarbol pushed a commit that referenced this issue Oct 3, 2022
As a consequence of #43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: #44003
PR-URL: #44083
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
juanarbol pushed a commit that referenced this issue Oct 4, 2022
As a consequence of #43014 ,
server sockets and others, once connected, report string family
names. But when feeding these to Socket.connect(), it passes
these to host resolution with a string for family while a numeric
family is expected internally. This results in wrong hints flags
to be set and resolution to fail.

As solution, is to add ability to handle both numeric and string
family names when doing lookup and connect.

Fixes: #44003
PR-URL: #44083
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dns Issues and PRs related to the dns subsystem. tsc-agenda Issues and PRs to discuss during the meetings of the TSC.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants