Does the params.fetch error message match the behaviour?

adbelsham · November 25, 2022, 4:41am

Hi Rails Community,

First time poster, long time Rails user, hoping to be contributing soon. I’m not sure if this has been covered before, I did search though, promise!

In any case, I fixed a bug at work recently, where we were sending a blank '' param value somehow (not super important in the context of this topic), which was then getting through params.fetch without raising an error.

Digging into the rails code, I found this curious as the error message specifically says:

ActionController::ParameterMissing: param is missing or the value is empty: :param_name

Interestingly, this doesn’t raise in our application for empty string, however the behaviour of empty? is to return true for an empty string ‘’.

''.empty?
=> true

Could/should this error message be improved? Or have I got it totally wrong? Look forward to hearing from someone who knows more than me about this!

nikita · November 25, 2022, 11:06pm

It looks like only require checks for both key presence and value being .present?

github.com

rails/rails/blob/36ef88ce9f921ecf48c321d38179836d39a19ff8/actionpack/lib/action_controller/metal/strong_parameters.rb#L515-L520


      
            if value.present? || value == false
              value
            else
              raise ParameterMissing.new(key, @parameters.keys)
            end
          end

While .fetch only cares about the key presence

github.com

Shopify/rails/blob/36ef88ce9f921ecf48c321d38179836d39a19ff8/actionpack/lib/action_controller/metal/strong_parameters.rb#L684-L690


      
          def fetch(key, *args)
            convert_value_to_parameters(
              @parameters.fetch(key) {
                if block_given?
                  yield
                else
                  args.fetch(0) { raise ActionController::ParameterMissing.new(key, @parameters.keys) }

Docs for .fetch don’t mention the presence check

github.com

Shopify/rails/blob/36ef88ce9f921ecf48c321d38179836d39a19ff8/actionpack/lib/action_controller/metal/strong_parameters.rb#L671-L673


      
          # Returns a parameter for the given +key+. If the +key+
          # can't be found, there are several options: With no other arguments,
          # it will raise an <tt>ActionController::ParameterMissing</tt> error;

So assuming both methods behave as designed we may have an opportunity to improve the error message and only have the or the value is empty part if raised from .require Or maybe the message can be even smarter as require should be able to tell whether the value is empty or key is missing

adbelsham · November 27, 2022, 11:12pm

Thanks Nikita, this was what I was thinking as well.

Do you think this is worth me submitting an issue/PR?

nikita · November 30, 2022, 10:48pm

Assuming that the docs for both methods are correct and these methods behave as expected I do believe that the error message will benefit from being a bit more specific