Herb introduces risk of production runtime exceptions

[joelhawksley]

As we continue to get closer to running Herb in production on GitHub.com, we've identified an operational risk in the library.

When turning intercept_erb, it's possible to introduce runtime exceptions if Herb/ReActionView produces invalid Ruby, such as in marcoroth/reactionview#83. If a template is not rendered in tests, the invalid Ruby will not be eval'd until the template is lazily compiled in production.

As a stopgap, I have added a script to compile and eval all of our templates in a CI job, but I would expect Herb to provide a better safety mechanism.

[marcoroth]

Thanks for raising this, this is a fair concern!

Technically speaking, this is a "flaw" in the way ActionView compiles templates, and could also happen with erubi, haml, or slim today. That said, herb is newer and hasn't been around for that long, compared to these other gems, which is why it's more likely to happen.

The good news is that this is something I want to tackle and to ideally improve in upstream ActionView when we get there (also see #655). I think it's also worth mentioning actionview_precompiler as a source of inspiration.

But yes, it's still a valid concern and something we should think about. It's also the reason why I recently updated herb analyze to try to eagerly render all templates and make sure they are valid, and if not, that they let the build fail, so that you don't accidentally deploy invalid templates (see #1259).

With all of that out of the way, we are planning on releasing v0.9 this week with a lot of fixes (and the improved analyze command)!

[joelhawksley]

Agreed, this is ultimately an upstream concern.

I've started the first step of getting pre-compilation into core here: rails/rails#56929. I could also see an intermediate step of pre-compiling all templates to Ruby at boot time, but not defining the template methods on the compiled method container until runtime.