You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
net.connect: behaviour when no arguments are passed #33930
net.connect() - no args does ECONNREFUSED but should be invalid usage
I think I commented on this, and fixed it in backports, and it was thought
that net.connect() should be same as net.connect({port:undefined}), but I
don't agree, it never makes sense to connect when you don't say what you are
connecting to, it has no use case. also strange that undefined works like no
args were passed, but null is like {port: null} was passed. This all
seems ugly and messy. Check the tests.
For the quote above outlined in #33715 (comment), two solutions come to mind.
Throw ERR_MISSING_ARGS when no arguments are passed (as suggested above -- invalid usage).
To fix the difference in behavior between port: undefined and port: null, either make port a mandatory field that throws ERR_INVALID_ARGS for anything other than string or numeric values or don't throw ERR_INVALID_ARGS for port: null. This needs more brainstorming.
I'd appreciate a few pointers here to see where this should be going.
IMO it should throw ERR_MISSING_ARGS if possible as none of those arguments are defined as optional in our docs and we should follow that and don't try to deduce what was meant by no-arg call. Furthermore, it makes it confusing when user upon calling net.connect() gets error that port value is not correct when the user didn't even provide that value in any way.
I'm currently looking into this. Tried both of the suggestions, but they do not seem to be a clean solution. In my opinion, the problem is that the function normalizeArgs is used both for listen and connect functions. While for listen it makes sense to bind to a random port, there is no use case for connecting to a random port.
Notice that the problem also relates to net.Socket.connect().
My conclusion is that sorting this out means a huge change to the Net module.
Activity
lundibundi commentedon Jun 17, 2020
IMO it should throw
ERR_MISSING_ARGSif possible as none of those arguments are defined as optional in our docs and we should follow that and don't try to deduce what was meant by no-arg call. Furthermore, it makes it confusing when user upon callingnet.connect()gets error thatportvalue is not correct when the user didn't even provide that value in any way.ping @nodejs/net
mkrawczuk commentedon Jun 22, 2020
I'm currently looking into this. Tried both of the suggestions, but they do not seem to be a clean solution. In my opinion, the problem is that the function
normalizeArgsis used both forlistenandconnectfunctions. While forlistenit makes sense to bind to a random port, there is no use case for connecting to a random port.Notice that the problem also relates to
net.Socket.connect().My conclusion is that sorting this out means a huge change to the Net module.net: check args in net.connect() and socket.connect() calls
lib: handle one of args case in ERR_MISSING_ARGS