search

Skylar-Tech / node-red-contrib-matrix-chat

Matrix chat server support for Node-RED
GNU General Public License v3.0
31 stars 10 forks source link

Make errors catchable #86

Closed bvmensvoort closed 10 months ago

bvmensvoort commented 1 year ago

Fixes #85 (errors are not catchable)

One downside though. At this moment Node-RED doesn't log errors on the config-node when no catch-node is available. Issue 4219. Other nodes work well. Until this is fixed in Node-RED, it is required to have a catch-node in order to view the errors of the config-node.

skylord123 commented 1 year ago

@bvmensvoort thanks for the PR!

Based on https://nodered.org/docs/user-guide/writing-functions#handling-errors

the function should call node.error with the original message as a second argument

Can you update all your uses of node.error so that it passes the message as the second argument? This way you can get more information about which message triggered the error. If there is no message object (such as receiving an event from the server) you can just leave the second argument undefined, no need to pass a blank object.

I'll review and test the rest of it when a get a chance.

Thanks again.

bvmensvoort commented 1 year ago

Thanks for the feedback. The branch is updated. Where possible, msg is passed instead of an empty object.

But for other cases, an object must be passed in order to pass the error to the catch node. When using undefined the error won't be caught. A simple test did confirm this. See Node.js source (line 535).

skylord123 commented 1 year ago

But for other cases, an object must be passed in order to pass the error to the catch node. When using undefined the error won't be caught. A simple test did confirm this. See Node.js source (line 535).

Oh nice! Thanks for digging into that.

I looked it over again and have a question about the server node tracking it's consumers. I didn't see this actually being used anywhere. Are you using this? An alternative to this approach would be to use RED.nodes.eachNode(function(node) {}); which I believe can be used to build the same information.

bvmensvoort commented 1 year ago

Node-RED needs node.users to handle the error correctly. Something I found out the hard way. See Flow.js source (line 660). An example of a node using this is the MQTT code node. See 10-mqtt.js (line 706).

skylord123 commented 10 months ago

@bvmensvoort thanks for your work on this, greatly appreciate it!

I got it merged and ready for next release. I may end up squeezing some other changes into that release as well so it may take a week before I get it released.