mattdavis90 / node-red-contrib-tado-client

Tado web API client node for Node Red
MIT License
23 stars 16 forks source link

Handle errors in a way that they can be caught #33

Closed Roaders closed 3 years ago

Roaders commented 3 years ago

This PR uses the new done() function for handling errors as recommended here:

https://nodered.org/docs/creating-nodes/node-js#handling-errors

This allows errors to be caught using a catch node:

https://nodered.org/docs/user-guide/handling-errors

I have issues with my Tado bridge losing connection to the internet. A catch node will allow me to automatically power cycle my bridge when it is unresponsive.

mattdavis90 commented 3 years ago

Thanks for the PR. Had no idea that the API had changed between v0.X and v1. Looks great, my only query is whether send and done will be null or undefined in v0.X. The official docs use

this.on('input', function(msg, send, done) {
    if (done) {
        done();
    }
});

Thanks

Roaders commented 3 years ago

checking if something is defined with if(done != null) is the safest method. This uses non-strict equality checks so it is actually making sure that it's not null OR undefined. if(done !== null) would strictly check that it's not null (but undefined would be ok) and if(done !== undefined) would strictly check that it's not undefined (and again null would be ok).

The problem with if(done) is that it evaluates if done is truthy in a non-strict way. That's fine in most case but anything that is defined but evaluates to falsy (such as false or 0) would not execute the if statement.

mattdavis90 commented 3 years ago

Thanks. I've merged this. I have a bumpversion to look at in the underlying library then I'll update the dependencies and push a new version. Thanks

mattdavis90 commented 3 years ago

v0.9.5 is now pushed to npm. I'll refresh the node-red library shortly

Roaders commented 3 years ago

Great. Thanks!