rails: Unable to rescue_from ActionDispatch::Http::Parameters::ParseError

Steps to reproduce

rails new bugreport
rails g controller home
Add rescue_from in https://github.com/jeffblake/paramsparserbug/blob/master/app/controllers/home_controller.rb
See test case: https://github.com/jeffblake/paramsparserbug/blob/master/test/controllers/home_controller_test.rb

Bug repo here: https://github.com/jeffblake/paramsparserbug

Expected behavior

According to this pull request, https://github.com/rails/rails/pull/34341 rescuing from ActionDispatch::Http::Parameters::ParseError should be possible in the controller.

Actual behavior

ActionDispatch::Http::Parameters::ParseError is not caught by the rescue_from in the controller.

System configuration

Rails version: latest 6.0.2.1

Ruby version: 2.6.5

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 10
Comments: 18 (9 by maintainers)

Commits related to this issue

Fix handling of ParseError in controllers It regressed in #34894, which moved the rescue to the wrong method. `_wrapper_enabled?` can access the parameters and the exception will raise there before e... — committed to rails/rails by rafaelfranca 3 years ago
Fix handling of ParseError in controllers It regressed in #34894, which moved the rescue to the wrong method. `_wrapper_enabled?` can access the parameters and the exception will raise there before e... — committed to rails/rails by rafaelfranca 3 years ago

Most upvoted comments

Now it is fixed and the test take care of making sure this specific problem don’t have regressions.

rafaelfranca on Mar 24, 2021

It was broken by #34894. I’m fixing.

rafaelfranca on Mar 24, 2021

This is currently breaking our Middleware that was supposed to catch the ParseErrors. The exception is no longer raised and just catched by https://github.com/wbotelhos/rails/commit/2cdf756808f43c9ee0ae2b83d4f06e93b8d9e763#diff-3834450d445e54f361a0cd4e9d6814c29d1279ca6dab4e99a0c88855e7689daeR287

In order to fix that and make our Middleware work again, I overrode the method in the ApplicationController:

def _wrapper_enabled?
    return false unless request.has_content_type?

    ref = request.content_mime_type.ref

    _wrapper_formats.include?(ref) && _wrapper_key && !request.parameters.key?(_wrapper_key)
  end

and removed the rescue block. Our middleware is now working again:

class CatchJsonParseErrors
  def initialize(app)
    @app = app
  end

  def call(env)
    @app.call(env)
  rescue ActionDispatch::Http::Parameters::ParseError => e
    Sentry.capture_exception(e)
    [400, { 'Content-Type' => 'application/json' }, [{ error_key: :bad_request }.to_json]]
  end
end

coorasse on Aug 31, 2021

Perhaps someone wants to re-open this issue? It seems that this still persists. Having to override proceed or introduce middleware seems silly considering this was previously acknowledged and addressed?

pfemo on Mar 17, 2021

@rafaelfranca My bug report includes steps to reproduce and a sample repo with tests.

jeffblake on Mar 24, 2021

The special rescue_from handling (introduced in PR #34341) no longer works, but @JoshCheek’s workaround still does.

dan-jensen on Jan 30, 2021

@jeffblake there is an existing workaround: https://github.com/rails/rails/issues/34244#issuecomment-433365579

freaz on Feb 22, 2020