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

Http2Session connection failure semantics #16645

Closed
murgatroid99 opened this issue Oct 31, 2017 · 7 comments
Closed

Http2Session connection failure semantics #16645

murgatroid99 opened this issue Oct 31, 2017 · 7 comments
Labels
http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers.

Comments

@murgatroid99
Copy link
Contributor

If a Http2Session created by http2.connect fails to connect to the remote end, is there a single event that is guaranteed to be emitted, whether the failure is a TCP connection failure or a TLS or HTTP2 handshake failure, or something else?

@mscdex mscdex added the http2 Issues or PRs related to the http2 subsystem. label Oct 31, 2017
@apapirovski
Copy link
Member

Not 100% certain but you could try socketError on the session?

@murgatroid99
Copy link
Contributor Author

The docs say that "socketError" is specifically used to forward errors from the underlying TCP/TLS socket. I wouldn't expect that to catch HTTP2 protocol errors that hopefully would still result in a failure to connect.

@apapirovski
Copy link
Member

apapirovski commented Nov 1, 2017

Right, missed that part of the question. I'm not sure if it makes sense on our end to have a single event for both — there's a pretty substantial difference between a TLS failure (or some other socket issue) & an http2-related error. But I'll ping @jasnell & @mcollina — maybe they have some thoughts.

@murgatroid99
Copy link
Contributor Author

From my point of view, if I'm trying to connect to an endpoint, what I care about is being notified that the client has successfully connected (the "connect" event) or that it has failed to connect (the event I'm looking for). For that, the specific reason for the failure is secondary.

I was hoping that the "closed" event would be what I'm looking for, even if there are other situations where it could also be emitted. But the documentation isn't entirely clear about whether that event gets emitted at all before the connection is established.

@apapirovski
Copy link
Member

I'm reasonably certain, after reviewing the code again, that you could listen to connect and close. If connect fires then remove the close handler (or just have some variable that lets you know that you already processed the connect event).

@grantila
Copy link

grantila commented Nov 2, 2017

The docs on events sure need more work, primarily what argument data is sent in the events (and what that data means) as I described in nodejs/help#877 as well as the flow of these events; when, if and under what circumstance they occur.

@apapirovski
Copy link
Member

I don't even think http is documented particularly well in this regard, hence modules like on-finished that everyone ends up using. PRs to improve documentation (in any way, even the tiniest changes) are always greatly appreciated.

@apapirovski apapirovski added the question Issues that look for answers. label Nov 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

4 participants