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

Render and RenderFile no longer return errors #7

Open
matthewp opened this issue Aug 24, 2010 · 9 comments · May be fixed by #58
Open

Render and RenderFile no longer return errors #7

matthewp opened this issue Aug 24, 2010 · 9 comments · May be fixed by #58

Comments

@matthewp
Copy link

This is bizarre because it must be intentional... which did you change Render and RenderFile to only return a string?

This goes against normal Go convention, which is when an Error is possible, have a return of (Whatever, os.Error), that way we can check to see if err != nil and handle errors that way.

Please fix this! You are a GREAT asset to the go community and it would be sad if one of Go's best programmers changed convention, because if you do, others probably will too :(

@hoisie
Copy link
Owner

hoisie commented Aug 24, 2010

Hi Matthew,

It was done purely based on usage patterns. I looked at my code and realized that every single time Render or RenderFile was called, the return value was just ignored. This might seem like lazy programming, but in general once a template is written and then checked for errors, Render or RenderFile will simply never return an error. Furthermore, on the rare occasions when I actually did capture the error from Render or RenderFile (I think one case), I just printed the error string on the terminal. Because I use mustache.go to render web pages, this new interface allows me to simply see the error string on the web page itself.

What use case do you have where the error value is meaningful? Are you finding random and unexpected errors when dealing with these methods?

@hoisie
Copy link
Owner

hoisie commented Aug 24, 2010

Furthermore, I'm a big subscriber of the DRY principle (don't repeat yourself), which is at odds with the way Go does error checking.

@matthewp
Copy link
Author

I've never encountered an error, however what you just said frightens me. I use mustache.go to render web pages as well. Do you want the error string in the browser in a production environment??? I don't! I want to capture the error, save it to a log, and render a backup "oops" page instead.

@matthewp
Copy link
Author

It's your library, I'll just check my diff when I update. It's only a couple of lines that need to be changed. Thanks anyways.

@hoisie
Copy link
Owner

hoisie commented Aug 24, 2010

I suppose I could add MustRender and MustRenderFile which panics instead of returning the error. However in a production environment that would bring down the server instead of displaying the error message.

@spiffytech
Copy link

It would only bring down the web server if you pass the error to panic() instead of handling it in a different way, and I would presume someone wanting MustRender would want to handle it in a different way. E.g., throw the user a 500 error instead of a broken template? Or something more complex, depending on the task.

I do think it's important to have a way to programatically recognize when a template fails to render.

@hoisie
Copy link
Owner

hoisie commented Apr 8, 2013

I'd accept a pull request that changes it back to returning an error. Is there one open?

@spiffytech
Copy link

I attempted to do this everywhere I found errors suppressed in this pull request, but I can't force a test program to error out by e.g., leaving out a variable I referenced in my template.

@nathanboktae
Copy link

You can (slightly) work around this by using ParseFile that does return an error:

    tmpl, err := mustache.ParseFile(templateFileName)
    if err != nil {
        return "", err
    }

    text := tmpl.Render(model)

Render still has no error, but You can get some templating errors.

Or we all move to a language that actually has exceptions 😒 sigh...

@movitz-s movitz-s linked a pull request Dec 30, 2020 that will close this issue
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 a pull request may close this issue.

4 participants