Use GOAWAY reason NO_ERROR in client-initiated graceful shutdown #312
Conversation
|
Yes in rfc7540 referencing the text in section 6.8. |
|
Please ping when all your PRs are good for review so I can do everything in one pass. |
|
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. |
|
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. |
|
Great. All good then? I went over the other PRs you commented on today. Tell me if there is anything more. |
Add GOAWAY NO_ERROR testcase in rfc7540_SUITE Merged PR: ninenines#312
test/rfc7540_SUITE.erl
Outdated
| %% 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) | ||
| {ok, _, Port} = init_origin(tcp, http2, fun(Parent, Socket, _Transport) -> |
There was a problem hiding this comment.
The fun takes (Parent, ListenSocket, ClientSocket, ClientTransport)
| {ok, _, Port} = init_origin(tcp, http2, fun(Parent, Socket, _Transport) -> | |
| {ok, _, Port} = init_origin(tcp, http2, fun(Parent, _ListenSocket, Socket, _Transport) -> |
Add GOAWAY NO_ERROR testcase in rfc7540_SUITE Merged PR: ninenines#312
|
Rebased and updated the new testcase accordingly. |
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?