Standardize GraphQL mutation error handling (#3680)

Our GraphQL mutations currently use a combination of thrown and caught
exceptions, lighthouse-php error handling, and a `messages` data field
used as an error field. This PR switches everything to report proper
GraphQL errors where appropriate, allowing clients to process errors
using built-in error handling mechanisms.

Author: williamjallen

app/Exceptions/GraphQLMutationException.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+use GraphQL\Error\ClientAware;
+
+class GraphQLMutationException extends Exception implements ClientAware
+{
+    public function isClientSafe(): bool
+    {
+        return true;
+    }
+}

app/GraphQL/Mutations/ChangeGlobalRole.php

@@ -5,8 +5,8 @@
 namespace App\GraphQL\Mutations;
 
 use App\Enums\GlobalRole;
+use App\Exceptions\GraphQLMutationException;
 use App\Models\User;
-use Exception;
 use Illuminate\Support\Facades\Validator;
 use Illuminate\Validation\Rule;
 use Illuminate\Validation\ValidationException;
@@ -21,9 +21,10 @@ final class ChangeGlobalRole extends AbstractMutation
      *     role: GlobalRole,
      * } $args
      *
+     * @throws GraphQLMutationException
      * @throws ValidationException
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         Validator::make($args, [
             'userId' => [
@@ -39,23 +40,23 @@ protected function mutate(array $args): void
         $user = auth()->user();
         if ($user === null) {
             // This should never happen, but we handle the case anyway to make PHPStan happy.
-            throw new Exception('Attempt to invite user when not signed in.');
+            throw new GraphQLMutationException('Attempt to invite user when not signed in.');
         }
 
         $userToChange = User::find((int) $args['userId']);
         if ($userToChange === null) {
-            $this->message = 'Cannot change role for user which does not exist.';
-            return;
+            throw new GraphQLMutationException('Cannot change role for user which does not exist.');
         }
 
         if ($user->cannot('changeRole', $userToChange)) {
-            $this->message = 'Insufficient permissions.';
-            return;
+            throw new GraphQLMutationException('Insufficient permissions.');
         }
 
         $userToChange->admin = $args['role'] === GlobalRole::ADMINISTRATOR;
         $userToChange->save();
 
         $this->user = $userToChange;
+
+        return $this;
     }
 }

app/GraphQL/Mutations/ChangeProjectRole.php

@@ -5,12 +5,12 @@
 namespace App\GraphQL\Mutations;
 
 use App\Enums\ProjectRole;
+use App\Exceptions\GraphQLMutationException;
 use App\Models\Project;
 use App\Models\User;
 use Exception;
 use Illuminate\Support\Facades\Validator;
 use Illuminate\Validation\Rule;
-use Illuminate\Validation\ValidationException;
 
 final class ChangeProjectRole extends AbstractMutation
 {
@@ -24,9 +24,10 @@ final class ChangeProjectRole extends AbstractMutation
      *     role: ProjectRole,
      * } $args
      *
-     * @throws ValidationException
+     * @throws GraphQLMutationException
+     * @throws Exception
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         Validator::make($args, [
             'userId' => [
@@ -44,20 +45,17 @@ protected function mutate(array $args): void
         /** @var ?User $user */
         $user = auth()->user();
         if ($user === null) {
-            // This should never happen, but we handle the case anyway to make PHPStan happy.
-            throw new Exception('Attempt to invite user when not signed in.');
+            throw new GraphQLMutationException('Attempt to invite user when not signed in.');
         }
 
         $userToChange = User::find((int) $args['userId']);
         if ($userToChange === null) {
-            $this->message = 'Cannot change role for user which does not exist.';
-            return;
+            throw new GraphQLMutationException('Cannot change role for user which does not exist.');
         }
 
         $project = Project::find((int) $args['projectId']);
         if ($project === null || $user->cannot('changeUserRole', [$project, $userToChange])) {
-            $this->message = 'This action is unauthorized.';
-            return;
+            throw new GraphQLMutationException('This action is unauthorized.');
         }
 
         $rowsEdited = $userToChange->projects()->updateExistingPivot($project->id, [
@@ -73,5 +71,7 @@ protected function mutate(array $args): void
 
         $this->user = $userToChange;
         $this->project = $project;
+
+        return $this;
     }
 }

app/GraphQL/Mutations/ClaimSite.php

@@ -4,6 +4,7 @@
 
 namespace App\GraphQL\Mutations;
 
+use App\Exceptions\GraphQLMutationException;
 use App\Models\Site;
 use App\Models\User;
 
@@ -15,25 +16,29 @@ final class ClaimSite extends AbstractMutation
     /** @param array{
      *     siteId: int
      * } $args
+     *
+     * @throws GraphQLMutationException
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         /** @var ?User $user */
         $user = auth()->user();
 
         if ($user === null) {
-            abort(401, 'Authentication required to claim sites.');
+            throw new GraphQLMutationException('Authentication required to claim sites.');
         }
 
         $site = Site::find((int) $args['siteId']);
 
         if ($site === null) {
-            abort(404, 'Requested site not found.');
+            throw new GraphQLMutationException('Requested site not found.');
         }
 
         $site->maintainers()->syncWithoutDetaching($user);
 
         $this->site = $site;
         $this->user = $user;
+
+        return $this;
     }
 }

app/GraphQL/Mutations/CreateGlobalInvitation.php

@@ -5,6 +5,7 @@
 namespace App\GraphQL\Mutations;
 
 use App\Enums\GlobalRole;
+use App\Exceptions\GraphQLMutationException;
 use App\Mail\InvitedToCdash;
 use App\Models\GlobalInvitation;
 use App\Models\User;
@@ -26,9 +27,10 @@ final class CreateGlobalInvitation extends AbstractMutation
      *     role: GlobalRole,
      * } $args
      *
+     * @throws GraphQLMutationException
      * @throws Exception
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         // This field might not reset when testing since the same mocked request is reused.
         $this->invitedUser = null;
@@ -47,20 +49,19 @@ protected function mutate(array $args): void
         /** @var ?User $user */
         $user = auth()->user();
         if ($user === null) {
-            // This should never happen, but we handle the case anyway to make PHPStan happy.
-            throw new Exception('Attempt to invite user when not signed in.');
+            throw new GraphQLMutationException('Attempt to invite user when not signed in.');
         }
 
         if ($user->cannot('createInvitation', GlobalInvitation::class)) {
-            abort(401, 'This action is unauthorized.');
+            throw new GraphQLMutationException('This action is unauthorized.');
         }
 
         if (GlobalInvitation::where('email', $args['email'])->exists()) {
-            abort(400, 'Duplicate invitations are not allowed.');
+            throw new GraphQLMutationException('Duplicate invitations are not allowed.');
         }
 
         if (User::where('email', $args['email'])->exists()) {
-            abort(401, 'User is already a member of this instance.');
+            throw new GraphQLMutationException('User is already a member of this instance.');
         }
 
         $password = Str::password();
@@ -76,5 +77,7 @@ protected function mutate(array $args): void
         // The email gets sent to the queue, so we have no way to know immediately whether it was sent or not.
         // TODO: We should eventually track whether the email was actually sent.
         Mail::to($args['email'])->send(new InvitedToCdash($this->invitedUser, $password));
+
+        return $this;
     }
 }

