diff --git a/CHANGELOG.md b/CHANGELOG.md index ff55a8509d..ac01cfa8ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ You can find and compare releases at the [GitHub release page](https://github.co - Remove subscriber reference from topic when deleted https://github.com/nuwave/lighthouse/pull/1288 - Improve subscription context serializer https://github.com/nuwave/lighthouse/pull/1283 - Allow replacing the `SubscriptionRegistry` implementation using the container https://github.com/nuwave/lighthouse/pull/1286 +- Report errors that are not client-safe through Laravel's `ExceptionHandler` https://github.com/nuwave/lighthouse/pull/1303 ## 4.11.0 diff --git a/src/Execution/ErrorHandler.php b/src/Execution/ErrorHandler.php index 13a7c689e1..c8b5ac49cd 100644 --- a/src/Execution/ErrorHandler.php +++ b/src/Execution/ErrorHandler.php @@ -12,6 +12,7 @@ interface ErrorHandler * * Always call $next($error) to keep the Pipeline going. Multiple such Handlers may be registered * as an array in the config. + * TODO change to non-static in v5 */ public static function handle(Error $error, Closure $next): array; } diff --git a/src/Execution/ExtensionErrorHandler.php b/src/Execution/ExtensionErrorHandler.php index edd48e0388..1373ea6fa4 100644 --- a/src/Execution/ExtensionErrorHandler.php +++ b/src/Execution/ExtensionErrorHandler.php @@ -6,13 +6,13 @@ use GraphQL\Error\Error; use Nuwave\Lighthouse\Exceptions\RendersErrorsExtensions; +/** + * Handle Exceptions that implement Nuwave\Lighthouse\Exceptions\RendersErrorsExtensions + * and add extra content from them to the 'extensions' key of the Error that is rendered + * to the User. + */ class ExtensionErrorHandler implements ErrorHandler { - /** - * Handle Exceptions that implement Nuwave\Lighthouse\Exceptions\RendersErrorsExtensions - * and add extra content from them to the 'extensions' key of the Error that is rendered - * to the User. - */ public static function handle(Error $error, Closure $next): array { $underlyingException = $error->getPrevious(); diff --git a/src/Execution/ReportingErrorHandler.php b/src/Execution/ReportingErrorHandler.php new file mode 100644 index 0000000000..c0e47b1c91 --- /dev/null +++ b/src/Execution/ReportingErrorHandler.php @@ -0,0 +1,29 @@ +isClientSafe()) { + return $next($error); + } + + // TODO inject through constructor once handle is non-static + /** @var \Illuminate\Contracts\Debug\ExceptionHandler $reporter */ + $reporter = app(ExceptionHandler::class); + $reporter->report($error->getPrevious()); + + return $next($error); + } +} diff --git a/src/Testing/MocksResolvers.php b/src/Testing/MocksResolvers.php index 3a76f35ccc..9bfd39ad80 100644 --- a/src/Testing/MocksResolvers.php +++ b/src/Testing/MocksResolvers.php @@ -34,9 +34,7 @@ protected function mockResolver($resolverOrValue = null, string $key = 'default' */ protected function mockResolverExpects(/* TODO add strong type hint when bumping PHPUnit */ $invocationOrder, string $key = 'default'): InvocationMocker { - $mock = $this - ->getMockBuilder(MockResolver::class) - ->getMock(); + $mock = $this->createMock(MockResolver::class); $this->registerMockResolver($mock, $key); diff --git a/src/lighthouse.php b/src/lighthouse.php index 5f74410184..bbb19aa0c5 100644 --- a/src/lighthouse.php +++ b/src/lighthouse.php @@ -180,6 +180,7 @@ 'error_handlers' => [ \Nuwave\Lighthouse\Execution\ExtensionErrorHandler::class, + \Nuwave\Lighthouse\Execution\ReportingErrorHandler::class, ], /* diff --git a/tests/Integration/ReportingErrorHandlerTest.php b/tests/Integration/ReportingErrorHandlerTest.php new file mode 100644 index 0000000000..5b6fde1bd2 --- /dev/null +++ b/tests/Integration/ReportingErrorHandlerTest.php @@ -0,0 +1,62 @@ +mockResolver(function () use ($exception) { + throw $exception; + }); + + $handler = $this->createMock(ExceptionHandler::class); + $handler + ->expects($this->atLeastOnce()) + ->method('report') + ->with($exception); + app()->singleton(ExceptionHandler::class, function () use ($handler) { + return $handler; + }); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + '); + } + + public function testDoesNotReportClientSafeErrors(): void + { + $error = new Error('an expected error that is shown to clients'); + $this->mockResolver(function () use ($error) { + throw $error; + }); + + $handler = $this->createMock(ExceptionHandler::class); + $handler + ->expects($this->never()) + ->method('report') + ->with($error); + app()->singleton(ExceptionHandler::class, function () use ($handler) { + return $handler; + }); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + '); + } +} diff --git a/tests/Unit/Schema/Directives/Fixtures/Foo.php b/tests/Unit/Schema/Directives/Fixtures/Foo.php index ba2eaee2bd..ce315428a0 100644 --- a/tests/Unit/Schema/Directives/Fixtures/Foo.php +++ b/tests/Unit/Schema/Directives/Fixtures/Foo.php @@ -10,6 +10,5 @@ class Foo { public function bar() { - // } } diff --git a/tests/Unit/Schema/SecurityTest.php b/tests/Unit/Schema/SecurityTest.php index d7712a279b..f450a06b5d 100644 --- a/tests/Unit/Schema/SecurityTest.php +++ b/tests/Unit/Schema/SecurityTest.php @@ -13,11 +13,11 @@ public function testCanSetMaxComplexityThroughConfig(): void { config(['lighthouse.security.max_query_complexity' => 1]); - $this->schema = ' + $this->schema = /** @lang GraphQL */ ' type Query { user: User @first } - + type User { name: String } @@ -30,11 +30,11 @@ public function testCanSetMaxDepthThroughConfig(): void { config(['lighthouse.security.max_query_depth' => 1]); - $this->schema = ' + $this->schema = /** @lang GraphQL */ ' type Query { user: User @first } - + type User { name: String user: User @@ -53,7 +53,7 @@ public function testCanDisableIntrospectionThroughConfig(): void protected function assertMaxQueryComplexityIs1(): void { - $result = $this->graphQL(' + $result = $this->graphQL(/** @lang GraphQL */ ' { user { name @@ -69,7 +69,7 @@ protected function assertMaxQueryComplexityIs1(): void protected function assertMaxQueryDepthIs1(): void { - $result = $this->graphQL(' + $result = $this->graphQL(/** @lang GraphQL */ ' { user { user { @@ -89,7 +89,7 @@ protected function assertMaxQueryDepthIs1(): void protected function assertIntrospectionIsDisabled(): void { - $result = $this->graphQL(' + $result = $this->graphQL(/** @lang GraphQL */ ' { __schema { queryType { diff --git a/tests/Unit/Subscriptions/StorageManagerTest.php b/tests/Unit/Subscriptions/StorageManagerTest.php index a941c78104..307a723b56 100644 --- a/tests/Unit/Subscriptions/StorageManagerTest.php +++ b/tests/Unit/Subscriptions/StorageManagerTest.php @@ -37,9 +37,7 @@ protected function setUp(): void protected function subscriber(string $queryString): Subscriber { /** @var \Nuwave\Lighthouse\Subscriptions\Subscriber $subscriber */ - $subscriber = $this->getMockBuilder(Subscriber::class) - ->disableOriginalConstructor() - ->getMock(); + $subscriber = $this->createMock(Subscriber::class); $subscriber->channel = Subscriber::uniqueChannelName(); $subscriber->query = Parser::parse($queryString); diff --git a/tests/Unit/Subscriptions/SubscriberTest.php b/tests/Unit/Subscriptions/SubscriberTest.php index 62444baf23..984512cc12 100644 --- a/tests/Unit/Subscriptions/SubscriberTest.php +++ b/tests/Unit/Subscriptions/SubscriberTest.php @@ -25,9 +25,7 @@ public function testSerializable(): void { $args = ['foo' => 'bar']; - $resolveInfo = $this->getMockBuilder(ResolveInfo::class) - ->disableOriginalConstructor() - ->getMock(); + $resolveInfo = $this->createMock(ResolveInfo::class); $operationName = 'baz'; $resolveInfo->operation = (object) [ 'name' => (object) [