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

More structured errors #322

Closed
remexre opened this issue Aug 24, 2018 · 9 comments
Closed

More structured errors #322

remexre opened this issue Aug 24, 2018 · 9 comments

Comments

@remexre
Copy link

remexre commented Aug 24, 2018

It'd be nice to be able to get errors more precisely when #234 / #297 gets resolved. For example,

tera/src/tera.rs

Lines 195 to 199 in 97174b7

bail!(
"Circular extend detected for template '{}'. Inheritance chain: `{:?}`",
start.name,
parents,
);

and

tera/src/tera.rs

Lines 208 to 211 in 97174b7

None => bail!(
"Template '{}' is inheriting from '{}', which doesn't exist or isn't loaded.",
template.name, p,
),

should return their own enum branch.

@Keats
Copy link
Owner

Keats commented Sep 3, 2018

Can you expand a bit on the errors you want? Both for parsing and rendering stages.

I was planning to have a pretty basic error enum since I didn't think people would want specific errors. I thought that in most of the cases I can think of they would just print them and exit, at least for the parsing stage. In the 2 cases linked, why do you need 2 different enum branch to represent them rather than a simple CoherenceError(String) or something similar?

@remexre
Copy link
Author

remexre commented Sep 3, 2018

It'd be nice to at least have the names of the template with the error and the template that's missing/circular, so maybe a CoherenceError(CoherenceErrorKind, PathBuf, PathBuf) would be better?
(where enum CoherenceErrorKind { Circular, Missing }).

@Keats
Copy link
Owner

Keats commented Sep 4, 2018

But how would you handle differently errors like CircularInheritance, MissingTemplate or MissingMacroFile in your code? You can't really do much other than print the error as it's not fixable from the Rust side

@remexre
Copy link
Author

remexre commented Sep 4, 2018

This is for a use-case where I'm loading templates from a database, so I can handle the missing file errors as "check the db for the file and retry if found."

@Keats
Copy link
Owner

Keats commented Sep 4, 2018

Ah didn't think of that usecase. I'll have separate enum kinds then.
What about rendering? Just a single RenderFailed error?

@remexre
Copy link
Author

remexre commented Sep 5, 2018

Nothing in my usecase needs a more specific error there, but it might be useful to be able to point to a span corresponding to the error?

@Keats
Copy link
Owner

Keats commented Sep 5, 2018

Adding spans to the AST is a significant amount of work that I am probably not going to do myself but I would welcome a PR for it

@Keats
Copy link
Owner

Keats commented Nov 10, 2018

Errors are now without error-chain in the v1 branch if you want to have a go at it for the errors you need!

Keats pushed a commit that referenced this issue Dec 6, 2018
@Keats
Copy link
Owner

Keats commented Dec 6, 2018

@remexre I pushed some new error types on the v1 branch if you want to try it out

@Keats Keats added the done label Jan 4, 2019
@Keats Keats closed this as completed in 77ed8a2 Dec 7, 2019
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

2 participants