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

Errors from render_template have themselves as source #398

Closed
SuperCuber opened this issue Nov 27, 2020 · 6 comments
Closed

Errors from render_template have themselves as source #398

SuperCuber opened this issue Nov 27, 2020 · 6 comments

Comments

@SuperCuber
Copy link

Code:

extern crate handlebars;

fn main() {
    let handlebars = handlebars::Handlebars::new();
    let e = handlebars.render_template("{{ whoops", &std::collections::BTreeMap::<String, String>::new()).unwrap_err();
    let e: Box<dyn std::error::Error> = Box::new(e);
    println!("{}", e);
    println!("{}", e.source().unwrap());
    println!("{:?}", e.source().unwrap().source());
}

Expected output:

Template error: invalid handlebars syntax.
    --> Template error in "Unnamed template":1:10
     |
   0 | {{ whoops
     |---------
     |
     = reason: invalid handlebars syntax.

<panic because no source>

Actual output:

Template error: invalid handlebars syntax.
    --> Template error in "Unnamed template":1:10
     |
   0 | {{ whoops
     |---------
     |
     = reason: invalid handlebars syntax.

Template error: invalid handlebars syntax.
    --> Template error in "Unnamed template":1:10
     |
   0 | {{ whoops
     |---------
     |
     = reason: invalid handlebars syntax.

None

I haven't tested, but this could also be the case for other functions.

@sunng87
Copy link
Owner

sunng87 commented Nov 29, 2020

This is expected. The Error returned from render_template is a wrapped type TemplateRenderError which has two branch TemplateError and RenderErorr. This is going to change in #395 in which I made TemplateError a source of RenderError

@sunng87 sunng87 closed this as completed Dec 9, 2020
@SuperCuber
Copy link
Author

#395 is merged but it doesn't look like the behavior changed in master

@mkantor
Copy link
Contributor

mkantor commented Dec 25, 2020

this could also be the case for other functions

It does look like other errors have duplication between the main message and the source(). For example if you do {{array.[not a number]}} you'll get an error like Error on accessing array/vector with string index.: invalid digit found in string with the source() message being invalid digit found in string. It's like that for all errors that go through RenderError::from_error.

Is this a problem, though? I know it'll look odd if you're using a library like anyhow that prints the source(). I guess I'm not sure what the best practice is: Should the outer message be less detailed with the expectation that you'll also see the source()? Should the messages be combined (as now) but with no source() attached? Or are the current semi-redundant messages fine as is?

@mkantor
Copy link
Contributor

mkantor commented Dec 25, 2020

I also noticed that since #395 these errors have weird punctuation (Error blah.: whatever). I opened #403 to fix that.

@SuperCuber
Copy link
Author

I guess I'm not sure what the best practice is

I think either the outer message should be less detailed with elaboration in source(), or have it combined (probably on a case by case basis, for example for messages that have only one option for source() it perhaps makes sense to combine them)

I also think this doesn't matter much, because string-based errors basically cannot be caught, only displayed to the user (usually in a stack of source()s)

@Nemo157
Copy link

Nemo157 commented Dec 25, 2020

Errors should also not start with the word "Error"

The error message given by the Display representation of an error type should be lowercase without trailing punctuation, and typically concise.
C-GOOD-ERR

Error reporters such as anyhow will add an Error: prefix when printing the error chain, resulting in duplication in the first line (along with the duplication from the error printing its source itself):

Error: Error with parsing template: Template error: invalid handlebars syntax.
    --> Template error in "Unnamed template":1:10
     |
   0 | {{ whoops
     |---------
     |
     = reason: invalid handlebars syntax.

Caused by:
    Template error: invalid handlebars syntax.
        --> Template error in "Unnamed template":1:10
         |
       0 | {{ whoops
         |---------
         |
         = reason: invalid handlebars syntax.

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

No branches or pull requests

4 participants