Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing param raises an InvalidFormatter error? #34

Open
dblock opened this issue Aug 17, 2016 · 7 comments
Open

Missing param raises an InvalidFormatter error? #34

dblock opened this issue Aug 17, 2016 · 7 comments

Comments

@dblock
Copy link

dblock commented Aug 17, 2016

A POST request to an API with requires :token yields this without a token:

  1) test requires a token (Aprb.Api.SlackTest)
     test/api/slack_test.exs:13
     ** (Maru.Exceptions.InvalidFormatter) Parsing Param Error: token
     stacktrace:

Seems like there're two problems.

  1. This is not a formatter error, it's some kind of missing or invalid parameter error.
  2. Shouldn't this be handled and turned into a 400 Bad Request error and not raise?
@falood
Copy link
Member

falood commented Aug 18, 2016

  1. I defined all parameter errors as InvalidFormatter. I'll check how grape do this later.
  2. All exceptions will be caught by rescue_from. I think we should mostly catch similar exceptions by rescue_from and return custom error to client. As a result I raise exceptions instead of rendering an error page.
    Plug 1.2.0 released recently and I have a plan to show plug error page by default for :dev environment.

@dblock
Copy link
Author

dblock commented Aug 18, 2016

Grape has error formatters and default handling. It feels like the default for JSON should return a JSON error. This would be much more convenient and logical IMO, there's no user out there who wants a 500 with an HTML page that gives you a call stack and an exception when you supply an invalid parameter for example.

@dblock
Copy link
Author

dblock commented Aug 18, 2016

InvalidFormatter is just a poorly worded name I think. It says "the formatter is invalid", probably meant to say "the format is invalid"?

@falood
Copy link
Member

falood commented Aug 18, 2016

So the only thing I should do is change name and printed message of the exception module?
For example:

 ** (Maru.Exceptions.InvalidFormat) :token is required.

or

 ** (Maru.Exceptions.InvalidFormat) :age is illegal.

@falood
Copy link
Member

falood commented Aug 18, 2016

  1. Shouldn't this be handled and turned into a 400 Bad Request error and not raise?

I'll think more about how to handle it by default.

@dblock
Copy link
Author

dblock commented Aug 18, 2016

Yes on the exception name it would be a start.

I would namespace the parameter exceptions, maybe Maru.Exceptions.Validations.InvalidFormat and such? And they all would inherit form Maru.Exceptions.Validations.ValidationError so that they can be caught together?

@falood
Copy link
Member

falood commented Aug 18, 2016

There's no inherit in elixir world, I must remake namespace in elixir way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants