Odd error handling in ActionView#compile_template causes WSOD

If a view file cannot be compiled (eg it has a block with a missing 'end' statement), I'm experiencing WSODs - the browser reports a lost network connection, rather than the helpful compilation error that we used to have.

When the compilation fails, ActionView#compile_template raises this error :

TemplateError.new(find_base_path_for(file_name || template), file_name

template, @assigns, template, e)

...which appears to have a few things wrong with it.

First off, file_name is always absolute here - eg /Users/jon/ Developer/Stacker/app/views/kittens/index.rhtml. find_base_path_for is going to fail since it tries to append that path onto the path from @view_paths - typically /Users/jon/Developer/Stacker/app/views. So, find_base_path_for returns nil. I *think* what happens is that code inside template_error.rb later raises a second exception when it tries to use that path as a string and finds it's nil, and that exception isn't caught properly, so the response is never formed, which produces the WSOD. If I replace it with TemplateError.new(find_base_path_for(file_name || template) || "", file_name || template, @assigns, template, e) the compilation error is reported properly.

The reason I'm posting here rather than submitting a patch to Trac is that I'm struggling to understand a few things about that line of code, and so I'm not sure of the best way to fix it.

'template' is the contents of the template file. So why are we trying to use it as a filename for find_base_path_for()? Is it supposed to be find_base_path_for(file_name) || template ? If so, why are we creating a TemplateError with base_path=<file contents> ? Same applies to the second parameter - if file_name is nil, use the template content?? Why is find_base_path_for being used at all? Why not File.dirname ?

Thanks for any help figuring this out... Jon

Is it only me or has the template lookup code in Rails become an unmanagable-unrefactorable pile of gunk? (blantly put)

Apparently this is the result of some incorrectly transcribed code when I committed the multiple viewpath support.

BEFORE raise TemplateError.new(@base_path, file_name || template, @assigns, template, e)

AFTER raise TemplateError.new(lookup_template_base_path_for(file_name || template), file_name || template, @assigns, template, e)

The reason we have to call #find_base_path_for is TemplateError wants the current base path (app/views) instead of the path to the template file (app/views/your-controller).

Since file_name is the full path, perhaps we just go through the available base_paths, looking for any that match the beginning of file_name. If none match, then just assume the first base_path (99% of the time you'll only have 1 anyway).

Thanks for bringing this up!

URLs so you folks at home can play along:

BEFORE raise TemplateError.new(@base_path, file_name || template, @assigns, template, e)

http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_view/base.rb?rev=6119#L514

AFTER raise TemplateError.new(lookup_template_base_path_for(file_name || template), file_name || template, @assigns, template, e)

http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_view/base.rb?rev=6120#L514

Its not just you, I've tried to debug view problems before and often get lost in the tangle of code that's present.

My biggest pet peeve is the way erb, builder and rjs handling is hard coded within the system. I'd like to see the view code get refactored so that erb, builder, rjs AND custom template types get handled in the exact same way.

AFTER raise TemplateError.new(lookup_template_base_path_for(file_name || template), file_name || template, @assigns, template, e)

The reason we have to call #find_base_path_for is TemplateError wants the current base path (app/views) instead of the path to the template file (app/views/your-controller).

Since file_name is the full path, perhaps we just go through the available base_paths, looking for any that match the beginning of file_name. If none match, then just assume the first base_path (99% of the time you'll only have 1 anyway).

This should work:

http://dev.rubyonrails.org/changeset/6505