Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Websocket errors can be emitted as ErrorEvents instead of Errors #2493

Closed
1 of 2 tasks
Throne3d opened this issue Apr 21, 2018 · 3 comments
Closed
1 of 2 tasks

Websocket errors can be emitted as ErrorEvents instead of Errors #2493

Throne3d opened this issue Apr 21, 2018 · 3 comments

Comments

@Throne3d
Copy link

Please describe the problem you are having in as much detail as possible:
The parameter emitted by Client on the error event is sometimes an ErrorEvent, not an Error. This happens, in particular, when the websocket connection is broken by putting my laptop to sleep.

The documentation for Client#error claims that the parameter will be of type Error: https://discord.js.org/#/docs/main/stable/class/Client?scrollTo=e-error. At least when using ws, due to the way discord.js binds to it, the event handler is given a parameter of type ErrorEvent when the websocket connection is broken. (This ErrorEvent is not exactly https://developer.mozilla.org/en-US/docs/Web/API/ErrorEvent, but instead a recreation in ws: https://github.com/websockets/ws/blob/d390dc5b34c269669d5fa4450cd394d957055296/lib/event-target.js#L87.)

It is easy to get the error from this ErrorEvent – simply by using errorEvent.error – but it did cause me a bit of trouble, and means that simply logging errors that the client outputs won't always give a useful stack trace. (In a slightly different case, where I was handling these errors, it meant that I got a very confusing output for the event listener on ws's side, and no details of the error at all.)

Include a reproducible code sample here, if possible:

const discord = require('discord.js');
const token = process.argv[2];

const client = new discord.Client();
client.on('ready', () => console.log("Logged in"));
client.on('error', (error) => console.log(error));

client.login(token);

Run this code, with a token as a parameter, and then somehow disconnect your internet connection to prompt a websocket error. On my Windows 10 laptop, when I enable flight mode, I get the following output:

Logged in
ErrorEvent {
  target:
   WebSocket {
     _events:
      { message: [Function],
        open: [Function],
        error: [Function],
        close: [Function] },
     … },
  type: 'error',
  message: 'getaddrinfo EAI_AGAIN gateway.discord.gg:443',
  error:
   { Error: getaddrinfo EAI_AGAIN gateway.discord.gg:443
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:67:26)
     message: 'getaddrinfo EAI_AGAIN gateway.discord.gg:443',
     errno: 'EAI_AGAIN',
     code: 'EAI_AGAIN',
     syscall: 'getaddrinfo',
     hostname: 'gateway.discord.gg',
     host: 'gateway.discord.gg',
     port: 443 } }

Further details:

  • discord.js version: 11.3.2 (and separately b8a9a76)
  • node.js version: v9.7.1
  • Operating system: Windows 10
  • Priority this issue should have – please be realistic and elaborate if possible: medium or low. It's easy to get the error from the ErrorEvent, but the documentation is wrong and I had to debug this issue to get stack traces from this class of error.
  • I found this issue while running code on a user account
  • I have also tested the issue on latest master, commit hash: b8a9a76
@iCrawl
Copy link
Member

iCrawl commented Apr 21, 2018

EAI_AGAIN is quite the bad example here, as this is not your "usual" error, and if your host loses connection to the outside world you probably know anyway.

I'd be more interested in other examples you mentioned?

@Throne3d
Copy link
Author

I don't think I've encountered the issue with any non-EAI_AGAIN errors, but I do expect it to generalize to other websocket errors due to how discord.js looks to be binding to the websocket. (Well, except maybe uws websocket errors, but its bindings seem to suggest it just gives a basic object with message: 'uWs client connection error' for every client error.)

The reason I expect it to happen with other websocket errors is that they all seem to be handled the same way inside WebSocketConnection#onError. It seems to be consistently treating ErrorEvents as Errors:

onError(error) {
if (error && error.message === 'uWs client connection error') {
this.reconnect();
return;
}
/**
* Emitted whenever the client's WebSocket encounters a connection error.
* @event Client#error
* @param {Error} error The encountered error
*/
this.client.emit(Constants.Events.ERROR, error);
}
The code is bound to websocket errors by setting it as the ws.onerror property:
ws.onerror = this.onError.bind(this);

But both Mozilla's web API documentation and ws's documentation seem to suggest or state that, when bound this way, the callback receives an event, not the error itself (and so the client emits an event, not the error itself, hence the issue).

I'm not super confident submitting a pull request on this – partly because I've never contributed here before and partly because I don't know how the individual websocket libraries work that well – but I could try?

@vladfrangu
Copy link
Member

This will be fixed in master once #3192 gets merged, and Client#shardError will now consistently emit an Error object. This difference was mostly due to uWs emitting an Error, while ws emits a ErrorEvent-like structure, which discord.js didn't handle.

@iCrawl iCrawl closed this as completed Sep 10, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants