How can I avoid double render in this situation?

Question

I have an issue with this code in my controller:

class Api::V1::BaseController < ActionController::API
  include Pundit

  after_action :verify_authorized, except: :index
  after_action :verify_policy_scoped, only: :index

  rescue_from StandardError,                with: :internal_server_error
  rescue_from Pundit::NotAuthorizedError,   with: :user_not_authorized
  rescue_from ActiveRecord::RecordNotFound, with: :not_found

  private

  def user_not_authorized(exception)
    render json: {
      error: "Unauthorized #{exception.policy.class.to_s.underscore.camelize}.#{exception.query}"
    }, status: :unauthorized
  end

  def not_found(exception)
    render json: { error: exception.message }, status: :not_found
  end

  def internal_server_error(exception)
    if Rails.env.development?
      response = { type: exception.class.to_s, message: exception.message, backtrace: exception.backtrace }
    else
      response = { error: "Internal Server Error" }
    end
    render json: response, status: :internal_server_error
  end
end

The problem

The rescue_from StandardError is the root of all my troubles. This controller works nicely and rescues from Pundit whitelist checks when the pundit error is the only one happening.

But as soon as any other error happens alongside pundit, I get a DoubleRenderError since both rescues end up being triggered. I'm looking for a quick tweak that would avoid pundit from triggering when another error has already taken place, or an alternative solution for this problem.

Is there any other error class that I can use to avoid relying so heavily on StandardError?

Things I tried

Adding return after render It doesn't work. I assume the rescue chain interferes with the normal render :x and return behaviour.

Thank you very much!

Christian Bruckmayer · Accepted Answer · 2020-06-23T12:05:27.453

You don't really need to have the rescue_from StandardError as this is the default behaviour of Rails. Rails has a middleware called PublicExceptions which does (mostly) what you want so you can just let the StandardError propagate.

Instead of { error: "Internal Server Error" } it will render this

{ 
  status: status, 
  error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) 
}

which in case of an exception will render { status: 500, error: "Internal Server Error" }. This should be a reasonable compromise.

For development you could think about adapting this middleware. You can set it with config.exceptions_app.

https://guides.rubyonrails.org/configuring.html#rails-general-configuration

https://api.rubyonrails.org/classes/ActionDispatch/PublicExceptions.html

https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/public_exceptions.rb#L14

Thank you very much, man! Also, uncanny stuff having your answer since I marked you RailsConf 2020 talk a few days ago for watching. — Broquel, Jun 23 '20 at 12:39
You're welcome! Brilliant, let me know what you think on Twitter @bruckmayer! — Christian Bruckmayer, Jun 23 '20 at 12:42

score 0 · Answer 2 · answered Jun 23 '20 at 12:36

0

A simple quick fix would be to just use return after your render. This way nothing next will run. So in all your methods at the end just use return:

Ex:

  def user_not_authorized(exception)
    render json: {
      error: "Unauthorized #{exception.policy.class.to_s.underscore.camelize}.#{exception.query}"
   }, status: :unauthorized

    return
  end

answered Jun 23 '20 at 12:36

cdadityang

523
2
10

I should have specified in the question since this was my first instinct as well. It doesn't work tho. I assume the rescue chain interferes with the normal `render :x and return` behaviour. – Broquel Jun 23 '20 at 13:19

How can I avoid double render in this situation?

The problem

Things I tried

2 Answers2