app/GraphQL/Mutations/CreatePinnedTestMeasurement.php

@@ -18,7 +18,7 @@ final class CreatePinnedTestMeasurement extends AbstractMutation
      *     name: string,
      * } $args
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         $project = Project::find((int) $args['projectId']);
         Gate::authorize('createPinnedTestMeasurement', $project);
@@ -34,5 +34,7 @@ protected function mutate(array $args): void
             'name' => $args['name'],
             'position' => $nextAvailablePosition,
         ]);
+
+        return $this;
     }
 }

app/GraphQL/Mutations/DeletePinnedTestMeasurement.php

@@ -14,12 +14,14 @@ final class DeletePinnedTestMeasurement extends AbstractMutation
      *     id: int,
      * } $args
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         $measurement = PinnedTestMeasurement::find((int) $args['id']);
 
         Gate::authorize('deletePinnedTestMeasurement', $measurement?->project);
 
         $measurement?->delete();
+
+        return $this;
     }
 }

app/GraphQL/Mutations/InviteToProject.php

@@ -5,6 +5,7 @@
 namespace App\GraphQL\Mutations;
 
 use App\Enums\ProjectRole;
+use App\Exceptions\GraphQLMutationException;
 use App\Mail\InvitedToProject;
 use App\Models\Project;
 use App\Models\ProjectInvitation;
