Merge pull request #240 from heartcombo/ca-config-errors

Make error status for HTML/JS responses fully configurable

CHANGELOG.md

@@ -1,7 +1,7 @@
 ## Unreleased
 
 * Add config `responders.redirect_status` to allow overriding the redirect code/status used in redirects. The default is `302 Found`, which matches Rails, but it allows to change responders to redirect with `303 See Other` for example, to make it more compatible with how Hotwire/Turbo expects redirects to work.
-* Responding to an `HTML` or `JS` request that has errors on the resource now sets the status to `422 Unprocessable Entity`. (instead of the default of `200 OK`.) This makes it more consistent with other statuses more commonly used in APIs (JSON/XML for example), and works by default with Turbo/Hotwire which expects a 422 on form error HTML responses. Note that this change may break your application if you're relying on the previous 2xx status to handle error cases.
+* Add config `responders.error_status` to allow overriding the status code used to respond to `HTML` or `JS` requests that have errors on the resource. The default is `200 OK`, but it allows to change the response to be `422 Unprocessable Entity` in such cases for example, which makes it more consistent with other statuses more commonly used in APIs (like JSON/XML), and works by default with Turbo/Hotwire which expects a 422 on form error HTML responses. Note that changing this may break your application if you're relying on the previous 2xx status to handle error cases.
 * Add support for Ruby 3.0, 3.1, and 3.2, drop support for Ruby < 2.5.
 * Add support for Rails 6.1 and 7.0, drop support for Rails < 5.2.
 * Move CI to GitHub Actions.

README.md

@@ -213,7 +213,7 @@ def create
  @widget = Widget.new(widget_params)
  @widget.errors.add(:base, :invalid)
  # `respond_with` will render the `new` template again,
- # and set the status to `422 Unprocessable Entity`.
+ # and set the status based on the configured `error_status`.
  respond_with @widget
 end
 ```
@@ -240,16 +240,35 @@ class WidgetsController < ApplicationController
 end
 ```
 
-## Configuring redirect statuses
+## Configuring error and redirect statuses
 
-By default, `respond_with` will perform redirects using the HTTP status code `302 Found`.
+By default, `respond_with` will respond to errors on `HTML` & `JS` requests using the HTTP status code `200 OK`,
+and perform redirects using the HTTP status code `302 Found`, both for backwards compatibility reasons.
 
-You can configure this behavior by setting `config.responders.redirect_status` to the desired status code.
+You can configure this behavior by setting `config.responders.error_status` and `config.responders.redirect_status` to the desired status codes.
 
 ```ruby
+config.responders.error_status = :unprocessable_entity
 config.responders.redirect_status = :see_other
 ```
 
