Lighthouse should not return the 'data' key if an exception was thrown in the custom builder

[BehzadV]

Are you willing to provide a PR for this issue or aid in developing it? No, I do apologize.

What problem does this feature proposal attempt to solve? I am using the paginator directive with the builder parameter for a field as such: @paginate(builder: "App\\GraphQL\\Builders\\foo@bar")

and in the bar function (probably mistakingly) I check the user permissions before returning the query builder:

public function bar($root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo): Builder
{
   if ($context->user->can('Foo Bar')) {
      return Model::query();
   } else {
      throw new Unauthorized();
   }
}

And this works fine (sort of) in the way that the error is displayed correctly but a null data section is also returned:

{
  "errors": ["CORRECT EXCEPTION HANDLING"],
  "data": {
    "Query name": null
  }
}

Which possible solutions should be considered? If an exception was thrown during a custom builder, Lighthouse should not return the data key in the results.

[spawnia]

That behavior is expected, given the return type of your field is most probably nullable (not marked with !). In that case, returning null for the field is what should happen, since the query result is still valid up until that point.

Check https://lighthouse-php.com/master/security/authorization.html for more information on how to handle authorization.

[BehzadV]

@spawnia Well, the field was not nullable:

manyModels: [Model!]! @paginate(builder: "App\\GraphQL\\Builders\\foo@bar")

However, per your very appreciated suggestion, I created a Field Middleware to handle the authorization:

manyModels: [Model!]! @hasPermission(requiredPermission: "get models") @paginate(builder: "App\\GraphQL\\Builders\\ModelQuery@models")

With the middleware's handler function as:

public function handleField(FieldValue $fieldValue, Closure $next): FieldValue
{
   $originalResolver = $fieldValue->getResolver();

   return $next(
      $fieldValue->setResolver(
         function ($root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo) use ($originalResolver) {
            $requiredPermission = $this->directiveArgValue('requiredPermission');

            if (!isset($requiredPermission)) {
               throw new DefinitionException("Missing argument 'requiredPermission' for directive '@hasPermission'.");
            }

            $user = $context->user();
            if (!isset($user)) {
               throw new Unauthorized();
            } elseif (!$user->can($requiredPermission)) {
               throw new Unauthorized();
            }

            return $originalResolver($root, $args, $context, $resolveInfo);
         }
      )
   );
}

Now, this directive works fine. But the response remains in the shape from the first comment (has a 'data' section AND an 'error' section). Now I do realize the example from the documentation returns null (in the case a request is not authorized) for the handler function (and that will probably fix this) but throwing an exception is rather efficient since Lighthouse handles the error display. It is also worth mentioning that this "misshapen" response format does not occur when a custom builder is not involved (with this new directive). May I then humbly suggest that my proposal be applied? Or have I made a mistake?

[BehzadV]

@spawnia Excuse me, But can I grab your attention again? Your presumption regarding my field type being nullable was incorrect and this behavior from the library persists.

[spawnia]

The first step towards getting this fixed is to create a PR with a failing test case.