abedev / abe

API for Back-End (Haxe, NodeJS and Express)
54 stars 10 forks source link

Error Handler does not work on 404 #87

Open TheoSabattie opened 6 years ago

TheoSabattie commented 6 years ago

Hello',

I am trying to return a json message on error. There is no problem when I use defined route and next.error method. But when that is not a defined route, the 404 response return an html text. :(

Test :

class Main 
{    
    static function main() 
    {
        var app = new App();

        app.router.register(new ErrorMaker());
        app.error(ErrorHandler.handle(JsonMessage));

        app.http(8443);
    }
}

class ErrorMaker implements IRoute
{
    @:get("/")
    function index() {
        next.error(new Error("Something wrong happened"));
    }
}

http://myUrl/ => JSON http://myUrl/something => HTML

I assumed that come from l:373 from AutoRegisterRoute.hx but I don't understand very well the macro part of haxe for the moment.

Does someone will be able to fix that?

There is also an other issue related to the same piece of macro: https://github.com/abedev/abe/issues/55

Thanks in advance,

Théo Sabattié

fponticelli commented 6 years ago

Assuming that JSON is a good default for 404 is probably not a good strategy. I do not recall the details but content negotiation should be done based on the Accept header. If that is not the case (Might very well not be the case) I suggest to add a catch-all route at the end of your router list and use that to return the desired JSON structure.

TheoSabattie commented 6 years ago

Your patch works, thanks.

I'm ok with you and I tried to get a json by passing an Accept header with application/json but the response was a html/text. app.error should catch and handle 404 and if there is no error handler, macro must return data type relative to Accept header.