properly throw errors so Catch node can handle them.

[rophy]

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.

[smchamberlin]

trophy, sorry for the late reply.

I haven't heard of this catch node before. Thank you for making the effort to update the code to collaborate with this framework.

Have you tested your changes to make sure they work as they should? And nothing breaks?

Thanks

[rophy]

@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) ?

[smchamberlin]

It looks good, just have one question where a try/catch was removed. Thanks!

[smchamberlin]

Why did you remove the try/catch here ?