Skip to content

Commit 397a3d3

Browse files
PowerKiKiPowerKiKi
committed
Show snackbar to human if value cannot be parsed #10145
By throwing `\GraphQL\Error\Error` in all our custom types, we allow showing a snackbar to human if they somehow input something wrong that still passes client validation. So they might have a small chance to understand the message and correct their input.
1 parent 36c6ba8 commit 397a3d3

12 files changed

+72
-13
lines changed

‎src/Api/Scalar/AbstractDecimalType.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use GraphQL\Language\AST\StringValueNode;
1212
use GraphQL\Type\Definition\ScalarType;
1313
use GraphQL\Utils\Utils;
14-
use UnexpectedValueException;
1514

1615
abstract class AbstractDecimalType extends ScalarType
1716
{
@@ -78,7 +77,7 @@ public function parseValue(mixed $value): string
7877
{
7978
$parsedValue = (string) $value;
8079
if (!$this->isValid($parsedValue)) {
81-
throw new UnexpectedValueException('Query error: Not a valid ' . $this->name . ': ' . Utils::printSafe($value));
80+
throw new Error('Query error: Not a valid ' . $this->name . ': ' . Utils::printSafe($value));
8281
}
8382

8483
return $parsedValue;

‎src/Api/Scalar/AbstractMoneyType.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use GraphQL\Type\Definition\ScalarType;
1313
use GraphQL\Utils\Utils;
1414
use Money\Money;
15-
use UnexpectedValueException;
1615

1716
abstract class AbstractMoneyType extends ScalarType
1817
{
@@ -40,7 +39,7 @@ public function serialize(mixed $value): mixed
4039
public function parseValue(mixed $value): Money
4140
{
4241
if (!is_scalar($value)) {
43-
throw new UnexpectedValueException('Cannot represent value as Money: ' . Utils::printSafe($value));
42+
throw new Error('Cannot represent value as Money: ' . Utils::printSafe($value));
4443
}
4544

4645
$value = (string) $value;

‎src/Api/Scalar/AbstractStringBasedType.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use GraphQL\Language\AST\StringValueNode;
1010
use GraphQL\Type\Definition\ScalarType;
1111
use GraphQL\Utils\Utils;
12-
use UnexpectedValueException;
1312

1413
abstract class AbstractStringBasedType extends ScalarType
1514
{
@@ -34,7 +33,7 @@ public function parseValue(mixed $value): ?string
3433
{
3534
$typeOk = is_string($value) || $value === null;
3635
if (!$typeOk || !$this->isValid($value)) {
37-
throw new UnexpectedValueException('Query error: Not a valid ' . $this->name . ': ' . Utils::printSafe($value));
36+
throw new Error('Query error: Not a valid ' . $this->name . ': ' . Utils::printSafe($value));
3837
}
3938

4039
return $value;

‎src/Api/Scalar/ChronosType.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use GraphQL\Language\AST\StringValueNode;
1212
use GraphQL\Type\Definition\ScalarType;
1313
use GraphQL\Utils\Utils;
14-
use UnexpectedValueException;
1514

1615
final class ChronosType extends ScalarType
1716
{
@@ -35,7 +34,7 @@ public function serialize(mixed $value): mixed
3534
public function parseValue(mixed $value): ?Chronos
3635
{
3736
if (!is_string($value)) {
38-
throw new UnexpectedValueException('Cannot represent value as Chronos date: ' . Utils::printSafe($value));
37+
throw new Error('Cannot represent value as Chronos date: ' . Utils::printSafe($value));
3938
}
4039

4140
if ($value === '') {

‎src/Api/Scalar/DateType.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use GraphQL\Language\AST\StringValueNode;
1111
use GraphQL\Type\Definition\ScalarType;
1212
use GraphQL\Utils\Utils;
13-
use UnexpectedValueException;
1413

1514
final class DateType extends ScalarType
1615
{
@@ -34,7 +33,7 @@ public function serialize(mixed $value): mixed
3433
public function parseValue(mixed $value): ChronosDate
3534
{
3635
if (!is_string($value)) {
37-
throw new UnexpectedValueException('Cannot represent value as Chronos date: ' . Utils::printSafe($value));
36+
throw new Error('Cannot represent value as Chronos date: ' . Utils::printSafe($value));
3837
}
3938

4039
$date = ChronosDate::createFromFormat('Y-m-d+', $value);

‎src/Api/Scalar/TimeType.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use GraphQL\Language\AST\StringValueNode;
1111
use GraphQL\Type\Definition\ScalarType;
1212
use GraphQL\Utils\Utils;
13-
use UnexpectedValueException;
1413

1514
final class TimeType extends ScalarType
1615
{
@@ -34,15 +33,15 @@ public function serialize(mixed $value): mixed
3433
public function parseValue(mixed $value): ?ChronosTime
3534
{
3635
if (!is_string($value)) {
37-
throw new UnexpectedValueException('Cannot represent value as Chronos time: ' . Utils::printSafe($value));
36+
throw new Error('Cannot represent value as ChronosTime: ' . Utils::printSafe($value));
3837
}
3938

4039
if ($value === '') {
4140
return null;
4241
}
4342

4443
if (!preg_match('~^(?<hour>\d{1,2})(([h:]$)|([h:](?<minute>\d{1,2}))?$)~', trim($value), $m)) {
45-
throw new UnexpectedValueException('Invalid format Chronos time. Expected "14h35", "14:35" or "14h", but got: ' . Utils::printSafe($value));
44+
throw new Error('Invalid format for ChronosTime. Expected "14h35", "14:35" or "14h", but got: ' . Utils::printSafe($value));
4645
}
4746

4847
$value = $m['hour'] . ':' . ($m['minute'] ?? '00');

‎tests/Api/Scalar/AbstractDecimalTypeTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
namespace EcodevTests\Felix\Api\Scalar;
66

77
use Ecodev\Felix\Api\Scalar\AbstractDecimalType;
8+
use GraphQL\Error\Error;
89
use GraphQL\Language\AST\FloatValueNode;
910
use GraphQL\Language\AST\IntValueNode;
11+
use GraphQL\Language\AST\NullValueNode;
1012
use GraphQL\Language\AST\StringValueNode;
1113
use GraphQL\Utils\Utils;
1214
use PHPUnit\Framework\TestCase;
@@ -58,6 +60,7 @@ public function testParseValue(int $decimal, ?string $minimum, ?string $maximum,
5860
$type = $this->createType($decimal, $minimum, $maximum);
5961

6062
if ($expected === null) {
63+
$this->expectException(Error::class);
6164
$this->expectExceptionMessage('Query error: Not a valid TestDecimal' . ': ' . Utils::printSafe($input));
6265
}
6366

@@ -82,6 +85,7 @@ public function testParseLiteral(int $decimal, ?string $minimum, ?string $maximu
8285
}
8386

8487
if ($expected === null) {
88+
$this->expectException(Error::class);
8589
$this->expectExceptionMessage('Query error: Not a valid TestDecimal');
8690
}
8791

@@ -90,6 +94,18 @@ public function testParseLiteral(int $decimal, ?string $minimum, ?string $maximu
9094
self::assertSame($expected, $actual);
9195
}
9296

97+
public function testParseLiteralThrowIfInvalidNodeType(): void
98+
{
99+
$type = $this->createType(3, null, null);
100+
101+
$ast = new NullValueNode([]);
102+
103+
$this->expectException(Error::class);
104+
$this->expectExceptionMessage('Query error: Can only parse strings got: NullValue');
105+
106+
$type->parseLiteral($ast);
107+
}
108+
93109
public static function providerInputs(): array
94110
{
95111
return [

‎tests/Api/Scalar/AbstractStringBasedType.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace EcodevTests\Felix\Api\Scalar;
66

7+
use GraphQL\Error\Error;
78
use GraphQL\Language\AST\IntValueNode;
89
use GraphQL\Language\AST\StringValueNode;
910
use PHPUnit\Framework\TestCase;
@@ -34,6 +35,7 @@ public function testParseValue(?string $input, ?string $expected, bool $isValid)
3435
$type = $this->createType();
3536

3637
if (!$isValid) {
38+
$this->expectException(Error::class);
3739
$this->expectExceptionMessage('Query error: Not a valid ' . $this->getTypeName());
3840
}
3941

@@ -51,6 +53,7 @@ public function testParseLiteral(string $input, ?string $expected, bool $isValid
5153
$ast = new StringValueNode(['value' => $input]);
5254

5355
if (!$isValid) {
56+
$this->expectException(Error::class);
5457
$this->expectExceptionMessage('Query error: Not a valid ' . $this->getTypeName());
5558
}
5659

@@ -64,6 +67,7 @@ public function testParseInvalidNodeWillThrow(): void
6467
$type = $this->createType();
6568
$ast = new IntValueNode(['value' => '123']);
6669

70+
$this->expectException(Error::class);
6771
$this->expectExceptionMessage('Query error: Can only parse strings got: IntValue');
6872
$type->parseLiteral($ast);
6973
}

‎tests/Api/Scalar/CHFTypeTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use GraphQL\Language\AST\StringValueNode;
1212
use Money\Money;
1313
use PHPUnit\Framework\TestCase;
14+
use stdClass;
1415

1516
final class CHFTypeTest extends TestCase
1617
{
@@ -108,9 +109,19 @@ public function testParseValueThrowsWithInvalidValue(string $invalidValue): void
108109
$type = new CHFType();
109110

110111
$this->expectException(Error::class);
112+
$this->expectExceptionMessage('Query error: Not a valid Money: ');
111113
$type->parseValue($invalidValue);
112114
}
113115

116+
public function testParseValueThrowsWithInvalidValueASD(): void
117+
{
118+
$type = new CHFType();
119+
120+
$this->expectException(Error::class);
121+
$this->expectExceptionMessage('Cannot represent value as Money: instance of stdClass');
122+
$type->parseValue(new stdClass());
123+
}
124+
114125
/**
115126
* @dataProvider providerInvalidValues
116127
*/
@@ -120,6 +131,7 @@ public function testParseLiteralThrowsWithInvalidValue(string $invalidValue): vo
120131
$ast = new StringValueNode(['value' => $invalidValue]);
121132

122133
$this->expectException(Error::class);
134+
$this->expectExceptionMessage('Query error: Not a valid Money: ');
123135
$type->parseLiteral($ast);
124136
}
125137

‎tests/Api/Scalar/ChronosTypeTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Cake\Chronos\Chronos;
88
use Ecodev\Felix\Api\Scalar\ChronosType;
9+
use GraphQL\Error\Error;
910
use GraphQL\Language\AST\IntValueNode;
1011
use GraphQL\Language\AST\StringValueNode;
1112
use PHPUnit\Framework\TestCase;
@@ -68,10 +69,20 @@ public function testParseLiteralAsInt(): void
6869
$type = new ChronosType();
6970
$ast = new IntValueNode(['value' => '123']);
7071

72+
$this->expectException(Error::class);
7173
$this->expectExceptionMessage('Query error: Can only parse strings got: IntValue');
7274
$type->parseLiteral($ast);
7375
}
7476

77+
public function testParseValueAsInt(): void
78+
{
79+
$type = new ChronosType();
80+
81+
$this->expectException(Error::class);
82+
$this->expectExceptionMessage('Cannot represent value as Chronos date: 123');
83+
$type->parseValue(123);
84+
}
85+
7586
public static function providerValues(): array
7687
{
7788
return [

‎tests/Api/Scalar/DateTypeTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Cake\Chronos\ChronosDate;
88
use Ecodev\Felix\Api\Scalar\DateType;
9+
use GraphQL\Error\Error;
910
use GraphQL\Language\AST\IntValueNode;
1011
use GraphQL\Language\AST\StringValueNode;
1112
use PHPUnit\Framework\TestCase;
@@ -62,6 +63,7 @@ public function testParseLiteralAsInt(): void
6263
$type = new DateType();
6364
$ast = new IntValueNode(['value' => '123']);
6465

66+
$this->expectException(Error::class);
6567
$this->expectExceptionMessage('Query error: Can only parse strings got: IntValue');
6668
$type->parseLiteral($ast);
6769
}

‎tests/Api/Scalar/TimeTypeTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Cake\Chronos\ChronosTime;
88
use Ecodev\Felix\Api\Scalar\TimeType;
9+
use GraphQL\Error\Error;
910
use GraphQL\Language\AST\IntValueNode;
1011
use GraphQL\Language\AST\StringValueNode;
1112
use PHPUnit\Framework\TestCase;
@@ -60,10 +61,29 @@ public function testParseLiteralAsInt(): void
6061
$type = new TimeType();
6162
$ast = new IntValueNode(['value' => '123']);
6263

64+
$this->expectException(Error::class);
6365
$this->expectExceptionMessage('Query error: Can only parse strings got: IntValue');
6466
$type->parseLiteral($ast);
6567
}
6668

69+
public function testParseValueAsInt(): void
70+
{
71+
$type = new TimeType();
72+
73+
$this->expectException(Error::class);
74+
$this->expectExceptionMessage('Cannot represent value as ChronosTime: 123');
75+
$type->parseValue(123);
76+
}
77+
78+
public function testParseValueInvalidFormat(): void
79+
{
80+
$type = new TimeType();
81+
82+
$this->expectException(Error::class);
83+
$this->expectExceptionMessage('Invalid format for ChronosTime. Expected "14h35", "14:35" or "14h", but got: "123"');
84+
$type->parseValue('123');
85+
}
86+
6787
public static function providerValues(): array
6888
{
6989
return [

0 commit comments

Comments
 (0)