Skip to content

Commit

Permalink
trying to fix the fatal error handling #2
Browse files Browse the repository at this point in the history
  • Loading branch information
oscarotero committed Jun 21, 2017
1 parent 1c9525b commit bb329be
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/Whoops.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Interop\Http\ServerMiddleware\DelegateInterface;
use Whoops\Run;
use Whoops\Util\SystemFacade;
use Whoops\Handler\PrettyPageHandler;
use Whoops\Handler\PlainTextHandler;
use Whoops\Handler\JsonResponseHandler;
Expand Down Expand Up @@ -105,7 +106,20 @@ public function process(ServerRequestInterface $request, DelegateInterface $dele
*/
private static function getWhoopsInstance(ServerRequestInterface $request)
{
$whoops = new Run();
$system = new SystemFacade();
$whoops = new Run($system);

//E_ERROR in PHP 5.x
if (!class_exists('Throwable')) {
$system->registerShutdownFunction(function () use ($whoops) {

This comment has been minimized.

Copy link
@shadowhand

shadowhand Jun 21, 2017

Member

This won't work for us, we rely on dependency-injected Run instance.

This comment has been minimized.

Copy link
@oscarotero

oscarotero Jun 21, 2017

Author Member

Ok. Due the SystemFacade is private and is innaccesible, I can think in some alternatives:

  1. Make the getWhoopsInstance method public, so it can be used by the DI:
$whoops = Middlewares\Whoops::getWhoopsInstance();
$container->set('Whoops\\Run', $whoops);
  1. Allow to provide the SystemFacade as a dependency of the middleware. Example:
$middleware[] = new Middlewares\Whoops($whoops, $system);
  1. Do not use SystemFacade at all and use the register_shutdown_function directly.

Any other idea?

This comment has been minimized.

Copy link
@shadowhand

shadowhand Jun 21, 2017

Member

Option (2) would be ideal from a DI standpoint I think. Option (3) would also work. Option (1) definitely wouldn't work, as we configure Whoops\Run in a very specific way.

$whoops->allowQuit(true);
$whoops->writeToOutput(true);
$whoops->sendHttpCode(true);

$method = Run::SHUTDOWN_HANDLER;
$whoops->$method();
});
}

switch (self::getPreferredFormat($request)) {
case 'json':
Expand Down
14 changes: 14 additions & 0 deletions tests/WhoopsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ function () {
$this->assertNotFalse(strpos($response->getBody(), 'Error Processing Request'));
}

public function testFatalError()
{
$response = Dispatcher::run([
new Whoops(),
function () {
new UnexistingClass();
},
]);

$this->assertEquals(500, $response->getStatusCode());
$this->assertEquals('text/plain', $response->getHeaderLine('Content-Type'));
$this->assertNotFalse(strpos($response->getBody(), "Class 'Middlewares\\Tests\\UnexistingClass' not found"));
}

public function testNotError()
{
$response = Dispatcher::run([
Expand Down

0 comments on commit bb329be

Please sign in to comment.