search

ninenines / gun

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

Use GOAWAY reason NO_ERROR in client-initiated graceful shutdown #312

Open zuiderkwast opened 1 year ago

zuiderkwast commented 1 year ago

Fixes #311.

Do you want me to verify the goaway reason in a test case? If yes, do you prefer to put it in rfc7540_SUITE or in shutdown_SUITE?

essen commented 9 months ago

Yes in rfc7540 referencing the text in section 6.8.

essen commented 9 months ago

Please ping when all your PRs are good for review so I can do everything in one pass.

zuiderkwast commented 9 months ago

Sure, ping @essen. This one is good for review now, as are the other ones.

I struggled to find a good quote for a client's graceful shutdown. There isn't an obvious sentence to quote like there is for a server: "A server that is attempting to gracefully shut down a connection SHOULD send an initial GOAWAY frame (...)".

I picked this one: "(...) an endpoint that sends GOAWAY with NO_ERROR during graceful shutdown (...)" RFC7540 6.8

Another option is from section 7, the list of error codes: "NO_ERROR (0x0): The associated condition is not a result of an error. For example, a GOAWAY might include this code to indicate graceful shutdown of a connection." (RFC7540 7) I added this one in a comment.

essen commented 9 months ago

The "doc" string is not actually a quote but a rule inferred by sections of the RFC, and sometimes by the implementation choices (such as SHOULD becoming MUST). But don't worry I can update that when merging. I'll wait to do a single merge pass of all the PRs.

zuiderkwast commented 9 months ago

Great. All good then? I went over the other PRs you commented on today. Tell me if there is anything more.

bjosv commented 3 months ago

Rebased and updated the new testcase accordingly.