Skip to content

Commit 50a1c51

Browse files
authored
bug KnpLabs#941 Throw exception for graphql errors (acrobat)
This PR was merged into the 2.x branch. Discussion ---------- I noticed that errors from the graphql api didn't throw errors by our ExceptionThrower plugin. That's because the graphql api return HTTP code 200 even when the api had an error. > The GraphQL spec 6 doesn’t make any mention of HTTP status codes in the case of an error. Additionally, I would suggest that the error you show in your post is an application level error, not an HTTP protocol level error. It’s the same thing as when someone submits a value to an HTML form field that is incorrectly formatted … such as an invalid email address. Your web application returns a 200 HTTP status code because everything is fine at the HTTP protocol level, but displays an error message to the user asking them to supply a valid email address in the form. See https://github.community/t/github-api-v4-error-returns-200-status-code/14178/2 So I've adjusted the exception thrower to check HTTP 200 responses for grapql errors, with the early return's in the check function the overhead should be minimal. Commits ------- 820186f Throw exception for graphql errors
2 parents 562ca30 + 820186f commit 50a1c51

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

lib/Github/HttpClient/Plugin/GithubExceptionThrower.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla
2929
{
3030
return $next($request)->then(function (ResponseInterface $response) use ($request) {
3131
if ($response->getStatusCode() < 400 || $response->getStatusCode() > 600) {
32+
$this->checkGraphqlErrors($response);
33+
3234
return $response;
3335
}
3436

@@ -117,4 +119,38 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla
117119
throw new RuntimeException(isset($content['message']) ? $content['message'] : $content, $response->getStatusCode());
118120
});
119121
}
122+
123+
/**
124+
* The graphql api doesn't return a 5xx http status for errors. Instead it returns a 200 with an error body.
125+
*
126+
* @throws RuntimeException
127+
*/
128+
private function checkGraphqlErrors(ResponseInterface $response): void
129+
{
130+
if ($response->getStatusCode() !== 200) {
131+
return;
132+
}
133+
134+
$content = ResponseMediator::getContent($response);
135+
if (!is_array($content)) {
136+
return;
137+
}
138+
139+
if (!isset($content['errors']) || !is_array($content['errors'])) {
140+
return;
141+
}
142+
143+
$errors = [];
144+
foreach ($content['errors'] as $error) {
145+
if (isset($error['message'])) {
146+
$errors[] = $error['message'];
147+
}
148+
}
149+
150+
if (empty($errors)) {
151+
return;
152+
}
153+
154+
throw new RuntimeException(implode(', ', $errors));
155+
}
120156
}

test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,29 @@ public static function responseProvider()
154154
),
155155
'exception' => new \Github\Exception\RuntimeException('Error message', 555),
156156
],
157+
'Graphql error response (200)' => [
158+
'response' => new Response(
159+
200,
160+
[
161+
'content-type' => 'application/json',
162+
],
163+
json_encode(
164+
[
165+
'errors' => [
166+
[
167+
['path' => ['query', 'repository']],
168+
'message' => 'Field "xxxx" doesn\'t exist on type "Issue"',
169+
],
170+
[
171+
['path' => ['query', 'repository']],
172+
'message' => 'Field "dummy" doesn\'t exist on type "PullRequest"',
173+
],
174+
],
175+
]
176+
)
177+
),
178+
'exception' => new \Github\Exception\RuntimeException('Field "xxxx" doesn\'t exist on type "Issue", Field "dummy" doesn\'t exist on type "PullRequest"'),
179+
],
157180
];
158181
}
159182
}

0 commit comments

Comments
 (0)