Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error Handler node #560

Closed
3 tasks done
knolleary opened this issue Feb 11, 2015 · 9 comments
Closed
3 tasks done

Error Handler node #560

knolleary opened this issue Feb 11, 2015 · 9 comments
Milestone

Comments

@knolleary
Copy link
Member

Our general philosophy has been that if a node hits an error of some sort, it should log it (via node.error) and not send on a message.

We need to make it easier to handle error conditions within a flow.

We have considered some different approaches:

  1. standardise on attaching a msg.error property if something goes wrong and allow the flow to continue. Problem: you would have to add checking for msg.error after every single node - gives the entire problem back to the user.
  2. add an error output on every node. Problem: U G L Y.

And then @hindessm had an idea, and it was good.

The plan is to add a new 'error handler' input node to the palette. This node gets triggered whenever a node on the same tab as it calls node.error.

This means all existing flows carry on as before. But when a user wants to handle an error scenario, they can add this node to the tab and wire up whatever handling they want.

By scoping the handler to nodes on the same tab (ie, share the same z property), different flows can have different error handlers by putting them on different tabs. Subflows can also have their own internal error handler (if a subflow doesn't have one, the error would propagate up to the tab the subflow instance is on).

The follow changes are required:

  • create the error handler node
  • extend the prototype of the node.error function so a msg object can be passed in. This allows the message that caused the error to be passed on to the error handler node. Vital if dealing with an HTTP flow where msg.res is needed to response properly. Obvs needs to be done so existing callers of node.error work as expected.

Calling node.error("an error occurred") will not trigger the Catch node. A message object must be passed as the second argument to .error().

This allows non-flow-based errors to be properly reported as they are today, but where it is feasible for a catch flow to recover an error, it can be enabled.

The error handler node could be extended in the future to capture any level of log event, not just error. However that poses some design questions over what msg object gets passed. Not a requirement for this milestone.


  • add catch node to palette - done Add Catch node #571
  • update core nodes to pass msg to node.error when appropriate
  • update web nodes to pass msg to node.error when appropriate
@knolleary knolleary added this to the 0.11 milestone Feb 11, 2015
@dceejay
Copy link
Member

dceejay commented Feb 12, 2015

This will also be a good fix for Issue #510

@ly685-zz
Copy link

I like the suggestion. It allows the flow to remain relatively simple while still allowing better error handling. Can I assume an error handling node could be input to a subflow? For instance, if i want to have an error handling subflow to keep the top level cleaner. Minor comment but just thinking about maintaining how readable the flow is

@knolleary
Copy link
Member Author

@ly685 an error handler node acts as any other input node - so yes it can be wired into a subflow which contains the actual error handling logic.

@TotallyInformation
Copy link

Would the LACK of an error handler node mean that the flow behaves as per the current model?

@knolleary
Copy link
Member Author

@TotallyInformation yes. The behaviour in the originating flow stays exactly as it does today.

@TotallyInformation
Copy link

Cool. Thanks for clarifying. Nice.

@nganani
Copy link

nganani commented Mar 2, 2015

@knolleary very interested in this new node/feature. any estimate on when it will be available?

@knolleary
Copy link
Member Author

@nganani - it will be part of the next release, in the next couple of weeks.

The node itself is in git already - some more work needed in the other nodes to start making use of it.

@nganani
Copy link

nganani commented Mar 2, 2015

@knolleary - thanks for the quick response - this is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants