Open zuiderkwast opened 1 year ago
Both shutdown
and {shutdown, _}
should result in a NO_ERROR.
Protocol:closing/4
is only called from gun:closing/2
which is internal. gun:closing/2
is only ever called with 'shutdown' and 'owner_down' from within the same module. Thus, I believe handling of any other reason is dead code.
This 'closing' reason is not the same as the exit reason used elsewhere, for example in disconnect/2.
Yes as far as the code is currently implemented only shutdown
needs to be handled. But in the future it would be better to handle sys:terminate
or EXIT
signals properly and in that case both shutdown
and the tuple should result in a NO_ERROR. But for now a smaller patch is fine.
To handle supervisor shutting down gun connections, EXIT and sys:terminate, we would need to use trap exit. I think it would be good to have, but it's a larger change. I made a PR with only a minimal fix.
Hi Loic,
when
gun:shutdown/1
is called on HTTP/2, it causes a GOAWAY frame with reason INTERNAL_ERROR to be sent to the server. The expected message is GOAWAY with reason NO_ERROR.I though I had implemented the shutdown handling in Gun, but I see now that you did it in #206. (I did it in cowboy.)
Anyway, I traced it as follows:
gun:shutdown/1
→closing(status(State, shutdown), shutdown)
→Protocol:closing(Reason, _, _, _)
, sogun_http2:closing/4
is called with reason 'shutdown', but only reasons 'normal' and 'owner_down' cause a GOAWAY frame with reason NO_ERROR. Any other reason (including shutdown) causes a GOAWAY frame with reason INTERNAL_ERROR.The simple fix would be to add
shutdown -> no_error
here.In
gun_ws
there is specific handling for reason 'shutdown' which suggests this is the right way to fix it.gun_ws:closing(Reason, State, EvHandler, EvHandlerState) ...
```Erlang closing(Reason, State, EvHandler, EvHandlerState) -> Code = case Reason of normal -> 1000; owner_down -> 1001; shutdown -> 1001; {error, badframe} -> 1002; {error, badencoding} -> 1007 end, ```I checked for tests covering this, but the gun initiated shutdown tests in
shutdown_SUITE
all use cowboy as the server, so the messages are not checked byte-by-byte.