Expose error status codes

[pimterry]

• I've searched for any related issues and avoided creating a duplicate issue.

Description

When ws throws errors due to received content, it associates a websocket status code with them, which specifies the status code that will be sent when closing the connection.

It attaches this to the error behind a symbol called kStatusCode (used here, defined here), keeping that status code as internal private state that can't be easily or reliably accessed by application code.

That is inconvenient, because in some cases this is useful information for applications too, e.g. to tracking metrics of websocket failure reasons, or to implement various advanced error handling scenarios that depend on type of protocol failure involved.

Would you be open to making this a non-symbol property, so it's available as a reliable part of the API for error handling?

[lpinca]

I'm not very happy with it because it is an implementation detail and this is the reason why it is "hidden" behind a symbol. Not all errors have it. That info is already available in the 'close' event which always follows the 'error' event.

[lpinca]

which specifies the status code that will be sent when closing the connection.

It is not sent, it is simply used as the value of the code argument of the 'close' event. When an error occurs the connection is forcibly destroyed. No close frame is sent.

[pimterry]

It is not sent, it is simply used as the value of the code argument of the 'close' event.

Oh! That's very surprising behaviour, especially that it fires a close event without a close frame in that case. Why isn't this sent?

The websockets spec doesn't strictly require you to always send that close frame, but it is a 'SHOULD' if the connection is established before you hit the error, so it's supposed to be sent unless there's a specific reason not to:

If The WebSocket Connection is Established prior to the point where the endpoint is required to Fail the WebSocket Connection, the endpoint SHOULD send a Close frame with an appropriate status code (Section 7.4) before proceeding to Close the WebSocket Connection.

(from https://datatracker.ietf.org/doc/html/rfc6455#section-7.1.7)

---

I'm not very happy with it because it is an implementation detail and this is the reason why it is "hidden" behind a symbol.

Ok, fair enough. If you're not happy exposing the close status codes, would you be open to creating some other kind of recognizable error codes, or using separate error classes, to allow applications to recognize different types of websocket error?

Right now I'm looking at matching various error messages using regular expressions, which is super fragile and potentially open to abuse if external clients can inject any content into error messages anywhere. It would be great to have a better option!

[lpinca]

Oh! That's very surprising behaviour, especially that it fires a close event without a close frame in that case. Why isn't this sent?

The websockets spec doesn't strictly require you to always send that close frame, but it is a 'SHOULD' if the connection is established before you hit the error, so it's supposed to be sent unless there's a specific reason not to

It is to allow the connection to be closed as soon as possible when an error occurs. There might be buffered data to be sent before the close frame so we should wait for the socket.write() callback. What happens if data is not flowing and the callback is never called? We could use a timer (this is what ws.close() does) but it is hard to find a "delay" that works in all cases and it seems overkill when the spec uses the "SHOULD" key word. I mean there is a reason why that recommendation is not followed.

---

Ok, fair enough. If you're not happy exposing the close status codes, would you be open to creating some other kind of recognizable error codes, or using separate error classes, to allow applications to recognize different types of websocket error?

Right now I'm looking at matching various error messages using regular expressions, which is super fragile and potentially open to abuse if external clients can inject any content into error messages anywhere. It would be great to have a better option!

Can't you already do that using the status code from the 'close' event? I think applications should not be concerned about the specific protocol error (reserved bit set, invalid opcode, etc.) as long as they can identify a protocol error (1002 status code).

[lpinca]

For the spec recommendation we could use a best effort approach and do something like this:

diff --git a/lib/websocket.js b/lib/websocket.js
index 83b471d..040c41a 100644
--- a/lib/websocket.js
+++ b/lib/websocket.js
@@ -809,8 +809,13 @@ function receiverOnError(err) {

   websocket._socket.removeListener('data', socketOnData);

-  websocket._readyState = WebSocket.CLOSING;
   websocket._closeCode = err[kStatusCode];
+
+  if (websocket._readyState !== WebSocket.CLOSING) {
+    websocket._readyState = WebSocket.CLOSING;
+    websocket._sender.close(websocket._closeCode, '', !websocket._isServer);
+  }
+
   websocket.emit('error', err);
   websocket._socket.destroy();
 }

but if the other peer receives the close frame, it will try to send back a close frame on an already closed connection. It's a waste of resources.

[pimterry]

Can't you already do that using the status code from the 'close' event? I think applications should not be concerned about the specific protocol error (reserved bit set, invalid opcode, etc.) as long as they can identify a protocol error (1002 status code).

The close event is unfortunately ambiguous in this case right now, because it fires for both local errors and for remotely received close messages. Because of that, you have to process the error events and close events together to tell the difference.

It would be much easier to do this if there was some kind of error code directly attached to error events.

My case is a bit unusual, since I'm doing some low-level networking logging and analysis of my websocket traffic, so I'm especially interested in exactly how and why these connections fail. For basic websocket applications that won't matter, but I do think it'd be useful to have the error code available anyway for the people who do care about this, and I imagine most advanced websocket deployments will eventually want to know this kind of data.

For the spec recommendation...

I've done some more digging into the error logic in the spec, and I think I've now run into three things that differ between how WS works in error cases and what the spec says should happen. I've pulled them out into #1898 to track this and separate the topics before everything gets too mixed up.

In short, when there's an error:

• ws should send a close frame to the remote peer.
• ws should not immediately destroy the socket - it should wait for the remote peer to finish first.
• The close event code here should be 1006 (abnormal shutdown) not the close code we sent to the remote server (which helpfully makes the close event code here unambiguous! But also makes it harder to get the error value, unfortunately).

[lpinca]

Because of that, you have to process the error events and close events together to tell the difference.

Yes, they are emitted one after the other.

My case is a bit unusual, since I'm doing some low-level networking logging and analysis of my websocket traffic, so I'm especially interested in exactly how and why these connections fail. For basic websocket applications that won't matter, but I do think it'd be useful to have the error code available anyway for the people who do care about this, and I imagine most advanced websocket deployments will eventually want to know this kind of data.

ws is the only WebSocket implementation I know that actually exposes these errors. You won't get an 'error' event for invalid frames in other implementations.

[lpinca]

How would you like to procede here? Adding codes like 'WS_ERR_DATA_FRAMING' or with even more details like 'WS_ERR_RESERVED_BIT_SET'? I don't like to have codes only for Receiver errors but I can live with it.

[pimterry]

Yes, something like that would work perfectly for me - an error.code property, similar to how node has various HPE_* errors for HTTP parsing failures.

The more specific the better I guess? It seems easy and convenient to have a different code for distinct type of failure rather than grouping them together unnecessarily. I'd share codes if the same parsing failure can happen in different places, but I don't know if that's relevant.

[lpinca]

Can you help me to find code names for all the errors created by the Receiver? For example

• ws/lib/receiver.js

  Line 171 in 074e6a8

  return error(RangeError, 'RSV2 and RSV3 must be clear', true, 1002);

  and

  ws/lib/receiver.js

  Line 178 in 074e6a8

  return error(RangeError, 'RSV1 must be clear', true, 1002);

  'WS_ERR_RESERVED_BIT_SET' ?

• ws/lib/receiver.js

  Line 188 in 074e6a8

  return error(RangeError, 'RSV1 must be clear', true, 1002);

  ? This means that we saw a continuation frame. They cannot have RSV1 set.

[felixputera]

Sorry for commenting on this closed issue, but I think exposing error codes would be ideal for the WebSocket client too (in lib/websocket.js). I would be glad to pick up the work myself, but do you think we should create a new issue for this or reopen this current issue? @lpinca

[lpinca]

@felixputera feel free to open a PR.