Skip to content

Commit

Permalink
Modify Handler to call previously defined handlers
Browse files Browse the repository at this point in the history
Both set_exception_handler and set_error_handler return the values to
which they were previously set. Be respectful and call these handlers
as it may introduce unexpected behavior in the user's code and also
provides no way to attach an error handler besides manually wrapping
the bugsnag library.

PHPUnit's default behavior is to translate PHP errors into exceptions
and raise those. Rely on this assumption in our tests of the error
handler.

Ref: https://phpunit.de/manual/current/en/writing-tests-for-phpunit.html
  • Loading branch information
Matthew Terwilliger committed Aug 2, 2017
1 parent 8f00f8b commit 92d05e3
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 11 deletions.
98 changes: 87 additions & 11 deletions src/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,85 @@ class Handler
*/
protected $client;

/**
* The previously registered error handler as returned by the interpreter.
*
* @var callable|null
*/
protected $previousErrorHandler;

/**
* The previously registered exception handler as returned by the interpreter.
*
* @var callable|null
*/
protected $previousExceptionHandler;

/**
* Register our exception handler.
*
* @param \Bugsnag\Client|string|null $client client instance or api key
* @param bool $callPrevious whether or not to call the previous handlers
*
* @return static
*/
public static function register($client = null)
public static function register($client = null, $callPrevious = true)
{
$handler = new static($client instanceof Client ? $client : Client::make($client));

set_error_handler([$handler, 'errorHandler']);
set_exception_handler([$handler, 'exceptionHandler']);
register_shutdown_function([$handler, 'shutdownHandler']);
$handler->registerErrorHandler($callPrevious);
$handler->registerExceptionHandler($callPrevious);
$handler->registerShutdownHandler();

return $handler;
}

/**
* Register the bugsnag error handler and save the previous one
* (if it exists) to call later.
*
* @param bool $callPrevious whether or not to call the previous handler
*
* @return void
*/
public function registerErrorHandler($callPrevious = true)
{
$previous = set_error_handler([$this, 'errorHandler']);

if ($callPrevious) {
$this->previousErrorHandler = $previous;
}
}

/**
* Register the bugsnag exception handler and save the previous one
* (if it exists) to call later.
*
* @param bool $callPrevious whether or not to call the previous handler
*
* @return void
*/
public function registerExceptionHandler($callPrevious = true)
{
$previous = set_exception_handler([$this, 'exceptionHandler']);

if ($callPrevious) {
$this->previousExceptionHandler = $previous;
}
}

/**
* Register our shutdown handler.
*
* PHP will call shutdown functions in the order they were registered.
*
* @return void
*/
public function registerShutdownHandler()
{
register_shutdown_function([$this, 'shutdownHandler']);
}

/**
* Create a new exception handler instance.
*
Expand All @@ -55,6 +116,13 @@ public function exceptionHandler($throwable)
$report->setSeverity('error');

$this->client->notify($report);

if ($this->previousExceptionHandler) {
call_user_func(
$this->previousExceptionHandler,
$throwable
);
}
}

/**
Expand All @@ -69,15 +137,23 @@ public function exceptionHandler($throwable)
*/
public function errorHandler($errno, $errstr, $errfile = '', $errline = 0)
{
if ($this->client->shouldIgnoreErrorCode($errno)) {
return;
}
if (!$this->client->shouldIgnoreErrorCode($errno)) {
$report = Report::fromPHPError($this->client->getConfig(), $errno, $errstr, $errfile, $errline);

$report = Report::fromPHPError($this->client->getConfig(), $errno, $errstr, $errfile, $errline);

$this->client->notify($report);
$this->client->notify($report);
}

return false;
if ($this->previousErrorHandler) {
call_user_func(
$this->previousErrorHandler,
$errno,
$errstr,
$errfile,
$errline
);
} else {
return false;
}
}

/**
Expand Down
30 changes: 30 additions & 0 deletions tests/HandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ public function testErrorHandler()
Handler::register($this->client)->errorHandler(E_WARNING, 'Something broke', 'somefile.php', 123);
}

public function testErrorHandlerPreviousOff()
{
// PHPUnit's error handler should not get called, so no exception should be raised.
$exception_raised = false;
try {
Handler::register($this->client, false)->errorHandler(E_WARNING, 'Something broke', 'somefile.php', 123);
} catch (\PHPUnit_Framework_Error_Warning $e) {
$exception_raised = true;
}

$this->assertFalse($exception_raised);
}

public function testExceptionHandler()
{
$this->client->expects($this->once())->method('notify');
Expand All @@ -56,8 +69,23 @@ function ($e) use (&$previous_exception_handler_arg) {
$this->assertSame($e_to_throw, $previous_exception_handler_arg);
}

public function testExceptionHandlerCallsPreviousOff()
{
$previous_exception_handler_called = false;
set_exception_handler(
function ($e) use (&$previous_exception_handler_called) {
$previous_exception_handler_called = true;
}
);

Handler::register($this->client, false)->exceptionHandler(new Exception());

$this->assertFalse($previous_exception_handler_called);
}

public function testErrorReportingLevel()
{
$this->expectException(\PHPUnit_Framework_Error_Notice::class);
$this->client->expects($this->once())->method('notify');

$this->client->setErrorReportingLevel(E_NOTICE);
Expand All @@ -67,6 +95,7 @@ public function testErrorReportingLevel()

public function testErrorReportingLevelFails()
{
$this->expectException(\PHPUnit_Framework_Error_Warning::class);
$this->client->expects($this->never())->method('notify');

$this->client->setErrorReportingLevel(E_NOTICE);
Expand All @@ -76,6 +105,7 @@ public function testErrorReportingLevelFails()

public function testErrorReportingWithoutNotice()
{
$this->expectException(\PHPUnit_Framework_Error_Notice::class);
$this->client->expects($this->never())->method('notify');

$this->client->setErrorReportingLevel(E_ALL & ~E_NOTICE);
Expand Down

0 comments on commit 92d05e3

Please sign in to comment.