Skip to content

Commit e9576a6

Browse files
authored
bug KnpLabs#992 fixed php warning in GithubExceptionThrower (clxmstaab, acrobat)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- fixes ``` Warning: Illegal string offset 'code' in GithubExceptionThrower.php on line 59 ``` when deleting a deployment which is not yet set to inactive, see KnpLabs#991 I guess the warning beeing fixed is not explicitly related to the new endpoint added in KnpLabs#991 but I experienced it in this case. Commits ------- 2afb40c fixed php warning in GithubExceptionThrower 2b35311 added testcase a54c223 Update lib/Github/HttpClient/Plugin/GithubExceptionThrower.php e000f00 Update GithubExceptionThrower.php 0e0265f fix CS 1ae94ab Improve single message errors and testcase
1 parent 0081eae commit e9576a6

File tree

2 files changed

+24
-2
lines changed

2 files changed

+24
-2
lines changed

lib/Github/HttpClient/Plugin/GithubExceptionThrower.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla
5959
if (422 === $response->getStatusCode() && isset($content['errors'])) {
6060
$errors = [];
6161
foreach ($content['errors'] as $error) {
62-
switch ($error['code']) {
62+
switch ($error['code'] ?? null) {
6363
case 'missing':
6464
$errors[] = sprintf('The %s %s does not exist, for resource "%s"', $error['field'], $error['value'], $error['resource']);
6565
break;
@@ -81,6 +81,12 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla
8181
break;
8282

8383
default:
84+
if (is_string($error)) {
85+
$errors[] = $error;
86+
87+
break;
88+
}
89+
8490
if (isset($error['message'])) {
8591
$errors[] = $error['message'];
8692
}

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function testHandleRequest(ResponseInterface $response, ExceptionInterfac
3737
if ($exception) {
3838
$this->expectException(get_class($exception));
3939
$this->expectExceptionCode($exception->getCode());
40-
$this->expectExceptionMessage($exception->getMessage());
40+
$this->expectExceptionMessageRegExp('/'.preg_quote($exception->getMessage(), '/').'$/');
4141
}
4242

4343
$plugin->doHandleRequest(
@@ -196,6 +196,22 @@ public static function responseProvider()
196196
),
197197
'exception' => new \Github\Exception\RuntimeException('This endpoint requires you to be authenticated.', 401),
198198
],
199+
'Cannot delete active deployment' => [
200+
'response' => new Response(
201+
422,
202+
[
203+
'content-type' => 'application/json',
204+
],
205+
json_encode(
206+
[
207+
'message' => 'Validation Failed',
208+
'errors' => ['We cannot delete an active deployment unless it is the only deployment in a given environment.'],
209+
'documentation_url' => 'https://docs.github.com/rest/reference/repos#delete-a-deployment',
210+
]
211+
)
212+
),
213+
'exception' => new \Github\Exception\ValidationFailedException('Validation Failed: We cannot delete an active deployment unless it is the only deployment in a given environment.', 422),
214+
],
199215
];
200216
}
201217
}

0 commit comments

Comments
 (0)