dart-lang / web_socket_channel

StreamChannel wrappers for WebSockets.
https://pub.dev/packages/web_socket_channel
BSD 3-Clause "New" or "Revised" License
412 stars 109 forks source link

how to catch error response on channel.sink.add? #193

Open b14cknc0d3 opened 3 years ago

b14cknc0d3 commented 3 years ago

I want to catch the error response from the server when I send data. How can I do?

///
_channel --< initState 
///

  onPressed: () {
                    String body = state.body.value;
                    command.add(body);
                    try {
                      _channel!.sink.add(body);
                    } catch (e) {
                      print(e);
                    }
itadminmynapse commented 2 years ago

I don't know why this question is asked, but if I drop a 40mb message into the sink, the sink returns without exception, but the message is never send out and when the next message hits the sink, the websocket has closed and can't reconnect. I think the websocket bugs out on the large message and is closed uncleanly. How do we debug what is going on?

farr64 commented 2 years ago

Some way (any way) to be able to check for webSocketChannel.sink.add errors would be great.

For instance, something along the lines of onDone and onError (as implemented for webSocketChannel.stream.listen) would be useful for webSocketChannel.sink.add

This is a fundamental feature: Report the success or failure of the execution of a request.

In case of failure, it's useful to report the reason:

Any specific technical reason for not having implemented error-checking for webSocketChannel.sink.add ?

Thanks.

farr64 commented 2 years ago

I don't agree with your opinion, Gustav (please click on the ". . ." below to see Gustav's comment).

On the contrary, I believe that our Google friends are savvy, wise, and very competent.

I’m sure this lack of error checking for webSocketChannel.sink.add is a simple oversight. If a Google manager with vision and authority learns about this, I’m sure Google's considerable resources will be used to implement a great solution in no time at all.

On Jun 11, 2022, at 1:52 AM, Gustav Trede @.***> wrote:

How can this naive/ignorant design exist in the first place ? :( It shows of a fundamental problem, its just pure incompetence.

— Reply to this email directly, view it on GitHub https://github.com/dart-lang/web_socket_channel/issues/193#issuecomment-1152877022, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7IGACBDXOM7CE72K6BT2DVORAUBANCNFSM5CD5MBEA. You are receiving this because you commented.

kika commented 2 years ago

I believe it's by design unfortunately. The whole module is built this way - it only works in the happy path. My code using this module connects perfectly fine to "wss://fjdlkjfldsfjkdls.sjfdlkfjsdlkfjsldfk.c$$" for example. The underlying platform implementation throws errors of course (on web I can see them in the browser console) but the module just ignores all of this. When establishing websockets connections what could possibly go wrong, right? It's 21st century, folks! Networks are 100% up and 100% reliable these days!

sportmachine commented 1 year ago

agree with the above comments, this package is designed for only ideal cases, users have to detect errors and reconnect quickly.

brianquinlan commented 1 year ago

@natebosch

I looked at the 2.4.0 implementation and here are the events that I see in response to various scenrios.

Scenario ↓ channel.sink.done (complete) channel.sink.done (error) channel.stream - onDone received channel.stream - onError received
socket disconnect before WebSocket upgrade no no web: yes, vm: no web: yes, vm: no
socket disconnect after WebSocket upgrade web: yes, vm: no no yes no
client initiated close (channel.sink.add will throw StateError, vm: channel.stream.listen will throw) yes no yes no
server initiated close yes no yes no

The code to get these values looks like this:

var sinkDoneComplete = false;
var sinkDoneOnError = false;
var streamOnData = false;
var streamOnDone = false;
var streamOnError = false;

channel.sink.done.then((_) {
  sinkDoneComplete = true;
}, onError: (_) {
  sinkDoneOnError = true;
});

channel.stream.listen((_) {
  streamOnData = true;
}, onError: (_) {
  streamOnError = true;
}, onDone: () {
  streamOnDone = true;
});

// Test the scenario

So, as of 2.4.0, the most reliable way of handling errors would be to do something like:


// This will catch errors that occur *before* the WebSocket upgrade occurs
try {
  await channel.ready;
} on SocketException catch (e) {
  // Handle the exception.
} on WebSocketChannelException catch (e) {
  // Handle the exception.
}

// After the connection is established, listening to `channel.stream` and checking the channel close code
// when the stream is done is reliable.
channel.stream.listen((data) {
  // Do something
}, onDone: () {
  // Inspect `channel.closeCode` to determine the reason for the close.
  // See https://datatracker.ietf.org/doc/html/rfc6455#section-7.4.1
});

channel.sink is a StreamSink, which means that errors are not reported in add by design.

The current API has similar semantics to the one defined by the WhatWG WebSockets standard e.g. calling send after the connection is close silently discards the data.

natebosch commented 1 year ago

It looks like the web implementation has better semantics in the few places where they differ. If we can make the VM match without too much difficulty I think we should. WDYT?

brianquinlan commented 1 year ago

It looks like the web implementation has better semantics in the few places where they differ. If we can make the VM match without too much difficulty I think we should. WDYT?

I started writing some of my unification ideas up but often change my mind.

There are two places were I disagree with the web semantics:

  1. I would say that we should never add an error to channel.stream - right now we only do so on the web and only before the WebSocket upgrade has completed. It seems more consistent to document that channel.stream will never produce errors. Instead, you discover errors using channel.ready and channel.closeCode.
  2. On the web we don't complete channel.sink.done if there is a failure before the connection's WebSocket upgrade is complete. I think that we should always complete channel.sink.done when there is an error.

So the table would look like:

Scenario ↓ channel.sink.done (complete) channel.sink.done (error) channel.stream - onDone received channel.stream - onError received
socket disconnect before WebSocket upgrade yes (update both) no yes (update VM) no (update web)
socket disconnect after WebSocket upgrade yes (update VM) no yes no
client initiated close (channel.sink.add will throw StateError, vm: channel.stream.listen will throw) yes no yes no
server initiated close yes no yes no

I like the consistency in each column.

I also think that it would be reasonable for all errors to produce an error on channel.stream but what would require that we interpret the RFC6455 close codes and decide which ones represent errors (probably all values >1000,<3000).

brianquinlan commented 1 year ago

@gustav3d, @sportmachine, @kika, @farr64, @b14cknc0d3, @itadminmynapse

We'd be happy to here your opinions on error handling and, more generally, on how channel.stream and channel.sink are managed.