+These can also be set in your custom `ApplicationResponder` if you have generated one: (see install instructions)
+
+```ruby
+class ApplicationResponder < ActionController::Responder
+ self.error_status = :unprocessable_entity
+ self.redirect_status = :see_other
+end
+```
+
+_Note_: the application responder generated for new apps already configures a different set of defaults: `422 Unprocessable Entity` for errors, and `303 See Other` for redirects. _Responders may change the defaults to match these in a future major release._
+
+### Hotwire/Turbo and fetch APIs
+
+Hotwire/Turbo expects successful redirects after form submissions to respond with HTTP status `303 See Other`, and error responses to be 4xx or 5xx statuses, for example `422 Unprocessable Entity` for displaying form validation errors and `500 Internal Server Error` for other server errors. [Turbo documentation: Redirecting After a Form Submission](https://turbo.hotwired.dev/handbook/drive#redirecting-after-a-form-submission).
+
+The example configuration showed above matches the statuses that better integrate with Hotwire/Turbo.
+
 ## Examples
 
 Want more examples ? Check out these blog posts:

lib/action_controller/respond_with.rb

@@ -96,7 +96,9 @@ def clear_respond_to
  # 2. If there are validation errors, the response
  # renders a default action, which is <tt>:new</tt> for a
  # +post+ request or <tt>:edit</tt> for +patch+ or +put+,
- # and the status is set to <tt>422 Unprocessable Entity</tt>.
+ # and the status is set based on the configured `error_status`.
+ # (defaults to `422 Unprocessable Entity` on new apps,
+ # `200 OK` for compatibility reasons on old apps.)
  # Thus an example like this -
  #
  # respond_to :html, :xml

lib/action_controller/responder.rb

@@ -120,7 +120,9 @@ module ActionController # :nodoc:
  #
  # Using <code>respond_with</code> with a block follows the same syntax as <code>respond_to</code>.
  class Responder
- cattr_accessor :redirect_status, default: :found
+ class_attribute :error_status, default: :ok, instance_writer: false, instance_predicate: false
+ class_attribute :redirect_status, default: :found, instance_writer: false, instance_predicate: false
+
  attr_reader :controller, :request, :format, :resource, :resources, :options
 
  DEFAULT_ACTIONS_FOR_VERBS = {
@@ -238,7 +240,7 @@ def default_render
  if @default_response
  @default_response.call(options)
  elsif !get? && has_errors?
- controller.render({ status: :unprocessable_entity }.merge!(options))
+ controller.render({ status: error_status }.merge!(options))
  else
  controller.render(options)
  end
@@ -266,6 +268,8 @@ def display(resource, given_options = {})
  end
 
  def display_errors
+ # TODO: use `error_status` once we switch the default to be `unprocessable_entity`,
+ # otherwise we'd be changing this behavior here now.
  controller.render format => resource_errors, :status => :unprocessable_entity
  end
 
@@ -307,7 +311,7 @@ def error_rendering_options
  if options[:render]
  options[:render]
  else
- { action: default_action, status: :unprocessable_entity }
+ { action: default_action, status: error_status }
  end
  end
  end

lib/generators/responders/install_generator.rb

@@ -16,6 +16,10 @@ class ApplicationResponder < ActionController::Responder
  # Redirects resources to the collection path (index action) instead
  # of the resource path (show action) for POST/PUT/DELETE requests.
  # include Responders::CollectionResponder
+
+ # Configure default status codes for responding to errors and redirects.
+ self.error_status = :unprocessable_entity
+ self.redirect_status = :see_other
  end
  RUBY
  end

lib/responders.rb

@@ -19,6 +19,7 @@ class Railtie < ::Rails::Railtie
  config.responders = ActiveSupport::OrderedOptions.new
  config.responders.flash_keys = [:notice, :alert]
  config.responders.namespace_lookup = false
+ config.responders.error_status = :ok
  config.responders.redirect_status = :found
 
  # Add load paths straight to I18n, so engines and application can overwrite it.
@@ -28,6 +29,7 @@ class Railtie < ::Rails::Railtie
  initializer "responders.flash_responder" do |app|
  Responders::FlashResponder.flash_keys = app.config.responders.flash_keys
  Responders::FlashResponder.namespace_lookup = app.config.responders.namespace_lookup
+ ActionController::Responder.error_status = app.config.responders.error_status
  ActionController::Responder.redirect_status = app.config.responders.redirect_status
  end
  end

test/action_controller/respond_with_test.rb

@@ -177,13 +177,25 @@ def test_using_resource_with_js_simply_tries_to_render_the_template
  assert_equal 200, @response.status
  end
 
- def test_using_resource_for_post_with_js_renders_the_template_and_yields_unprocessable_entity_on_failure
+ def test_using_resource_for_post_with_js_renders_the_template_on_failure
  @request.accept = "text/javascript"
  errors = { name: :invalid }
  Customer.any_instance.stubs(:errors).returns(errors)
  post :using_resource
  assert_equal "text/javascript", @response.media_type
  assert_equal "alert(\"Hi\");", @response.body
+ assert_equal 200, @response.status
+ end
+
+ def test_using_resource_for_post_with_js_renders_the_template_and_yields_configured_error_status_on_failure
+ @request.accept = "text/javascript"
+ errors = { name: :invalid }
+ Customer.any_instance.stubs(:errors).returns(errors)
+ with_error_status(:unprocessable_entity) do
+ post :using_resource
+ end
+ assert_equal "text/javascript", @response.media_type
+ assert_equal "alert(\"Hi\");", @response.body
  assert_equal 422, @response.status
  end
 
@@ -270,12 +282,26 @@ def test_using_resource_for_post_with_html_redirects_on_success
  end
  end
 
- def test_using_resource_for_post_with_html_rerender_and_yields_unprocessable_entity_on_failure
+ def test_using_resource_for_post_with_html_rerender_on_failure
  with_test_route_set do
  errors = { name: :invalid }
  Customer.any_instance.stubs(:errors).returns(errors)
  post :using_resource
  assert_equal "text/html", @response.media_type
+ assert_equal 200, @response.status
+ assert_equal "New world!\n", @response.body
+ assert_nil @response.location
+ end
+ end
+
+ def test_using_resource_for_post_with_html_rerender_and_yields_configured_error_status_on_failure
+ with_test_route_set do
+ errors = { name: :invalid }
+ Customer.any_instance.stubs(:errors).returns(errors)
+ with_error_status(:unprocessable_entity) do
+ post :using_resource
+ end
+ assert_equal "text/html", @response.media_type
  assert_equal 422, @response.status
  assert_equal "New world!\n", @response.body
  assert_nil @response.location
@@ -330,25 +356,54 @@ def test_using_resource_for_patch_with_html_redirects_on_success
  end
  end
 
- def test_using_resource_for_patch_with_html_rerender_and_yields_unprocessable_entity_on_failure
+ def test_using_resource_for_patch_with_html_rerender_on_failure
  with_test_route_set do
  errors = { name: :invalid }
  Customer.any_instance.stubs(:errors).returns(errors)
  patch :using_resource
  assert_equal "text/html", @response.media_type
+ assert_equal 200, @response.status
+ assert_equal "Edit world!\n", @response.body
+ assert_nil @response.location
+ end
+ end
+
+ def test_using_resource_for_patch_with_html_rerender_and_yields_configured_error_status_on_failure
+ with_test_route_set do
+ errors = { name: :invalid }
+ Customer.any_instance.stubs(:errors).returns(errors)
+ with_error_status(:unprocessable_entity) do
+ patch :using_resource
+ end
+ assert_equal "text/html", @response.media_type
  assert_equal 422, @response.status
  assert_equal "Edit world!\n", @response.body
  assert_nil @response.location
  end
  end
 
- def test_using_resource_for_patch_with_html_rerender_and_yields_unprocessable_entity_on_failure_even_on_method_override
+ def test_using_resource_for_patch_with_html_rerender_on_failure_even_on_method_override
  with_test_route_set do
  errors = { name: :invalid }
  Customer.any_instance.stubs(:errors).returns(errors)
  @request.env["rack.methodoverride.original_method"] = "POST"
  patch :using_resource
  assert_equal "text/html", @response.media_type
+ assert_equal 200, @response.status
+ assert_equal "Edit world!\n", @response.body
+ assert_nil @response.location
+ end
+ end
+
+ def test_using_resource_for_patch_with_html_rerender_and_yields_configured_error_status_on_failure_even_on_method_override
+ with_test_route_set do
+ errors = { name: :invalid }
+ Customer.any_instance.stubs(:errors).returns(errors)
+ @request.env["rack.methodoverride.original_method"] = "POST"
+ with_error_status(:unprocessable_entity) do
+ patch :using_resource
+ end
+ assert_equal "text/html", @response.media_type
  assert_equal 422, @response.status
  assert_equal "Edit world!\n", @response.body
  assert_nil @response.location
@@ -365,26 +420,54 @@ def test_using_resource_for_put_with_html_redirects_on_success
  end
  end
 
- def test_using_resource_for_put_with_html_rerender_and_yields_unprocessable_entity_on_failure
+ def test_using_resource_for_put_with_html_rerender_on_failure
  with_test_route_set do
  errors = { name: :invalid }
  Customer.any_instance.stubs(:errors).returns(errors)
  put :using_resource
+ assert_equal "text/html", @response.media_type
+ assert_equal 200, @response.status
+ assert_equal "Edit world!\n", @response.body
+ assert_nil @response.location
+ end
+ end
 
+ def test_using_resource_for_put_with_html_rerender_and_yields_configured_error_status_on_failure
+ with_test_route_set do
+ errors = { name: :invalid }
+ Customer.any_instance.stubs(:errors).returns(errors)
+ with_error_status(:unprocessable_entity) do
+ put :using_resource
+ end
  assert_equal "text/html", @response.media_type
  assert_equal 422, @response.status
  assert_equal "Edit world!\n", @response.body
  assert_nil @response.location
  end
  end
 
- def test_using_resource_for_put_with_html_rerender_and_yields_unprocessable_entity_on_failure_even_on_method_override
+ def test_using_resource_for_put_with_html_rerender_on_failure_even_on_method_override
  with_test_route_set do
  errors = { name: :invalid }
  Customer.any_instance.stubs(:errors).returns(errors)
  @request.env["rack.methodoverride.original_method"] = "POST"
  put :using_resource
  assert_equal "text/html", @response.media_type
+ assert_equal 200, @response.status
+ assert_equal "Edit world!\n", @response.body
+ assert_nil @response.location
+ end
+ end
+
+ def test_using_resource_for_put_with_html_rerender_and_yields_configured_error_status_on_failure_even_on_method_override
+ with_test_route_set do
+ errors = { name: :invalid }
+ Customer.any_instance.stubs(:errors).returns(errors)
+ @request.env["rack.methodoverride.original_method"] = "POST"
+ with_error_status(:unprocessable_entity) do
+ put :using_resource
+ end
+ assert_equal "text/html", @response.media_type
  assert_equal 422, @response.status
  assert_equal "Edit world!\n", @response.body
  assert_nil @response.location
@@ -688,6 +771,14 @@ def with_test_route_set
  end
  end
 
+ def with_error_status(status)
+ old_status = ActionController::Responder.error_status
+ ActionController::Responder.error_status = status
+ yield
+ ensure
+ ActionController::Responder.error_status = old_status
+ end
+
  def with_redirect_status(status)
  old_status = ActionController::Responder.redirect_status
  ActionController::Responder.redirect_status = status