Report non client-safe errors through Laravel's ExceptionHandler #1303

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
 

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;
 }

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();

src/Execution/ReportingErrorHandler.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace Nuwave\Lighthouse\Execution;
+
+use Closure;
+use GraphQL\Error\Error;
+use Illuminate\Contracts\Debug\ExceptionHandler;
+
+/**
+ * Report errors through the default exception handler configured in Laravel.
+ */
+class ReportingErrorHandler implements ErrorHandler
+{
+    public static function handle(Error $error, Closure $next): array
+    {
+        // Client-safe errors are assumed to be something that a client can handle
+        // or is expected to happen, e.g. wrong syntax, authentication or validation
+        if ($error->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);
+    }
+}

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);
 

src/lighthouse.php

@@ -180,6 +180,7 @@
 
     'error_handlers' => [
         \Nuwave\Lighthouse\Execution\ExtensionErrorHandler::class,
+        \Nuwave\Lighthouse\Execution\ReportingErrorHandler::class,
     ],
 
     /*

tests/Integration/ReportingErrorHandlerTest.php

@@ -0,0 +1,62 @@
+<?php
+
+namespace Tests\Integration;
+
+use GraphQL\Error\Error;
+use Illuminate\Contracts\Debug\ExceptionHandler;
+use Tests\TestCase;
+
+class ReportingErrorHandlerTest extends TestCase
+{
+    protected $schema = /** @lang GraphQL */ '
+    type Query {
+        foo: ID @mock
+    }
+    ';
+
+    public function testReportsNonClientSafeErrors(): void
+    {
+        $exception = new \Exception('some internal error that was unexpected');
+        $this->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
+        }
+        ');
+    }
+}

tests/Unit/Schema/Directives/Fixtures/Foo.php

@@ -10,6 +10,5 @@ class Foo
 {
     public function bar()
     {
-        //
     }
 }

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 {

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);

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) [