ninenines / gun

HTTP/1.1, HTTP/2, Websocket client (and more) for Erlang/OTP.
ISC License
891 stars 232 forks source link

Exposing HTTP2 "additional debug data" in received GOAWAY frames #308

Open rnowak opened 1 year ago

rnowak commented 1 year ago

Being able to act on the "additional debug data" that can be part of the GOAWAY frames is necessary for correct usage of certain services.

One such service is Apple Push Notification service (APNs). When connecting to APNs, if the client certificate is bad or for the wrong environment (prod vs sandbox), the service will send a GOAWAY frame with some JSON stating the error reason, and then close the socket immediately after. In both of these cases, it is meaningless to attempt connecting again as the situation is unlikely to resolve itself and needs intervention.

In the APNs documentation, it is stated "The JSON data might also be included in a GOAWAY frame when a connection is terminated."

When this happens, gun sends a gun_down message with reason=normal:

{gun_down,<0.1386.0>,http2,normal,[]}

In the conversation in #cowboy on the Erlang Slack on the 5th of December, Loïc wrote:

Perhaps the down function could be modified to change the Reason and add GOAWAY data when that happens.
But I wonder what happens when it's tunnel that goes down right now instead

I know some people do APNS through tunnels

---

In theory it results in a gun_error for the stream but I'm not totally sure that it's handled properly

In that case the data can be passed around through the Reason field of both gun_error and gun_down.
I do not think this is a breaking change so it can wait after 2.0 is out.

---

I'm thinking of exposing it only when the gun_down has normal as a reason, there are no streams at that time 
(otherwise the streams already receive it) and there was no streams opened to begin with. And only changing if 
there was debug data. Basically handling the case where a connection was accepted and terminated apparently 
normally but with a sneaky error. Specifically to support this APNS scenario. So I do not think an option is necessary
rnowak commented 1 year ago

The third value in the goaway tuple, the error code, would also be great to have access to. This is already translated in cow_http2 by parse_error_code/1. I've so far seen 0 / no_error and 11 / enhance_your_calm emitted in the wild.

essen commented 1 year ago

If the error code is not no_error (equivalent to "normal") it probably should be reflected yes.

manas-chaudhari commented 5 months ago

+1 for this requirement. I am also willing to take a stab at implementing if I can get some guidance on the approach.

essen commented 5 months ago

The close reason just has to be propagated from gun_http2 to gun. Currently gun_http2 returns the close command, we would need a new command {close, Reason, Extra} that can then be given to the gun_down message. Note that if the Reason is no_error, we should handle it by using normal like currently and not propagate the extra data.

There is additional difficulty because:

The last item has the caveat that servers may not send the extra data multiple times, but we'll cross that bridge when we get to it. For now we should just leave a comment in the code indicating that because we keep the last GOAWAY information only, that data may be lost if the server changes what it sends in subsequent GOAWAY frames.