Implementing ClientAware on exceptions

[georgeboot]

Summary

Implement the ClientAware interface on the ValidationError and AuthorizationError.

This will add the error category to an error response. Please refer to https://webonyx.github.io/graphql-php/error-handling/.

Technically this could be a breaking change, because error category changes from graphql to validation|authorization. However, I don't believe that anyone implemented that since the graphql category was used everywhere, and it would thus be pointless to check for it.

I also fixed a small typo in the pull request template.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• Code style has been fixed via composer fix-style

[mfn]

In this case \Rebing\GraphQL\GraphQL::formatError should be adapted to make use of the category.

I would have pushed the change myself but can't,since you made this change only in your master

[mfn]

Oh, and could you please also add a CHANGELOG.md entry under "Changed"?

[georgeboot]

In this case \Rebing\GraphQL\GraphQL::formatError should be adapted to make use of the category.

Why does this need to be adjusted? webonyx/graphql-php will handle this automatically.

[mfn]

Why does this need to be adjusted? webonyx/graphql-php will handle this automatically.

How can it, this is our library here:

Or am I missing something?

[georgeboot]

Ah yes. Now I see.

The ClientSafe interface will only set the errors.x.category. This category is graphql by default.

However, in clients, it’s very hard to check what error is actually happening other than comparing error messages. And we all know that is bad.

If we have a category to compare with, it’s much easier. If the category is validation, we need to run logic to handle validation issues etc.

What you are referring to is the extension content. This is additional output you can set. It’s mainly used (as far as I know) for adding context to errors.

In my own projects i actually use a custom error handler and formatter. I’ll share it. Perhaps it can inspire the (basic) implementation this package currently has.

[georgeboot]

My custom error handler and formatter:

<?php

declare(strict_types=1);

namespace App\Support;

use GraphQL\Error\Debug;
use GraphQL\Error\Error;
use GraphQL\Error\FormattedError;
use NunoMaduro\Collision\Adapters\Laravel\ExceptionHandler;
use Rebing\GraphQL\Error\ValidationError;
use Throwable;

class GraphQLErrorHandler
{
    public static function formatError(Error $error): array
    {
        $debug = config('app.debug') ? (Debug::INCLUDE_DEBUG_MESSAGE | Debug::INCLUDE_TRACE) : 0;
        $formatter = FormattedError::prepareFormatter(null, $debug);
        
        return $formatter($error);
    }

    public static function handleErrors(array $errors, callable $formatter): array
    {
        $handler = function (Error $error) use ($formatter): array {
            $error = self::prepareError($error);

            return $formatter($error);
        };

        return \array_map($handler, $errors);
    }

    public static function prepareError(Error $error): Error
    {
        $underlyingException = $error->getPrevious();

        if ($underlyingException) {
            $error = self::generateNewErrorBasedOnPreviousException($error, $underlyingException);
        }

        resolve(ExceptionHandler::class)->report($error);

        return $error;
    }

    public static function generateNewErrorBasedOnPreviousException(Error $error, Throwable $previous): Error
    {
        $message = $error->message;
        $nodes = $error->nodes;
        $source = $error->getSource();
        $positions = $error->getPositions();
        $path = $error->getPath();
        $extensionContent = [];

        if ($previous instanceof RendersErrorsExtensions) {
            $extensionContent = $previous->extensionsContent();
        } elseif ($previous instanceof ValidationError) {
            $extensionContent = [
                'validation' => $previous->getValidatorMessages(),
            ];
        }

        return new Error(
            $message,
            $nodes,
            $source,
            $positions,
            $path,
            $previous,
            $extensionContent
        );
    }
}

In my exception handler, I've added the following to my '$dontReport`:

protected $dontReport = [
    \Rebing\GraphQL\Error\ValidationError::class,
];

[mfn]

Oh, I already understand. I kind of treated the graphql category and the extension sub-namespace be the same. In this case, disregard my comment.

Is it possible to add a test, exposing this field?

[georgeboot]

Is it possible to add a test, exposing this field?

Sure, on it.

[mfn]

I guess we'll be able to merge this soon, can you please also sync it with master and resolve the conflict?

[georgeboot]

Is there any desire to rewrite the error handler/formatter of this package to support extension content etc? I can do a PR for this.

[mfn]

Is there any desire to rewrite the error handler/formatter of this package to support extension content etc? I can do a PR for this.

Not sure what you mean exactly.

Thanks for the PR 😄