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

Exceptions stacktrace #88

Closed
wants to merge 6 commits into from
Closed

Conversation

kaol
Copy link
Contributor

@kaol kaol commented Aug 21, 2016

This code adds exception handling to template loading. I'm afraid runNode got uglier by this change, I had to make sure that the consolidate evaluation happens already there. By the time the consolidate in compileTemplate would get run the context would already have been lost. I'm not sure whether this causes the pure bytestrings to be created twice.

callTemplate didn't seem right with its lacking _curTemplateFile manipulation and I added it as another patch in this branch.

@mightybyte
Copy link
Member

Can you briefly summarize the main motivation for this? Adding contextNode to SpliceError makes perfect sense to me. But I don't quite understand how that relates to the exception stuff that was added.

Also, the Travis build failures need to be fixed before I can merge.

X.elementTag node):(_splicePath hs')}) $
compileNode node
hs' <- getHS
-- Plain rethrows for CompileException to avoid multiple annotations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add to this comment something like "Since this is compiled splice mode, runNode gets executed at load time so exceptions are ok"?

@mightybyte
Copy link
Member

Ok, I understand now. You're trying to provide better error messages when user splice code throws an exception. Using Either String instead of exceptions for this would be a large change because it would impact user-written splice code. Exceptions shouldn't be a problem here because the compiled splice runNode gets executed at load time. And, we already call error in here.

We could just eat the exception, create a SpliceError, and not re-throw. This would allow the user to see more errors at a time without having to fix some and then re-run. But this distinction probably won't be a big deal in practice, so I'm fine with either way.

@kaol kaol closed this Sep 3, 2016
@kaol kaol deleted the exceptions-stacktrace branch September 3, 2016 19:46
This was referenced Sep 3, 2016
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.

None yet

2 participants