Network Failure Unhandled

[ChrisTalman]

Please describe the problem you are having in as much detail as possible: An unhandled exception, as follows, is thrown when my network adaptor is disabled. This seems odd, as I was under the impression that this should be handled gracefully by the disconnect event.

Error: getaddrinfo ENOENT gateway.discord.gg:443
    at Object.exports._errnoException (util.js:1034:11)
    at errnoException (dns.js:33:15)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:73:26)

Include a reproducible code sample here, if possible: The exception is thrown whenever there is any active connection to the Discord API, such as below.

const Discord = require('discord.js');
let robot = new Discord.Client();
let token = 'example123';
robot.login(token);

Further details:

• discord.js version: 11.1.0.

• node.js version: 7.8.0.

• Operating system: Windows 10 Pro 10.0.15063 Build 15063.

• Priority this issue should have – please be realistic and elaborate if possible: Medium. This exception may not be thrown very often, but if the intention is for disconnect to handle it, it could cause ungraceful failures in some cases, which would seem undesirable.

• [x] I found this issue while running code on a bot account

• [ ] I have also tested the issue on latest master, commit hash:

[devsnek]

this is your internet, not a lib problem

[ChrisTalman]

@devsnek Well, in this case, my Internet was fine, and I deliberately disabled my network adaptor to see whether a loss of connection would be handled gracefully. It was not. Strictly speaking, is this not what the disconnect event is intended to handle? Its description is as follows: 'Emitted when the client's WebSocket disconnects'. Is this not a case of the client's WebSocket disconnecting?

[devsnek]

it only emits when the socket disconnects in a way that it can't reconnect, usually we will automatically resume the ws for you.

[ChrisTalman]

@devsnek Okay. Well, this seems to be such a case, where the socket cannot reconnect. It would be nice for Discord.js to gracefully handle an exception such as this. It seems to fall within the scope of the disconnect event as described in the documentation, and would be very helpful in a library which handles WebSockets at its core.

[ghost]

@ChrisTalman How exactly would you handle such an exception? If you can't get DNS info about an address, then you can't reconnect. The only thing you can do at this point is to shut down the process.

[ChrisTalman]

@CharlotteDunois The context is that a network connection is not currently possible, but that it could be in the future. Thus, it is possible to reconnect, eventually. The library could handle this itself by reconnecting automatically, or application code could handle disconnect and call login at some interval.

[ShadowDrakken]

This error happens when internet connectivity is lost by any means, not just manually disabling the NIC. Previously, the bot would sit there and wait and if the connection returned within about 2 minutes it would reconnect. Well, sometimes, it's always been flakey about reconnecting.

Anyway, the correct behavior would be to sit and wait, occasionally polling to see if it can reconnect. Say once per minute or something to that effect. Not to just dump out and give up without the user's interaction.

[devsnek]

I disagree, we aren't responsible for your internet. If you need to keep something hosted 24/7 buy yourself some hosting or get better internet.

[ShadowDrakken]

That's not what the issue is about. It's about gracefully handling network interruptions, like every other half decent piece of software.

[devsnek]

we will not keep a thread open "occasionally polling to see if it can reconnect". that is objectively stupid. you can poll yourself and call login if you need to.

[ChrisTalman]

If that's the behaviour the user wants, I don't see how that's 'objectively stupid'. It's hardly like that would be an intensive operation, and it could be made optional. But, in any case, this issue started with the even more basic expectation that the disconnect event be fired in the case of a network failure instead of an exception being thrown. If this were done, a user could implement their own reconnection attempts. Although, having said this, having reconnection functionality out of the box would clearly be valuable to some users as has been demonstrated by this issue thread.

[ShadowDrakken]

"You can poll yourself and call login"

That it patently untrue, because the exception is closing the process out leaving no opportunities to do this.

[ShadowDrakken]

ChrisTalman: it used to have auto-reconnect. Prior to this error developing.

[devsnek]

the option known as autoReconnect was removed in 11.x because we made it so you cannot turn it off. The issue with the error not bubbling up through client#onerror and client#ondisconnect will be fixed, but no new methods for "extended" reconnection will be added.

[aemino]

Personally, I think that the library should be tolerant of various network faults. If no one else bothers to implement this, I will eventually.

[amishshah]

Mostly fixed in master, may still error on failed reconnects (not sure can't test atm)

[SpaceEEC]

Is this still an issue? Simply handling the error event with a console.log and letting discord.js reconnecting is working fine for me on 11.3 and https://github.com/discordjs/discord.js/commit/5352e28700e51f1148463f350fe5694124a3c270.

[ChrisTalman]

I'm not sure. I didn't see this issue mentioned in the release notes for 11.3.

I certainly know that socket hang exceptions still seem to happen (for instance, see #2047). This may or may not be related to the network failure handling, but this is difficult to determine given that in both cases the exception is thrown out of nowhere. Both certainly concern appropriately handling failure on the network, though.

[kyranet]

After voice-rewrite, it seems to be fixed. The commit from the master branch in 5efddac (before voice-rewrite) doesn't handle these errors, and oddly enough, it logs the ECONNRESET twice: one as handled, and another as unhandled.

After 86da7af, that got fixed in my end and these errors are properly handled by the error event.

[Lewdcario]

This is not an issue on 11.3-dev, so I will close this.