diff --git a/src/Runtime/LambdaRuntime.php b/src/Runtime/LambdaRuntime.php index f0b70f1ac..23e736472 100755 --- a/src/Runtime/LambdaRuntime.php +++ b/src/Runtime/LambdaRuntime.php @@ -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; } @@ -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). @@ -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()), ]); } } @@ -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; + } } diff --git a/tests/Runtime/LambdaRuntimeTest.php b/tests/Runtime/LambdaRuntimeTest.php index 885e386c4..aecc7d89a 100644 --- a/tests/Runtime/LambdaRuntimeTest.php +++ b/tests/Runtime/LambdaRuntimeTest.php @@ -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; @@ -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; @@ -37,6 +39,8 @@ protected function setUp(): void { parent::setUp(); + Bref::reset(); + $this->runtime = new LambdaRuntime('localhost:8126', 'phpunit'); } @@ -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()); + } }