Skip to content

Commit

Permalink
Felix decides how the client handles errors #9900
Browse files Browse the repository at this point in the history 
Because webonyx/graphql dropped support for `category` we need a new
mechanism to decide how to handle error in the client. This is now the
role of Felix and the client is simplified.

An error may have one the extension set to true:

- `showSnack` means the error has a friendly user message that should be
    shown via a snackbar (not a full page error)
- `objectNotFound` means an object was not found. The full page error
    does not suggest reloading, since it is not an API incompatibility
    issue

All other errors are redirected to full page error with a suggestion to
reload the app because we suspect an incompatibility between client and
server (or a temporary failure of the server, but we don't want to let
the client enter an incoherent state)
  • Loading branch information
@PowerKiKi
PowerKiKi committed Nov 2, 2023
1 parent 8b5bbab commit 5109e04
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 6 deletions.
15 changes: 11 additions & 4 deletions src/Api/Server.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
namespace Ecodev\Felix\Api;

use Doctrine\DBAL\Exception\DriverException;
use GraphQL\Error\ClientAware;
use GraphQL\Error\DebugFlag;
use GraphQL\Error\UserError;
use GraphQL\Executor\ExecutionResult;
use GraphQL\GraphQL;
use GraphQL\Server\ServerConfig;
Expand Down Expand Up @@ -102,9 +102,16 @@ private function handleError(Throwable $exception, callable $formatter): array

$result = $formatter($exception);

if (!$exception instanceof ClientAware || !$exception->isClientSafe()) {
$result['extensions'] ??= [];
$result['extensions']['category'] = 'internal';
// Invalid variable that end-user might have crafted via the URL
$isInvalidVariables = preg_match('~^Variable \".*\" got invalid value ~', $exception->getMessage());
$isFelixException = $exception->getPrevious() instanceof Exception;
if ($isFelixException || $isInvalidVariables) {
$result['extensions']['showSnack'] = true;
}

// Not found object
if ($exception->getPrevious() instanceof UserError && preg_match('~^Entity not found for class `~', $exception->getMessage())) {
$result['extensions']['objectNotFound'] = true;
}

return $result;
Expand Down
191 changes: 189 additions & 2 deletions tests/Api/ServerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,81 @@
namespace EcodevTests\Felix\Api;

use Ecodev\Felix\Api\Server;
use EcodevTests\Felix\Traits\TestWithContainer;
use Exception;
use GraphQL\Error\UserError;
use GraphQL\Executor\ExecutionResult;
use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\Type;
use GraphQL\Type\Schema;
use Laminas\ConfigAggregator\ArrayProvider;
use Laminas\ConfigAggregator\ConfigAggregator;
use Laminas\Diactoros\CallbackStream;
use Laminas\Diactoros\ServerRequest;
use Laminas\Log\LoggerInterface;
use PHPUnit\Framework\TestCase;

class ServerTest extends TestCase
{
use TestWithContainer;

/**
* @var LoggerInterface&\PHPUnit\Framework\MockObject\MockObject
*/
private LoggerInterface $logger;

protected function setUp(): void
{
$this->logger = $this->createMock(LoggerInterface::class);
$aggregator = new ConfigAggregator([
new ArrayProvider([
'dependencies' => [
'factories' => [
LoggerInterface::class => fn () => $this->logger,
],
],
]),
]);

$this->createContainer($aggregator);
}

/**
* @dataProvider providerExecute
*/
public function testExecute(string $body, array $expected): void
public function testExecute(string $body, array $expected, string $expectedLog = ''): void
{
$schema = new Schema(['query' => new ObjectType(['name' => 'Query', 'fields' => []])]);
$schema = new Schema(['query' => new ObjectType([
'name' => 'Query',
'fields' => [
'nativeException' => [
'type' => Type::boolean(),
'resolve' => fn () => throw new Exception('Fake message'),
],
'felixException' => [
'type' => Type::boolean(),
'resolve' => fn () => throw new \Ecodev\Felix\Api\Exception('Fake message'),
],
'notFoundException' => [
'type' => Type::boolean(),
'resolve' => fn () => throw new UserError('Entity not found for class `foo` and ID `bar`.'),
],
'invalidVariables' => [
'args' => [
'myArg' => Type::boolean(),
],
'type' => Type::boolean(),
'resolve' => fn () => true,
],
],
])]);

if ($expectedLog) {
$this->logger->expects(self::once())->method('err')->with($expectedLog);
} else {
$this->logger->expects(self::never())->method('err');
}

$server = new Server($schema, false);
$request = new ServerRequest(method: 'POST', body: new CallbackStream(fn () => $body));
$request = $request->withHeader('content-type', 'application/json');
Expand Down Expand Up @@ -73,5 +133,132 @@ public static function providerExecute(): iterable
],
],
];

yield 'native exception' => [
'{"query": "{ nativeException }"}',
[
'errors' => [
[
'message' => 'Internal server error',
'locations' => [
[
'line' => 1,
'column' => 3,
],
],
'path' => [
'nativeException',
],
],
],
'data' => [
'nativeException' => null,
],
],
<<<STRING
Fake message
GraphQL request (1:3)
1: { nativeException }
^
STRING
];

yield 'Felix exception shows snackbar' => [
'{"query": "{ felixException }"}',
[
'errors' => [
[
'message' => 'Fake message',
'locations' => [
[
'line' => 1,
'column' => 3,
],
],
'path' => [
'felixException',
],
'extensions' => [
'showSnack' => true,
],
],
],
'data' => [
'felixException' => null,
],
],
<<<STRING
Fake message
GraphQL request (1:3)
1: { felixException }
^
STRING
];

yield 'not found exception does not show snackbar but is flagged' => [
'{"query": "{ notFoundException }"}',
[
'errors' => [
[
'message' => 'Entity not found for class `foo` and ID `bar`.',
'locations' => [
[
'line' => 1,
'column' => 3,
],
],
'path' => [
'notFoundException',
],
'extensions' => [
'objectNotFound' => true,
],
],
],
'data' => [
'notFoundException' => null,
],
],
<<<STRING
Entity not found for class `foo` and ID `bar`.
GraphQL request (1:3)
1: { notFoundException }
^
STRING
];

yield 'invalidVariables shows snackbar' => [
'{"query": "query ($v: Boolean) { invalidVariables(myArg: $v) }", "variables": {"v": 123}}',
[
'errors' => [
[
'message' => 'Variable "$v" got invalid value 123; Boolean cannot represent a non boolean value: 123',
'locations' => [
[
'line' => 1,
'column' => 8,
],
],
'extensions' => [
'showSnack' => true,
],
],
],
],
<<<STRING
Variable "\$v" got invalid value 123; Boolean cannot represent a non boolean value: 123
GraphQL request (1:8)
1: query (\$v: Boolean) { invalidVariables(myArg: \$v) }
^
STRING
];
}
}

0 comments on commit 5109e04

Please sign in to comment.