@@ -26,9 +27,10 @@ final class InviteToProject extends AbstractMutation
      *     role: ProjectRole,
      * } $args
      *
+     * @throws GraphQLMutationException
      * @throws Exception
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         // This field might not reset when testing since the same mocked request is reused.
         $this->invitedUser = null;
@@ -56,18 +58,15 @@ protected function mutate(array $args): void
 
         $project = isset($args['projectId']) ? Project::find((int) $args['projectId']) : null;
         if ($project === null || $user->cannot('inviteUser', $project)) {
-            $this->message = 'This action is unauthorized.';
-            return;
+            throw new GraphQLMutationException('This action is unauthorized.');
         }
 
         if (ProjectInvitation::where(['email' => $args['email'], 'project_id' => $args['projectId']])->exists()) {
-            $this->message = 'Duplicate invitations are not allowed.';
-            return;
+            throw new GraphQLMutationException('Duplicate invitations are not allowed.');
         }
 
         if ($project->users()->where('email', $args['email'])->exists()) {
-            $this->message = 'User is already a member of this project.';
-            return;
+            throw new GraphQLMutationException('User is already a member of this project.');
         }
 
         $this->invitedUser = ProjectInvitation::create([
@@ -81,5 +80,7 @@ protected function mutate(array $args): void
         // The email gets sent to the queue, so we have no way to know immediately whether it was sent or not.
         // TODO: We should eventually track whether the email was actually sent.
         Mail::to($args['email'])->send(new InvitedToProject($this->invitedUser));
+
+        return $this;
     }
 }

app/GraphQL/Mutations/JoinProject.php

@@ -4,9 +4,9 @@
 
 namespace App\GraphQL\Mutations;
 
+use App\Exceptions\GraphQLMutationException;
 use App\Models\Project;
 use App\Models\User;
-use Exception;
 
 final class JoinProject extends AbstractMutation
 {
@@ -15,9 +15,9 @@ final class JoinProject extends AbstractMutation
      *     projectId: int,
      * } $args
      *
-     * @throws Exception
+     * @throws GraphQLMutationException
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         /** @var ?User $user */
         $user = auth()->user();
@@ -28,7 +28,7 @@ protected function mutate(array $args): void
             || $project === null
             || $user->cannot('join', $project)
         ) {
-            abort(401, 'This action is unauthorized.');
+            throw new GraphQLMutationException('This action is unauthorized.');
         }
 
         $project
@@ -40,5 +40,7 @@ protected function mutate(array $args): void
                 'emailmissingsites' => false,
                 'role' => Project::PROJECT_USER,
             ]);
+
+        return $this;
     }
 }

app/GraphQL/Mutations/RemoveProjectUser.php

@@ -4,6 +4,7 @@
 
 namespace App\GraphQL\Mutations;
 
+use App\Exceptions\GraphQLMutationException;
 use App\Models\Project;
 use App\Models\User;
 
@@ -14,8 +15,10 @@ final class RemoveProjectUser extends AbstractMutation
      *     userId: int,
      *     projectId: int,
      * } $args
+     *
+     * @throws GraphQLMutationException
      */
-    protected function mutate(array $args): void
+    public function __invoke(null $_, array $args): self
     {
         /** @var ?User $user */
         $user = auth()->user();
@@ -36,9 +39,11 @@ protected function mutate(array $args): void
             )
             || !$project->users()->where('id', $userToRemove->id)->exists()
         ) {
-            abort(401, 'This action is unauthorized.');
+            throw new GraphQLMutationException('This action is unauthorized.');
         }
 
         $project->users()->detach($userToRemove->id);
+
+        return $this;
     }
 }