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

When exceptions occur in handler functions or helpers, call chunk.setError #267

Closed
wants to merge 2 commits into from
Closed

When exceptions occur in handler functions or helpers, call chunk.setError #267

wants to merge 2 commits into from

Conversation

sethkinast
Copy link
Contributor

If a provided handler or helper throws an exception, dust doesn't handle them gracefully. This makes it hard to quickly debug handlers.

This patch captures errors from helpers and handlers and uses chunk.setError() to ensure that dust passes the error along.

The easiest way to see the difference is to use the test page with the default asynchronous streaming example. Try adding

throw "Error!";

to either the stream or delay functions, or misspelling setTimeout in the delay function. In the current dust, you'll just see "Pending..." forever because dust has died and can't go on.

With this patch applied, you'll see the error captured and re-thrown dust-style, so that it's displayed in the error panel below the textbox.

@sethkinast
Copy link
Contributor Author

Is there anything I could help out with to push this one along? It's fairly tiny but makes a big impact to not have uncaught exceptions getting thrown.

@rragan
Copy link
Contributor

rragan commented Jun 5, 2013

LGTM

@sethkinast
Copy link
Contributor Author

Last bump before I give up for now.

@vybs
Copy link
Contributor

vybs commented Jun 28, 2013

we have been discussing how to do this in a generic and extensible way

see the PR #279

@rragan
Copy link
Contributor

rragan commented Jun 28, 2013

Which PR has the discussion?

@sethkinast
Copy link
Contributor Author

Is there any chance of this making it in before I just fork the repo? Would like to save the effort if possible.

This is literally the difference between my entire Express server just crashing completely when something bad happens in a helper, and a graceful error output with the page continuing to render.

@jimmyhchan
Copy link
Contributor

We are doing a lot of work now to add helpful debugging. This is definitely
a good use case. Communicating exceptions, debugging and errors in helpers
and in dust in general is currently missing.

Do you expect the template to try to keep rendering the stuff after the
helper?

@sethkinast
Copy link
Contributor Author

Yes, I would expect a runtime exception to be handled the same way an errored chunk is-- the block emits an error, and the rest of the template parses as normal.

@jimmyhchan
Copy link
Contributor

Currently the only thing that uses seterror is when a template is not
found. I believe the current implementation is to immediately stop
rendering. I could be wrong.

@sethkinast sethkinast closed this Oct 2, 2013
@michaelleeallen
Copy link

Just out of curiosity, why was this closed? Has this functionality been merged in a different PR? I need this same functionality for a UI library I am working on that runs in node.js, and when one of my helpers throws an error it shuts down the entire server.

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

Successfully merging this pull request may close these issues.

5 participants