hepcat72 / node-red-contrib-life360

GNU General Public License v3.0
3 stars 4 forks source link

Error Handling: connection errors crash node-red #8

Closed MaksimShakavin closed 2 years ago

MaksimShakavin commented 2 years ago

Hi, I have installed the latest version of this plugin from github and have 2.1.3 version of Node Red. When I deploy a location and server nodes, Node Red crashes and goes into an infinite restart loop. The following error appears in the log: Error: Life360 server error getting circle: 504 Node-RED[26493]: at /mnt/data/root/.node-red/node_modules/node-red-contrib-life360/index.js:106:18

I accept, that there might be a connection issues on my side, or life360 backend downtime. Never the less I believe that such errors should be handled properly and should not crash Node Red. When my internet goes down, all my home automation stops working because of this node.

hepcat72 commented 2 years ago

What happens if you revert back to the previous release?

Honestly, I don't think that the Life360 error is indicative of or responsible for the crash and restart loop you're experiencing. The changes to the node are very small and minor. The errors you're seeing were happening before but just weren't being logged (they were ignored). And the only other change to the code in the latest version is that upon deploy, if a timer was going, it is stopped so the old config node can be cleaned up.

I feel that the crash and restart loop you're experiencing is coincidental and that the problem is due to something else. But try reverting the life360 node version and report back what happens. It would also be good to see more of your logs.

I have experienced continual crash/restarts before of homebridge (unrelated to this node) in the past, and that had been due to a syntax error in my config. I would suggest starting up node-red manually and see what you get on the console.

@brianmay

MaksimShakavin commented 2 years ago

Hi @hepcat72 , thanks for the answer. If I load node red in safe mode and disable life360, the loop stops. But if I enable it again, NR restarts with an error in some seconds. And I don't thinks that this is introduced by a new changes. Before I had a following error sometimes, which also caused a restart: TypeError: Cannot read property 'name' of undefined ноя 22 19:38:43 wirenboard-ASWHWRGG Node-RED[17526]: at ServerNode.sendChanged (/mnt/data/root/.node-red/node_modules/node-red-contrib-life360/nodes/server.js:64:37) ноя 22 19:38:43 wirenboard-ASWHWRGG Node-RED[17526]: at /mnt/data/root/.node-red/node_modules/node-red-contrib-life360/nodes/server.js:241:26 ноя 22 19:38:43 wirenboard-ASWHWRGG Node-RED[17526]: at /mnt/data/root/.node-red/node_modules/node-red-contrib-life360/nodes/server.js:248:17 So it looks like the same bad response that fails while trying to parse it. And I have had this error maybe 2 times a week. So I would blame my internet connection.

But my point was that maybe the plugin should not throw unhandled errors out, which crashes node red, but logs it, and changes status of location node to red?

hepcat72 commented 2 years ago

I see. That makes sense. So this sounds like an uncaught exception when there's an internet connection disruption, resulting in an undefined object on which a key index is executed (name). Seems fairly straightforward to fix. I'll put this on my ToDo. Not sure off the top of my head how I would test to make sure the catch works. Also, not sure when I'll have a chance to implement it.

Thanks for reporting the error.

hepcat72 commented 2 years ago

And thanks for the clarification. I had immediately assumed that the problem started upon upgrading to the latest version.

brianmay commented 2 years ago

Sorry about that. Two factors at play here:

I have a suggested/untested change that I think will fix this.

brianmay commented 2 years ago

See #9.

MaksimShakavin commented 2 years ago

The PR looks good, thanks for the fix. I've done almost the same in my fork, and deployed it locally. So far works ok. But I faced an error, that is not catchable and I don't know why. Here are the logs:

ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]: OperationalError: getaddrinfo ENOTFOUND api.life360.com
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:     at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:66:26) {
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   cause: Error: getaddrinfo ENOTFOUND api.life360.com
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:       at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:66:26) {
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:     errno: 'ENOTFOUND',
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:     code: 'ENOTFOUND',
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:     syscall: 'getaddrinfo',
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:     hostname: 'api.life360.com'
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   },
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   isOperational: true,
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   errno: 'ENOTFOUND',
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   code: 'ENOTFOUND',
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   syscall: 'getaddrinfo',
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]:   hostname: 'api.life360.com'
ноя 27 02:12:28 wirenboard-ASWHWRGG Node-RED[13283]: }

Looks like my internet was down this time and dns lookup failed. However it was not catched by a bluebird promise. I've googled a bit, and it seems like, to handle it we need to subscribe to a request error event, but I don't know how to do it, when request is wrapped into a bluebird promise. Also request has been deprecated for more than two years already. Maybe we should consider migration to an another http client. And promises are now supported by node.js, so no need for a bluebird.

brianmay commented 2 years ago

Yes, agreed, we should not be using a deprecated library. I noticed that also, but felt that rewriting this was beyond the scope of my pull request.

Having said that, if not all errors are being caught with my code, this seems to imply that the library does unexpected stuff, and that might be an excellent reason to change to a supported library for making http requests.