Apiepie::ParamErrors Inconsistent interface

[wozza35]

It is suggested in the documentation to rescue both ParamMissing and ParamInvalid exceptions by rescuing from the Apipie::ParamError superclass. However, the interface of these two sub-classes is different.

For example, I'm trying to return a custom error JSON message in the case of a param error:

rescue_from Apipie::ParamError do |e|
  render json: {e.param => 'has an error'}, status: :unprocessable_entity
end

api :GET, '/foo'
param :arg, Array, required: true
def foo
  ...

ParamMissing#param returns an instance of Apipie::ParamDescription, whereas ParamInvalid#param returns a symbol. So, If the parameter is not an Array:

{
"arg": "has an error"
}

If the parameter is missing:

{
"ParamDescription: site#foo#arg": "has an error"
}

I realize I can rescue the error classes separately or make use of meta programming, but it seems like this might be unintentional behaviour?

[mathieujobin]

I am thinking this might have happened due to a refactor. probably best to update the documentation, unless we're really missing out on something...

[davidwessman]

Might be related to my change with returning multiple errors, so either we need to reconsider that change or update the documentation.

#886

[mathieujobin]

@davidwessman Can you update the documentation ?

[davidwessman]

EDIT Now I realized the point about the different interfaces of param for ParamMissing vs ParamInvalid.
So my answer was probably not on point - sorry.

Previous answer

@mathieujobin I reviewed the README and find that it has been updated to this syntax: ```ruby # ParamError is superclass of ParamMissing, ParamInvalid rescue_from Apipie::ParamError do |e| render text: e.message, status: :unprocessable_entity end ```

using e.message instead of e.param.

---

@wozza35 Where did you find the e.param example?

---

This is my suggestion to handle all the errors, but I am not sure if it should be in the README or not.

rescue_from Apipie::ParamError do |e|
  errors = if e.is_a?(Apipie::ParamMultipleMissing)
    e.params.to_h {|p| [p.name, "has an error"]}
  else
    {e.param => "has an error"}
  end
  
  render json: errors, status: :unprocessable_entity
end

[davidwessman]

So it seems like these errors have always used different interfaces for the params.

ParamInvalid is initialized like:

apipie-rails/lib/apipie/validator.rb

Lines 75 to 77 in 7cc859e

	def error
	ParamInvalid.new(param_name, @error_value, description)
	end

ParamMissing is initialized like:

apipie-rails/lib/apipie/validator.rb

Lines 41 to 49 in 7cc859e

	def self.raise_if_missing_params
	missing_params = []
	yield missing_params
	if missing_params.size > 1
	raise ParamMultipleMissing.new(missing_params)
	elsif missing_params.size == 1
	raise ParamMissing.new(missing_params.first)
	end
	end

apipie-rails/lib/apipie/validator.rb

Lines 359 to 368 in 7cc859e

	BaseValidator.raise_if_missing_params do |missing|
	@hash_params&.each do |k, p|
	if Apipie.configuration.validate_presence?
	missing << p if p.required && !value.key?(k)
	end
	if Apipie.configuration.validate_value?
	p.validate(value[k]) if value.key?(k)
	end
	end
	end

So it seems like it would make sense the change the ParamInvalid error to receive the whole ParamDescription as well.
But that seems like breaking change :/

[mathieujobin]

Inconsistent interface isn't ideal, I convey, but considering the little audience of this gem. I'm not sure its worth fixing.

I guess if enough people vote that its worth it. otherwise, I think this is really minor. or at least, I don't see what exact problem is causes.

[baarde]

Hello,

Thanks for your work. I've recently run into this issue. For me the problem it causes is the following:

I'm working on an API that accepts nested params and rescue ParamError to return a detailed error to the user. I'd like to include the source attribute described in the JSON API spec. When param is a ParamDescription I can used the parent attribute to reconstruct the parameter's full path. But that's not possible when param is a String.

I agree this is a minor issue and I do have a workaround (implementing custom wrapper validators to throw errors with the ParamDescription). But it's slightly annoying.

Suggestion

Here is my suggestion to implement the change in a (more or less) non-breaking way:

• ParamError may be constructed with either a ParamDescription or a String.
• Errors thrown by Apipie always use a ParamDescription.
• The param attribute is:
  1. Always a String.
  2. Always a ParamDescription or nil.
  3. Always the value that was passed to the initializer (so probably a ParamDescription).
  4. A ParamDescription for ParamMissing, a String otherwise (ugly but 100% non-breaking).
• We add the param_name (except in case i.) and param_description (except in case ii.) attributes to ParamError.

I'd be happy to submit a PR if one of the proposed options is acceptable.

[mathieujobin]

@PanosCodes Do you have an opinion on the above?

[davidwessman]

@baarde I think it makes sense, but maybe it would be easier to see some code examples and some tests? Would be great if you could draft something up 🙂