Keats / tera

A template engine for Rust based on Jinja2/Django
http://keats.github.io/tera/
MIT License
3.43k stars 279 forks source link

Rendering should not silent fail when using an object in a template #481

Open Ten0 opened 4 years ago

Ten0 commented 4 years ago

When rendering with an invalid template that refers to an Object instead of strings/integers/bools/arrays, the rendering silent fails by rendering "[object]", due to: https://github.com/Keats/tera/blob/b91a985951fccf6896c7192ea7c78ba64608bb62/src/context.rs#L119 (by the way, this could be Cow::Borrowed)

This is does not match the rust philosophy of being explicit and quickly failing so that we know what went wrong (be it, here, the template being invalid). - Feels like JS :((

I could see pretty-printing or JSON serialization make some sense here, but my favorite option by far would be a rendering failure, telling to whoever wrote the template to update it because this was probably a mistake.

Keats commented 4 years ago

I'm not sure an error would be great here, it's a trait to render a Value to a string and I don't think it's faillible. Pretty printing is likely bettter imo

Ten0 commented 4 years ago

It's currently not faillible but I don't see a reason why it couldn't be.

Keats commented 4 years ago

Transforming a JSON value into a string is not faillible, no reason to make that an error imo

Ten0 commented 4 years ago

I think there are two reasons : first, if you end up, during template rendering (probably destined to final user), being asked to display a struct (a non-final type), that is more likely to be a mistake in the template than anything else - in other words, "it's a mistake" is the most common scenario - so the corresponding behavior (throwing an error to whoever wrote the template) should be the default.

Also, even if that wasn't the most common, just the risk it would create of sending to the end user the "Hello [object]" or "Hello {first:John, last:Smith}" (for e.g. a name struct) justifies making any other behavior than throwing a rendering error error require explicit description.

Now to enable this we would indeed need to edit that trait and make it possibly issue a rendering failure, but I don't see anything that prevents us from doing so.

Ten0 commented 4 years ago

Oh, length does some silent-failing to 0 as well 😞

tobx commented 2 years ago

@Keats First of all, Tera is awesome, I looked into many template engines lately and have chosen this, but I came across this issue today and want to raise some points again and hope you might reevaluate the situation:

First, the current render function is not consistent. An array is rendered "json-like" while an object is just rendered as a static string. Why not both render in the same way?

Second, I do not understand your point that a JSON Value is not faillible. What has JSON to do with a template engine? If you really want to compare this, then the only reason why a JSON Value is not failible, is because there are no errors in its template. The template is well defined in recursing through the Value while never actually printing an object, it only prints String, Number, Boolean or Null. The rest (curly brackets etc.) is part of its template.

If you would be able to use custom templates to render JSON I am pretty sure they should be failible.

Third, from a user's or programmer's perspective I think it could be fatal to silently fail, just think about how long website errors would persist without notice if there are changes and someone is missing an [object]. This could lead to unnoticed follow up errors (e.g. when JavaScript expects a number while it receives [object]).

Last but not least, why would anyone prefer to search through all of his rendered templates in order to find an error instead of just getting the error presented by the system?

Keats commented 2 years ago

What has JSON to do with a template engine? I

Tera serializes all values into JSON before rendering a template.

In practice, this is not going to be changed in Tera v1 but will likely be completely changed in v2 (not sure how yet, but no [object] at least). I've changed the tags on this issue.

tobx commented 2 years ago

Sounds great, thank you for the fast answer and nice to hear this is going forward!

mamcx commented 2 years ago

I hit with a problem related to this (https://www.reddit.com/r/rust/comments/t9rujx/how_decode_a_structjson_from_an_actixtera_form/) and I'm unable to transfer a struct from/back using tera/actix.

The irony is that the template out "[object]" and trying to make a custom filter I get is instead JSON encoded. So, I say the behaviour now is confusing...