elixir-maru / maru

Elixir RESTful Framework
https://maru.readme.io
BSD 3-Clause "New" or "Revised" License
1.32k stars 84 forks source link

Missing param raises an InvalidFormatter error? #34

Open dblock opened 8 years ago

dblock commented 8 years ago

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 commented 8 years ago
  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 commented 8 years ago

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 commented 8 years ago

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

falood commented 8 years ago

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 commented 8 years ago

2) 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 commented 8 years ago

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 commented 8 years ago

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