Add note on avoiding DoubleRenderError in Rails to Readme #53

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@thomasklemm
Collaborator

When a Pundit::NotAuthorizedError is raised in an after_action (Rails 4)/ after_filter (Rails 3), the response has already been rendered.

To avoid a DoubleRenderError being raised, the response body can be cleared before redirecting or rendering a new view.

Tested only in the browser in Rails 4.0. Fixes #52.

Please add a comment if you know another solution so we can work it in.

@thomasklemm
Collaborator

If this works in Rails 4 and 3, should we incorporate that fix in the verify_authorized filter?

@jnicklas
Collaborator

I think the use case where this matters is a bit slim. You probably want to do authorization before any action takes place, thus it doesn't really make sense to authorize in an after filter, or after any rendering at all really. In the case of errors raised due to authorization not occuring, as in verify_authorized, we want those errors to bubble, because that should never happen in a production setting.

@thomasklemm
Collaborator

True, that note would only be useful in development. Let's leave it at the discussion here as a reference if someone googles the reason for the DoubleRenderError.

@ghost
ghost commented Aug 26, 2013

👍

@thomasklemm
Collaborator

@BigNerdRanchDan Ok with that? The DoubleRenderError might look a little confusing when it happens at first, but I hope people will find the missing authorize call (or missing skip statement for the verify_authorized filter) quickly.

@ghost
ghost commented Aug 26, 2013

I think that @jnicklas is correct. Me personally, I would rather have the gem enforce authorization even in production so that I can capture an authorization exception and handle it with the rescue_from block. What if I want to display 404, not 500, when an authorization is intercepted (the same way Github works)?

That's said, you shouldn't concern the Pundit gem itself with Rails anyways (its a pure Ruby library to me). Perhaps a better solution is to extract some of the Rails specific code to a gem called "pundit-rails". In that gem we can experimemt with different ways of integrating Pundit into Rails, without polluting the core gem. Does that sound more reasonable?

@thomasklemm
Collaborator

@BigNerdRanchDan A pundit-rails gem doesn't sound unreasonable, but I don't think this issue is important enough to justify the extra effort. When there's more Rails-specific stuff we can come back and add in this error handling as a standard, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment