Ensure render_to_string cleans up after itself when an exception is raised. Closes #6658. Great tests!

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5591 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 52 insertions, 4 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 506ad30822..ed13fa1359 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Ensure render_to_string cleans up after itself when an exception is raised.  #6658 [rsanheim]
+
 * Extract template_changed_since? from compile_template? so plugins may override its behavior for non-file-based templates.  #6651 [Jeff Barczewski]
 
 * Update to Prototype and script.aculo.us [5579]. [Thomas Fuchs]
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index f477c9d587..9b4b729e8d 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -763,13 +763,11 @@ module ActionController #:nodoc:
       # Renders according to the same rules as <tt>render</tt>, but returns the result in a string instead
       # of sending it as the response body to the browser.
       def render_to_string(options = nil, &block) #:doc:
-        result = ActiveSupport::Deprecation.silence { render(options, &block) }
-
+        ActiveSupport::Deprecation.silence { render(options, &block) }
+      ensure
         erase_render_results
         forget_variables_added_to_assigns
         reset_variables_added_to_assigns
-
-        result
       end
 
       def render_action(action_name, status = nil, with_layout = true) #:nodoc:
diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb
index 16709b4ea6..6e1ec3417a 100644
--- a/actionpack/test/controller/new_render_test.rb
+++ b/actionpack/test/controller/new_render_test.rb
@@ -160,6 +160,27 @@ class NewRenderTestController < ActionController::Base
     @customers = [ Customer.new("david"), Customer.new("mary") ]
     render :text =>  "How's there? #{render_to_string("test/list")}"
   end
+  
+  def render_to_string_with_assigns
+    @before = "i'm before the render"
+    render_to_string :text => "foo"
+    @after = "i'm after the render"
+    render :action => "test/hello_world"
+  end
+  
+  def render_to_string_with_exception
+    render_to_string :file => "exception that will not be caught - this will certainly not work", :use_full_path => true
+  end
+  
+  def render_to_string_with_caught_exception
+    @before = "i'm before the render"
+    begin
+      render_to_string :file => "exception that will be caught- hope my future instance vars still work!", :use_full_path => true
+    rescue
+    end
+    @after = "i'm after the render"
+    render :action => "test/hello_world"
+  end
 
   def accessing_params_in_template
     render :inline =>  "Hello: <%= params[:name] %>"
@@ -187,6 +208,11 @@ class NewRenderTestController < ActionController::Base
     render :text => "hello"
     redirect_to :action => "double_render"
   end
+  
+  def render_to_string_and_render
+    @stuff = render_to_string :text => "here is some cached stuff"
+    render :text => "Hi web users! #{@stuff}"
+  end
 
   def rendering_with_conflicting_local_vars
     @name = "David"
@@ -523,6 +549,22 @@ EOS
     assert_not_deprecated { get :hello_in_a_string }
     assert_equal "How's there? goodbyeHello: davidHello: marygoodbye\n", @response.body
   end
+  
+  def test_render_to_string_doesnt_break_assigns
+    get :render_to_string_with_assigns
+    assert_equal "i'm before the render", assigns(:before)
+    assert_equal "i'm after the render", assigns(:after)
+  end
+  
+  def test_bad_render_to_string_still_throws_exception
+    assert_raises(ActionController::MissingTemplate) { get :render_to_string_with_exception }
+  end
+  
+  def test_render_to_string_that_throws_caught_exception_doesnt_break_assigns
+    assert_nothing_raised { get :render_to_string_with_caught_exception }
+    assert_equal "i'm before the render", assigns(:before)
+    assert_equal "i'm after the render", assigns(:after)
+  end
 
   def test_nested_rendering
     get :hello_world
@@ -555,6 +597,12 @@ EOS
   def test_render_and_redirect
     assert_raises(ActionController::DoubleRenderError) { get :render_and_redirect }
   end
+  
+  # specify the one exception to double render rule - render_to_string followed by render
+  def test_render_to_string_and_render
+    get :render_to_string_and_render
+    assert_equal("Hi web users! here is some cached stuff", @response.body)
+  end
 
   def test_rendering_with_conflicting_local_vars
     get :rendering_with_conflicting_local_vars