Allow to set custom error handler for ActionDispatch::ParamsParser.

[szimek]

This allows to set a custom response (e.g. 400) when ParamsParser raises an exception when parsing request params. One can do it by swapping the default ActionDispatch::ParamsParser:

config.middleware.swap ActionDispatch::ParamsParser, 
                       ActionDispatch::ParamsParser, {}, lambda { |e| [400, {}, ["Bad request"]]}

This is still a bit awkward, so it could be further simplified to e.g.

config.action_dispatch.params_parser_error_handler = lambda { ... }

but it's not implemented in this patch.

Without this patch it's of course still possible to return custom response, however slightly more difficult. Here's one of possible solutions:

class MyParamsParser < ActionDispatch::ParamsParser
  def call
    super
  rescue MultiJson::DecodeError
    [400, {}, ["Bad Request"]]
  end
end

config.middleware.swap ActionDispatch::ParamsParser, MyParamsParser

The other question is why Rails doesn't return 400 by default instead of 500 when parsing request params fails :)

[rafaelfranca]

cc/ @josevalim @spastorino

[tenderlove]

The subclass seems better. I don't want to add more configuration values, and adding a special lambda to call in the case of an exception seems strange. Regular OO via composition or subclassing seems better.

As for 400 vs 500, I don't have a good answer. I'm going to guess that it's because this thing rescues and Exception, and we don't know if that was because of parameter parsing failing, or some other reason.

[rafaelfranca]

I agree with @tenderlove about the subclassing.

[szimek]

Yeah, I guess you're right - subclassing allows you to explicitly select which exceptions should result in custom response - e.g. only MultiJson::DecodeError or some strange exception from a custom parser.

[szimek]

@rafaelfranca @tenderlove Unfortunately it's not so simple and my subclassing example in the first post is flawed.

It overrides call which doesn't have any idea if parsing exception was raised by parsing params in ParamsParser or somewhere later down the chain. My subclassing example would return 400 response even if parsing request params would succeed, but later in an action you'd make a request to external service and fail parsing an invalid response from it.

You also can't override ParamsParser#parse_formatted_parameters and return a custom response from that, because result of this method is assigned to params variable (https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/params_parser.rb#L16-22).

It looks like currently many people are inserting custom middleware before ParamsParser that's only responsible for parsing request params and returning custom response if it fails, but it actually results in parsing request params twice if everything is ok.

[tenderlove]

@szimek I think we should be raising a special exception in the case of parse parameters failing. Can you make that happen?

[szimek]

@tenderlove Sure, I'll prepare new patch tomorrow. Just to make sure we're on the same page: instead of re-raising exception at params_parser.rb#L57, it should raise there a custom one e.g. ParamsParser::ParsingError, right?

[tenderlove]

@szimek yup, exactly. I'm going to close this. Send a new PR when you get that working. Thanks!