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

Parsing errors on invalid templates are uncaught, unclear, and unhelpful #42

Closed
shonfeder opened this issue Mar 27, 2020 · 4 comments
Closed

Comments

@shonfeder
Copy link

A minimal example:

let () =
  ignore (Mustache.of_string "{{.name}}")

Which crashes with

Fatal error: exception Mustache_parser.MenhirBasics.Error

A first-level fix would be to catch this error and raise something a bit more informative (e.g., Invalid_mustache_template). A better fix would be to identify the location of the syntax error.

I'm happy to help with the first part, at least, if that would be welcome.

@shonfeder
Copy link
Author

When I've managed to trigger tracebacks, they look like this:

Uncaught exception:  
  
  Mustache_parser.MenhirBasics.Error

Raised at file "lib/mustache_parser.ml", line 347, characters 8-18
Called from file "lib/mustache.ml" (inlined), line 151, characters 18-49
Called from file "lib/mustache.ml", line 212, characters 31-44

@rgrinberg
Copy link
Owner

Proper error messages would definitely be welcome.

@gasche
Copy link
Collaborator

gasche commented Dec 27, 2020

I added support for reporting of lexing and parsing errors in #47, and then more support for render-time errors in #51 (and some more on #52). I also propose to add reference outputs for various error messages in #55. (As mentioned in #55, the error messages I have proposed so far do a reasonable job when the mistake is in the template file, not when the mistake is in the JSON, but this issue is on invalid templates.)

The particular example given by @shonfeder, however, is trickier than most and still badly handled with all these PRs. I don't understand all the details, but I think that this is related to the (somewhat unorthodox) way the lexer is setup. (.name is not a valid section name, but . is.) What happens in practice is that the error is only noticed at the very end of the file: a location is shown, but it is not helpful.

@gasche
Copy link
Collaborator

gasche commented Dec 28, 2020

The particular example in this issue, {{.name}}, is fixed by #56 which provides a good error location in this case (right after the dot, and it says '}}' expected.)

@gasche gasche closed this as completed Dec 29, 2020
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

3 participants