search

smchamberlin / node-red-contrib-ibm-db2

Node-RED nodes to work with a Db2 LUW database. Works with "Db2 on Cloud" and "Db2 Warehouse on Cloud" services as well as standalone installation of Db2 LUW.
4 stars 14 forks source link

properly throw errors so Catch node can handle them. #21

Open rophy opened 5 years ago

rophy commented 5 years ago

Currently dashdb node doesn't properly handle errors - it logs the error on console, then "swallows" it.

Impact: when errors happened in dashdb nodes, the flow simply terminates and there is no way for flow developer to handle error and recover.

The proper way to handle errors as documented in node-red official docs:

Handling errors

If the function encounters an error that should halt the current flow, it should return nothing. To trigger a Catch node on the same tab, the function should call node.error with the original message as a second argument:

node.error("hit an error", msg);

This PR updates dashdb code to pass in msg in all node.error() calls so that Catch node will be able to catch those errors.

rophy commented 5 years ago

@smchamberlin I have been using the patched version in my project for a month and it worked fine so far.

In terms of "breaking changes", there is one thing worth mentioning:

when errors happen, the node need to decide whether:

  1. throw an exception or not (which should get caught by Catch node)
  2. continue to send outputs, or terminate the message flow

The PR added (1) in all error workflows, which is fine. But in terms of (2), in current master branch, it is inconsistent on whether the node sends output or not. For example, 98-sqldb-dashdb-cf.js#L115 terminates the flow and send nothing, while 98-sqldb-dashdb-cf.js#L353 sets msg.error but continue to send an output of msg.payload = null.

The PR tries to be consistent that the node "terminates the flow" in all error cases (mainly https://github.com/smchamberlin/node-red-nodes-cf-sqldb-dashdb/pull/21/files#diff-6a9587bbfd51a40c36f0ab107ab1e3b1R212), but this can be a breaking changes if a workflow assumes the node will always send output in 98-sqldb-dashdb-cf.js#L353 even when error happens.

Do you have any thoughts on what we want to define behavior in the case of (2) ?