[endpoint] Return a promise that resolves/reject on listen(). #296
Conversation
This commit makes sure that the returned promise from endpoint.listen() resolves whenever the server is successfully listening on the provided port. Otherwise reject with the first error.
src/endpoint/endpoint_impl.ts
Outdated
| }); | ||
| server.listen(actualPort, () => { | ||
| if (!failed) { | ||
| resolve(); |
There was a problem hiding this comment.
How about resolving this promise with at least the current port? This is especially useful if you start listening on 0
There was a problem hiding this comment.
okay, good idea!
There was a problem hiding this comment.
just out of curiosity we don't want to give the user access to the server, basically resolving the promise with the server.
There was a problem hiding this comment.
we can, but it seems like leaking a concrete implementation, will be hard to use anything else if we do this.
There was a problem hiding this comment.
@nikrooz would you find it more useful to have access to the h2 server?
There was a problem hiding this comment.
Thanks everyone for the suggestions, I'll merge this meanwhile. We can iterate with a progressively more advanced options later.
There was a problem hiding this comment.
Not necessarily, I just mentioned it in case we want to be aligned with nodes api.
https://nodejs.org/api/net.html#serverlistenport-host-backlog-callback
src/endpoint/endpoint_impl.ts
Outdated
| }); | ||
| server.listen(actualPort, () => { | ||
| if (!failed) { | ||
| resolve(); |
There was a problem hiding this comment.
just out of curiosity we don't want to give the user access to the server, basically resolving the promise with the server.
This PR makes sure that the returned promise from endpoint.listen() resolves whenever the server is successfully listening on the provided port. Otherwise reject with the first error.