PrismarineJS / node-yggdrasil

Node.js library to interact with Mojang's authentication system, known as Yggdrasil
Other
40 stars 27 forks source link

Better error in case of no connection #37

Open Moire9 opened 4 years ago

Moire9 commented 4 years ago

I'll copy from an issue I mistakenly made on a project that uses yggdrasil.

My parents have my internet set to automatically turn off at 21:00. However, this is not what you think. Instead of it turning off, instead, all requests are intercepted and sent to a random xfinity 404 page, essentially blocking communication between client and server. However, this causes an error in node-at yggdrasil.


/mnt/d/Users/SirNapkin1334/Desktop/node-autotip-mod/node_modules/yggdrasil/lib/utils.js:23
if (resp.body.length === 0) {
^

TypeError: Cannot read property 'body' of null at /mnt/d/Users/SirNapkin1334/Desktop/node-autotip-mod/node_modules/yggdrasil/lib/utils.js:23:14 at ClientRequest. (/mnt/d/Users/SirNapkin1334/Desktop/node-autotip-mod/node_modules/phin/lib/phin.compiled.js:1:2551) at ClientRequest.emit (events.js:198:13) at TLSSocket.socketErrorListener (_http_client.js:401:9) at TLSSocket.emit (events.js:198:13) at emitErrorNT (internal/streams/destroy.js:91:8) at emitErrorAndCloseNT (internal/streams/destroy.js:59:3) at process._tickCallback (internal/process/next_tick.js:63:19) npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! node-autotip@2.1.3 start: node index npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the node-autotip@2.1.3 start script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in: npm ERR! /home/sirnapkin1334/.npm/_logs/2020-05-05T04_21_00_075Z-debug.log


It seems, that when the internet connection is shut off, that the `response` becomes null, however, since this was never expected, there are no checks for this, therefore it attempts to retrieve data it cannot get.
wvffle commented 4 years ago

When does the error pop up? It looks like it's a npm install, is it?

Also do you expect to throw a descriptive message or what?

Moire9 commented 4 years ago

Well, in this case, node-at uses it as a dependency or something, and in order to use node-at, you cd to it's directory, run npm install and then npm start. When using node-at, it is using yggdrasil to connect to hypixel.net, and I assume it is constantly using yggdrasil to send heartbeats or receive or send packages to the server or something. And ideally, it should have a descriptive message, something like "Connection to server lost" or "Connection to auth lost" or continue to attempt to connect in case it is simply a blip in the connection or a mistake.

wvffle commented 4 years ago

The topic needs some digging to be done.

A quick way to do this is to add following statement before the line where error occurs.

if (resp === undefined) {
  cb(new Error('Message'))
}

There may be an option to simply prepend the failing condition with resp !== undefined. I'd check if it's not a better way to do this but currently I have no time for that.

Also, the error does not mean that the connection is lost. To be sure why the err and resp are undefined one should check phin documentation and issue tracker.

Try updating the dependencies. The error stack points to line 23 where I couldn't find the source of the problem. You're using an older version of yggdrasil (and mineflayer) Updating the dependencies may fix this error.

If updating dependencies won't work and you can find the source of the problem, I encourage you to create a PR

Moire9 commented 4 years ago

Oh gosh. This isn't my project, it belongs to @builder-247. Perhaps I'm looking at this wrong, but it says that resp is null, so wouldn't checking if it === undefined always be false?

wvffle commented 4 years ago

This isn't my project, it belongs to @builder-247 I'd probably start with creating the issue on his project then

that resp is null, so wouldn't checking if it === undefined always be false? Yes, I meant === null

Moire9 commented 4 years ago

I did. However, it is not a problem with the project, rather with yggdrasil. Still, I will see about getting him to update yggdrasil.

rom1504 commented 4 years ago

I don't understand at all what this issue is about. If you lose the connection, you will lose the connection to minecraft, so anyway you will have to reconnect. So you need to reconnect from your project (ie recreate mineflayer or Yggdrasil instance)

Does that solve the issue ?

Moire9 commented 4 years ago

Not neccessarily that, but I was thinking, it'd be better to use a try-catch and then throw a more descriptive custom error message, such as "Internet Connection Interrupted"