render: Errors are hard to track down

arschles commented 6 years ago

If a template has an error in it, you get a slightly-ambiguous error message and a stack trace. For example:

jobs.html: jobs: unknown identifier
github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo.sessionSaver.func1
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo/session.go:72
github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo.RequestLoggerFunc.func1
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo/request_logger.go:51
github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo.(*App).PanicHandler.func1
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo/errors.go:69
github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo.RouteInfo.ServeHTTP
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo/handler.go:75
github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo.(*RouteInfo).ServeHTTP
<autogenerated>:1
github.com/arschles/<repo>/vendor/github.com/gorilla/mux.(*Router).ServeHTTP
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/gorilla/mux/mux.go:114
github.com/arschles/<repo>/vendor/github.com/markbates/refresh/refresh/web.ErrorChecker.func1
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/markbates/refresh/refresh/web/web.go:23
net/http.HandlerFunc.ServeHTTP
/usr/local/Cellar/go/1.9.2/libexec/src/net/http/server.go:1918
github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo.(*App).ServeHTTP
/Users/arschles/gocode/src/github.com/arschles/<repo>/vendor/github.com/gobuffalo/buffalo/app.go:132
net/http.serverHandler.ServeHTTP
/usr/local/Cellar/go/1.9.2/libexec/src/net/http/server.go:2619
net/http.(*conn).serve
/usr/local/Cellar/go/1.9.2/libexec/src/net/http/server.go:1801
runtime.goexit
/usr/local/Cellar/go/1.9.2/libexec/src/runtime/asm_amd64.s:2337

I actually still have not tracked down this issue, but I think I forgot a closing } somewhere.

Is there a "dev" flag I can pass to the renderer or parser?

(I wasn't sure where to file this issue -- here or in https://github.com/gobuffalo/plush. If I put this in the wrong place, I'm happy to re-file it over there.)

Thanks!

markbates commented 6 years ago

The first line is telling you what the problem is:

jobs.html: jobs: unknown identifier

you are reference a variable, or function, named jobs that isn't on the context.

The problem with the stack trace is that Go doesn't actually give you stack traces, so we use a package called github.com/pkg/errors to generate a stack trace. Sometimes it's good and helpful, sometimes not so much. Unfortunately it's one of the big down sides of go. :(

arschles commented 6 years ago

Yep, makes sense. I think I didn't explain well -- I was more referring to the lack of information to help developers debug their templates.

Let me give some better examples.

Example 1 - Missing Closing `%>`

Given this template:

<!DOCTYPE html>
<html lang="en-us">
<head>
    <%= if (true) {
    <title>Hello buffalo!</title>
    <% } %>
</head>
</html>

The renderer will panic because the if doesn't have a closing %>. In this particular case, the plush parser looks to be the culprit. I think here, it would be more helpful to indicate that there's a missing closing %> and print the line number.

In renderer terms, the heuristic might be to record the line break token and, if there's a syntax error (or nil pointer as seems to be the case here), tell the user something like "we got a syntax error, did you maybe forget to add a closing %> on line X?"

Example 2: Missing Curly Brace

If you forget to add an opening or closing curly brace ({ or }):

<!DOCTYPE html>
<html lang="en-us">
<head>
    <%= if (true) %>
    <title>Hello buffalo!</title>
    <% } %>
</head>
</html>

Then you get a stack trace and a descriptive error message:

file.html: expected next token to be {, got %> instead

If the file is not complex (1-2 block-level statements), I believe these messages are just fine, but as the template gets more complex, I think it's a lot harder to figure out where the error is. For me, a line number along with the file name would be extremely helpful.

Conclusion

I hope this doesn't come off as me saying there's something wrong with plush, the template syntax, or something else. A huge :100: from me on all things buffalo templating. This change is just a bit to give developers a little more ease of use.

And if you think this is a reasonable way to go, I'm happy to take a crack at these changes in plush.

Thoughts?

markbates commented 6 years ago

I did just commit this, https://github.com/gobuffalo/plush/pull/26/commits/c2c634d757d1b98674eaf58bcf0d346a23bedecb, which adds line numbers to the error message, which should go a long way to address.

If you've got ideas on improving it, please open a PR. I would love to see better, and clearer, error messages come out of Plush.

https://www.evernote.com/l/ABE9Toh7M4dCuIYkWsZVPz1tLHbQhBg9spU

arschles commented 6 years ago

Line numbers FTW indeed 😄

I'll put together some repro steps for the panic when I get a minute, and submit a PR to plush for it.

gobuffalo / buffalo