search

sighmon / mjml-rails

MJML + ERb/Haml/Slim view template
https://mjml.io
Other
300 stars 65 forks source link

Raise render errors per default #37

Closed sbiastoch closed 5 years ago

sbiastoch commented 6 years ago

Hi everybody, I do not see any reason why errors during mjml-compilation should be silently ignored? I had an issue where due to a unintended update of jemalloc the compilation of mjml fails silently in production only, which lead to sending blank emails (since mjml created no html output) to my users in production. Also, erroneous templates syntax results in silent fails, which makes it really hard to debug when you are not aware that mjml has disabled error propagation. I am sure that there are scenarios why you want to disable errors, but the default should be the most common case, not an "use-with-caution" setting. Cheers!

sighmon commented 6 years ago

Hi @sbiastoch, looks good to me.

The feature was introduced by @anhtrantuan here: https://github.com/sighmon/mjml-rails/commit/ef469b2d04bea69e225cbc4a772b1e1e67281c5f - so perhaps they might have an opinion.

But if we haven't heard back in a few days I'm happy to merge this.

tzar commented 5 years ago

I too am a victim of the blank emails and would appreciate this being the default!

It might also be worth noting that mjml-rails reads stderr to determine parsing issues, but mjml itself only starts outputting to stderr for versions starting at 4.2.1 ( https://github.com/mjmlio/mjml/issues/1342 )

sighmon commented 5 years ago

@sbiastoch @tzar thanks so much for the feedback - sorry it took so long to merge in.