skipperbent / simple-php-router

Simple, fast and yet powerful PHP router that is easy to get integrated and in any project. Heavily inspired by the way Laravel handles routing, with both simplicity and expand-ability in mind.
631 stars 116 forks source link

Incorrect status code returned when catching an error using the SimpleRouter::error() function #677

Open Burial0268 opened 1 year ago

Burial0268 commented 1 year ago

As you can see, when I use SimpleRouter::error() in conjunction with Request's setRewriteCallback() method to catch an error route, instead of getting an HTTP status code of 4XX or 5XX, I get 200.

Here is my code:

use Pecee\Http\Request;

SimpleRouter::error(function (Request $request, \Exception $e) {
    $request->setRewriteCallback(function () use ($e) {
         return (new ErrorManager($e))->handle(); 
    }); 
});

Here return (new ErrorManager($e))->handle(); returns a normal rendered page.

Tried solution: add SimpleRouter::response()->httpCode($e->getCode()); statement

After adding the SimpleRouter::response()->httpCode($e->getCode()); statement, the status code is correct, but the default Nginx interface is displayed instead of the error page that I set.

It seems that all sites that use this Router(include official demo) have this problem.

If you need a more detailed code, reply here.

ms-afk commented 10 months ago

In the example above you get a 200 HTTP error because, inside of your error handler, you are just rendering a page through the ErrorManager object and you are never telling php or the router to send a custom HTTP code in the response's header (or at least this is what I think you are [not] doing, given that I don't have the code of ErrorManager). So the default HTTP code sent by php/SimpleRouter will be the 200 code. On the other hand, in the second part of your question you are instructing the router to use a different HTTP code in the response, and that is working as well, so no issue here.

The problem you are talking about in the end is caused by your nginx configuration, not by this library. Try looking here, it seems to be a similar issue to yours: https://stackoverflow.com/questions/59495015/index-html-should-return-404-but-shows-nginx-welcome-page-instead.

Burial0268 commented 2 months ago
use Pecee\Http\Request;

SimpleRouter::error(function(Request $request, \Exception $exception) {
    response()->httpCode($exception->getCode());
});
return (new ErrorManager($exception))->handle();

If follow your view, the code would theoretically work, but it would only end up returning the status code and the page would not be rendered correctly

ms-afk commented 2 months ago

You have to both set the error and to return the page inside of the function. Starting from the code you posted in the first post:

use Pecee\SimpleRouter\SimpleRouter;
use Pecee\Http\Request;

SimpleRouter::error(function (Request $request, \Exception $e) {
    $request->setRewriteCallback(function () use ($e) {
        SimpleRouter::response()->httpCode(404); //set the http code first
        return (new ErrorManager($e))->handle(); //return the page second
    }); 
});