Error handling for JSON::ParseException not working if request with invalid JSON body

[ucibar]

Hi, I'm using crystal 0.29.0 and lucky 0.15.1.

If I send a POST request with invalid JSON body, lucky response with 500. But there is a handler method for JSON::ParseException in the actions/errors/show.cr file and lucky not trigger the method. And if I delete the handler for JSON::ParseException, It must trigger default handler for unhandled exceptions but It isn't trigger any handler.

Example:
Let's say we have a action named Bug::Create:
lucky gen.action.api Bug::Create

class Bug::Create < ApiAction
  route do
    text "Render something in Bug::Create"
  end
end

And we have error handler for JSON::ParseException:

 def handle_error(error : JSON::ParseException)
    message = "There was a problem parsing the JSON." +
              " Please check that it is formed correctly"

    json Errors::ShowSerializer.new(message), status: 400
  end

And we send a POST request with invalid JSON, e.g. {"foo"="bar"} and we got 500 and
Unhandled exception on HTTP::Handler: Unexpected char '=' at 2:12 (JSON::ParseException).

But if you doing JSON.Parse operations in your actions/forms/models and if there is a invalid JSON, JSON::ParseException perfectly captured by error handler method. For example:

class Bug::Index < ApiAction
  route do
    JSON.parse("{\"foo\"=\"bar\"}") # Invalid JSON
    text "Render something in Bug::Index"
  end
end

And JSON::ParseException will be captured by error handler.

Sorry for bad English and language mistakes.

[ucibar]

I put ErrorHandler before the HttpMethodOverrideHandler in my src/app_server.cr after #854 and its worked. But I think it's should be default behavior when lucky-cli generate a project.

[jwoertink]

Yeah, I'm thinking as long as HttpMethodOverrideHandler is before the RouterHandler, it should be ok to move it. We'll probably need to test a few things, but good catch!

[paulcsmith]

Sorry to chime in late. I think HttpMethodOverrideHandler should be before ErrorHandler because we need to make sure the http method is correct. I think what we should do is rescue json parse exceptions in the method override handler or something. Bit busy right now, but will try to tackle this later

[paulcsmith]

and thank you for reporting this @ucibar and for investigating the cause. That was extremely helpful!

[paulcsmith]

Closed by ed1e51b