Skip to content

Commit

Permalink
Merge pull request #1851 from brefphp/harden-error-handling
Browse files Browse the repository at this point in the history 
Harden error handling for errors in event listeners
  • Loading branch information
@mnapoli
mnapoli authored Aug 14, 2024
2 parents 4335077 + 515f62d commit 5c173d7
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 35 deletions.
90 changes: 55 additions & 35 deletions src/Runtime/LambdaRuntime.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,33 @@ public function processNextEvent(Handler | RequestHandlerInterface | callable $h
// Expose the context in an environment variable
$this->setEnv('LAMBDA_INVOCATION_CONTEXT', json_encode($context, JSON_THROW_ON_ERROR));

Bref::triggerHooks('beforeInvoke');
Bref::events()->beforeInvoke($handler, $event, $context);
try {
Bref::triggerHooks('beforeInvoke');
Bref::events()->beforeInvoke($handler, $event, $context);

$this->ping();
$this->ping();

try {
$result = $this->invoker->invoke($handler, $event, $context);

$this->sendResponse($context->getAwsRequestId(), $result);

Bref::events()->afterInvoke($handler, $event, $context, $result);
} catch (Throwable $e) {
$this->signalFailure($context->getAwsRequestId(), $e);

Bref::events()->afterInvoke($handler, $event, $context, null, $e);
try {
Bref::events()->afterInvoke($handler, $event, $context, null, $e);
} catch (Throwable $e) {
$this->logError($e, $context->getAwsRequestId());
}

return false;
}

// Any error in the afterInvoke hook happens after the response has been sent,
// we can no longer mark the invocation as failed. Instead we log the error.
try {
Bref::events()->afterInvoke($handler, $event, $context, $result);
} catch (Throwable $e) {
$this->logError($e, $context->getAwsRequestId());

return false;
}
Expand Down Expand Up @@ -195,33 +207,7 @@ private function sendResponse(string $invocationId, mixed $responseData): void
*/
private function signalFailure(string $invocationId, Throwable $error): void
{
$stackTraceAsArray = explode(PHP_EOL, $error->getTraceAsString());
$errorFormatted = [
'errorType' => get_class($error),
'errorMessage' => $error->getMessage(),
'stack' => $stackTraceAsArray,
];

if ($error->getPrevious() !== null) {
$previousError = $error;
$previousErrors = [];
do {
$previousError = $previousError->getPrevious();
$previousErrors[] = [
'errorType' => get_class($previousError),
'errorMessage' => $previousError->getMessage(),
'stack' => explode(PHP_EOL, $previousError->getTraceAsString()),
];
} while ($previousError->getPrevious() !== null);

$errorFormatted['previous'] = $previousErrors;
}

// Log the exception in CloudWatch
// We aim to use the same log format as what we can see when throwing an exception in the NodeJS runtime
// See https://github.com/brefphp/bref/pull/579
/** @noinspection JsonEncodingApiUsageInspection */
echo $invocationId . "\tInvoke Error\t" . json_encode($errorFormatted) . PHP_EOL;
$this->logError($error, $invocationId);

/**
* Send an "error" Lambda response (see https://github.com/brefphp/bref/pull/1483).
Expand All @@ -238,7 +224,7 @@ private function signalFailure(string $invocationId, Throwable $error): void
$this->postJson($url, [
'errorType' => get_class($error),
'errorMessage' => $error->getMessage(),
'stackTrace' => $stackTraceAsArray,
'stackTrace' => explode(PHP_EOL, $error->getTraceAsString()),
]);
}
}
Expand Down Expand Up @@ -444,4 +430,38 @@ private function setEnv(string $name, string $value): void
throw new RuntimeException("Failed to set environment variable $name");
}
}

/**
* Log the exception in CloudWatch
* We aim to use the same log format as what we can see when throwing an exception in the NodeJS runtime
*
* @see https://github.com/brefphp/bref/pull/579
*/
private function logError(Throwable $error, string $invocationId): void
{
$stackTraceAsArray = explode(PHP_EOL, $error->getTraceAsString());
$errorFormatted = [
'errorType' => get_class($error),
'errorMessage' => $error->getMessage(),
'stack' => $stackTraceAsArray,
];

if ($error->getPrevious() !== null) {
$previousError = $error;
$previousErrors = [];
do {
$previousError = $previousError->getPrevious();
$previousErrors[] = [
'errorType' => get_class($previousError),
'errorMessage' => $previousError->getMessage(),
'stack' => explode(PHP_EOL, $previousError->getTraceAsString()),
];
} while ($previousError->getPrevious() !== null);

$errorFormatted['previous'] = $previousErrors;
}

/** @noinspection JsonEncodingApiUsageInspection */
echo $invocationId . "\tInvoke Error\t" . json_encode($errorFormatted) . PHP_EOL;
}
}
70 changes: 70 additions & 0 deletions tests/Runtime/LambdaRuntimeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Bref\Test\Runtime;

use Bref\Bref;
use Bref\Context\Context;
use Bref\Event\EventBridge\EventBridgeEvent;
use Bref\Event\EventBridge\EventBridgeHandler;
Expand All @@ -13,6 +14,7 @@
use Bref\Event\Sns\SnsHandler;
use Bref\Event\Sqs\SqsEvent;
use Bref\Event\Sqs\SqsHandler;
use Bref\Listener\BrefEventSubscriber;
use Bref\Runtime\LambdaRuntime;
use Bref\Runtime\ResponseTooBig;
use Bref\Test\RuntimeTestCase;
Expand All @@ -37,6 +39,8 @@ protected function setUp(): void
{
parent::setUp();

Bref::reset();

$this->runtime = new LambdaRuntime('localhost:8126', 'phpunit');
}

Expand Down Expand Up @@ -361,4 +365,70 @@ public function handleEventBridge(EventBridgeEvent $event, Context $context): vo

$this->assertEquals(new EventBridgeEvent($eventData), $handler->event);
}

public function test exceptions in beforeInvoke result in an invocation error()
{
Bref::events()->subscribe(new class extends BrefEventSubscriber {
public function beforeInvoke(mixed ...$params): void
{
throw new Exception('This is an exception in beforeInvoke');
}
});

$this->givenAnEvent([]);

$output = $this->runtime->processNextEvent(fn () => []);

$this->assertFalse($output);
$this->assertInvocationErrorResult('Exception', 'This is an exception in beforeInvoke');
$this->assertErrorInLogs('Exception', 'This is an exception in beforeInvoke');
}

/**
* Once we reported to Lambda that the execution was successful, we should not report a failure.
* So any exception in `afterInvoke` should not be reported as a failure.
*/
public function test a failure in afterInvoke after a success does not signal a failure()
{
Bref::events()->subscribe(new class extends BrefEventSubscriber {
public function afterInvoke(mixed ...$params): void
{
throw new Exception('This is an exception in afterInvoke');
}
});

$this->givenAnEvent([]);
$output = $this->runtime->processNextEvent(fn () => []);

$this->assertFalse($output);
$this->assertErrorInLogs('Exception', 'This is an exception in afterInvoke');
$requests = Server::received();
// Only the event request and the success response should be sent, no error should be sent
$this->assertCount(2, $requests);
[$eventRequest, $eventSuccessResponse] = $requests;
$this->assertSame('GET', $eventRequest->getMethod());
$this->assertSame('http://localhost:8126/2018-06-01/runtime/invocation/next', $eventRequest->getUri()->__toString());
$this->assertSame('POST', $eventSuccessResponse->getMethod());
$this->assertSame('http://localhost:8126/2018-06-01/runtime/invocation/1/response', $eventSuccessResponse->getUri()->__toString());
}

public function test a failure in afterInvoke after a failure does not crash the runtime()
{
Bref::events()->subscribe(new class extends BrefEventSubscriber {
public function afterInvoke(mixed ...$params): void
{
throw new Exception('This is an exception in afterInvoke');
}
});

$this->givenAnEvent([]);
$output = $this->runtime->processNextEvent(fn () => throw new Exception('Invocation error'));

$this->assertFalse($output);
// The error response was already sent, it contains the handler error
$this->assertInvocationErrorResult('Exception', 'Invocation error');
// The logs should contain both the handler error and the afterInvoke error
$this->assertStringContainsString('Invoke Error {"errorType":"Exception","errorMessage":"Invocation error","stack":', $this->getActualOutput());
$this->assertStringContainsString('Invoke Error {"errorType":"Exception","errorMessage":"This is an exception in afterInvoke","stack":', $this->getActualOutput());
}
}

0 comments on commit 5c173d7

Please sign in to comment.