graceful shutdown: terminate on error

[niklasad1]

This fixes a soketto::Receiver drop issue where two streams were created and this PR uses the same stream for both receiving messages and shutdown.

In addition, this PR reverts the behavior to keep the socket alive despite errors and the server will only perform the graceful shutdown if a shutdown was requested.

master:

2023-10-19T08:46:52.456649Z  INFO jsonrpsee_server::transport::ws: server is closed because: Ok(Stopped)
// This is not a message that is sent by the client i.e, read as empty......
2023-10-19T08:46:52.456664Z TRACE soketto::connection: 5627ceca: recv: (Continue (fin 0) (rsv 000) (mask (0 0)) (len 0))
2023-10-19T08:46:52.456671Z DEBUG soketto::connection: 5627ceca: continue frame while not processing message fragments
2023-10-19T08:46:52.456678Z  WARN jsonrpsee_server::transport::ws: Graceful shutdown got WebSocket error: unexpected opcode: Continue but polling until the connection closed or all pending calls has been executed
// Websocket Close handshake .... 

This branch:

2023-10-19T08:52:19.060822Z  INFO jsonrpsee_server::transport::ws: server is closed because: Ok(Stopped)
2023-10-19T08:52:19.162134Z TRACE method_call{method="sleep_20s"}: jsonrpsee_core::tracing: send="{\"jsonrpc\":\"2.0\",\"id\":10,\"method\":\"sleep_20s\"}"
2023-10-19T08:52:19.162269Z TRACE jsonrpsee_client_transport::ws: send: {"jsonrpc":"2.0","id":10,"method":"sleep_20s"}
2023-10-19T08:52:19.162301Z TRACE soketto::connection: 2425d827: send: (Text (fin 1) (rsv 000) (mask (1 1e4e0b66)) (len 46))
2023-10-19T08:52:19.162341Z TRACE soketto::connection: 2425d827: Sender flushing connection
2023-10-19T08:52:19.162486Z TRACE soketto: read 2 bytes
2023-10-19T08:52:19.162511Z TRACE soketto: read 4 bytes
2023-10-19T08:52:19.162524Z TRACE soketto::connection: 33584b71: recv: (Text (fin 1) (rsv 000) (mask (1 1e4e0b66)) (len 46))
// Websocket Close handshake happend but no empty message is read above....

On errors we just terminate the connection for simplicity as it did before.

[jsdw]

Where was the cancel safety issue? Offhand I can't see anything problematic in the old code so I'm probably missing something!

But the code looks good anyways!

[niklasad1]

Not sure whether it's a cancel-safety issue but something weird is happening once we create a new stream on every receive operation in try_recv and finally in graceful_shutdown

master:

2023-10-19T08:46:52.456649Z  INFO jsonrpsee_server::transport::ws: server is closed because: Ok(Stopped)
// This is not a message that is sent by the client i.e, read as empty......
2023-10-19T08:46:52.456664Z TRACE soketto::connection: 5627ceca: recv: (Continue (fin 0) (rsv 000) (mask (0 0)) (len 0))
2023-10-19T08:46:52.456671Z DEBUG soketto::connection: 5627ceca: continue frame while not processing message fragments
2023-10-19T08:46:52.456678Z  WARN jsonrpsee_server::transport::ws: Graceful shutdown got WebSocket error: unexpected opcode: Continue but polling until the connection closed or all pending calls has been executed
// Websocket Close handshake .... 

This branch:

2023-10-19T08:52:19.060822Z  INFO jsonrpsee_server::transport::ws: server is closed because: Ok(Stopped)
2023-10-19T08:52:19.162134Z TRACE method_call{method="sleep_20s"}: jsonrpsee_core::tracing: send="{\"jsonrpc\":\"2.0\",\"id\":10,\"method\":\"sleep_20s\"}"
2023-10-19T08:52:19.162269Z TRACE jsonrpsee_client_transport::ws: send: {"jsonrpc":"2.0","id":10,"method":"sleep_20s"}
2023-10-19T08:52:19.162301Z TRACE soketto::connection: 2425d827: send: (Text (fin 1) (rsv 000) (mask (1 1e4e0b66)) (len 46))
2023-10-19T08:52:19.162341Z TRACE soketto::connection: 2425d827: Sender flushing connection
2023-10-19T08:52:19.162486Z TRACE soketto: read 2 bytes
2023-10-19T08:52:19.162511Z TRACE soketto: read 4 bytes
2023-10-19T08:52:19.162524Z TRACE soketto::connection: 33584b71: recv: (Text (fin 1) (rsv 000) (mask (1 1e4e0b66)) (len 46))
// Websocket Close handshake happend but no empty message is read above....

If you search the logs you can find that an empty WebSocket message is read by soketto which is a red-flag i.e,
recv: (Continue (fin 0) (rsv 000) (mask (0 0)) (len 0))

[jsdw]

Aah I can see what might have been an issue now yeah; receiver.receive() isn't cancel safe, but we create a stream over it in try_recv and then can there are a few cases that can lead to the receiver.receive() future being dropped midway through, and then later in eg graceful_shutdown we start receiving again but maybe from a weird/broken state because some of the data has lready been puleld into the receiver.receive() future which was dropped.

So yeah keeping the same stream for everything makes a lot of sense, so that we aren't throwing away any data as long as the stream is reused!