klein / klein.php

A fast & flexible router
MIT License
2.66k stars 290 forks source link

Handing maintenance #280

Closed Aristona closed 9 years ago

Aristona commented 9 years ago

Hi,

I needed a global maintenance handler so I tried the following:

$klein->respond(function ($req, $res) {
    if(file_exists(__DIR__ . '/../storage/maintenance'))
    {
        return $res->code(500);
    }
});

However, it still kept returning the different headers. (404)

I tried doing:

class MaintenanceException extends Exception {}
$klein->respond(function ($req, $res) {
    if(file_exists(__DIR__ . '/../storage/maintenance'))
    {
        throw new MaintenanceException;
    }
});

try {
    $klein->dispatch();
}
catch(MaintenanceException $e)
{
     dd("maintenance");
}
catch(Exception $e)
{
    $klein->app()->bugsnag->notifyError('ErrorType', $e->getMessage());
    $klein->response()->header('Access-Control-Allow-Origin', '*');
    $klein->response()->header('Access-Control-Allow-Methods', 'GET');
    $klein->response()->code(400);
    return $klein->response()->json(['message' => $e->getMessage()]);
}

The catch(MaintenanceException $e) block is never reached. (It goes into Exception catch block)

This is my onHttpError callback:

$klein->onHttpError(function ($code, $router) {
    switch ($code) {
        case 404:
            $router->response()->code($code);
            $router->response()->json(['message' => 'Requested endpoint not found.']);
            break;
        default:
            $router->response()->code($code);
            $router->response()->json(['message' => 'Oh no, a bad error happened that caused a '. $code]);
    }
});

What I want to do is:

"If a file called maintenance exists, then respond 503 headers along with {"message": "Maintenance"}, and simply stop."

I've checked the API but couldn't find anything else. (Tried locking response, looked for a http error trigger method) The behaviour seemed a little weird. Am I overlooking something?

Rican7 commented 9 years ago

Ah, yea, sorry. There's a few things going on here... all of which should definitely be documented better:

  1. Your global maintenance handler is a "catch-all" route, which Klein will correctly run, but if it doesn't match any other endpoints, Klein will consider the overall request to not be matched and will return a 404 response. If you'd like to change that behavior, you can simply mark the route as one that should be counted as a match with $route_instance->setCountMatch(true) or $klein->respond(...)->setCountMatch(true);.
  2. Your try/catch block is attempting to catch an exception that's getting wrapped. Klein's designed to allow for a stack of error handlers, all registered by the $klein->onError() method. If no error handler is registered, an UnhandledException is thrown. You can easily catch that exception, if need-be, and then grab the underlying exception using $exception->getPrevious().

If you'd like to simply show a 503 in a maintenance mode, then you could either create a subclass of HttpException (like MaintenanceException) or you could throw the HttpException class itself with a simple message and then handle the exception in the $klein->onHttpError().

Let me know if that works for you. :) Sorry for any confusion!

Aristona commented 9 years ago

Hi, thanks for the indepth explanation. I went with $exception->getPrevious way. Did a instanceof MaintenanceException and returned 503.

catch(Klein\Exceptions\UnhandledException $e)
{
    $exception = $e->getPrevious();

    if($exception instanceof MaintenanceException)
    {
        $klein->response()->code(503);
        return $klein->response()->json(['message' => $exception->getMessage()]);
    }
}

Not the best looking thing but glad it works now.

I could document this if needed, but there are probably better approaches so I'll leave it to somebody who knows core API better than me, unless you want me to add this as-is to readme.

Rican7 commented 9 years ago

I'm glad I could help!

I definitely welcome edits to the wiki. :)