Fix socket transport when TransportKind.socket uses port 0 - #1846
Fix socket transport when TransportKind.socket uses port 0#1846Sebastian Ratz (sratz) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
@microsoft-github-policy-service agree company="SAP SE" |
| server.on('error', reject); | ||
| server.listen(port, '127.0.0.1', () => { | ||
| server.removeListener('error', reject); | ||
| const boundPort = (server.address() as AddressInfo).port; |
There was a problem hiding this comment.
Why is this cast necessary. The typing is:
There was a problem hiding this comment.
The cast is for server.address(), which according to
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/127fd9871abcf3ff2a66bdc92fc85c77e725a466/types/node/net.d.ts#L680-L706
returns <AddressInfo> | <string> | <null>.
null only if called before listening, string in case of a unix domain socket, and AddressInfo in the IP socket case we have here.
There was a problem hiding this comment.
But we should still not cast it without a type check of !== null && typeof ... !== 'string' in which case TS infers AddressInfo
There was a problem hiding this comment.
Updated to clearly reject() in the unexpected null | string case.
Dirk Bäumer (dbaeumer)
left a comment
Dirk Bäumer (dbaeumer)
left a comment
There was a problem hiding this comment.
See latest comment
When the client is configured with `transport: { kind:
TransportKind.socket, port: 0 }`, Node's `server.listen(0, ...)`
picks a random port. The client was pushing `--socket=0` to the
server args before the transport was created, so the server never
learned the actual bound port and was unable to connect.
Expose the actually-bound port on SocketTransport via
`port(): number`, and defer pushing `--socket=<port>` until after
`createClientSocketTransport` resolves in all three spawn paths
(runtime, fork, executable command).
2b25e26 to
652dfe6
Compare
When the client is configured with
transport: { kind: TransportKind.socket, port: 0 }, Node'sserver.listen(0, ...)picks a random port. The client was pushing--socket=0to the server args before the transport was created, so the server never learned the actual bound port and was unable to connect.Expose the actually-bound port on SocketTransport via
port(): number, and defer pushing--socket=<port>until aftercreateClientSocketTransportresolves in all three spawn paths (runtime, fork, executable command).Resolves #1845.