From 2afb40cd9e775668fe3329c9179df39e3d625b8e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 29 Mar 2021 13:55:32 +0200 Subject: [PATCH 1/6] fixed php warning in GithubExceptionThrower fixes ``` Warning: Illegal string offset 'code' in GithubExceptionThrower.php on line 59 ``` when deleting a deployment which is not yet set to inactive --- lib/Github/HttpClient/Plugin/GithubExceptionThrower.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php index 21e6dd3434f..b846691042b 100644 --- a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php +++ b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php @@ -59,7 +59,12 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla if (422 === $response->getStatusCode() && isset($content['errors'])) { $errors = []; foreach ($content['errors'] as $error) { - switch ($error['code']) { + $code = null; + if (isset($error['code'])) { + $code = $error['code']; + } + + switch ($code) { case 'missing': $errors[] = sprintf('The %s %s does not exist, for resource "%s"', $error['field'], $error['value'], $error['resource']); break; From 2b353115fbefe0d792175852828d1d47059248ae Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 30 Mar 2021 10:07:38 +0200 Subject: [PATCH 2/6] added testcase --- .../Plugin/GithubExceptionThrowerTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php b/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php index b367fe15779..26d6bdfadf8 100644 --- a/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php +++ b/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php @@ -196,6 +196,22 @@ public static function responseProvider() ), 'exception' => new \Github\Exception\RuntimeException('This endpoint requires you to be authenticated.', 401), ], + 'Cannot delete active deployment' => [ + 'response' => new Response( + 422, + [ + 'content-type' => 'application/json', + ], + json_encode( + [ + 'message' => 'Validation Failed', + 'errors' => ['We cannot delete an active deployment unless it is the only deployment in a given environment.'], + 'documentation_url' => 'https://docs.github.com/rest/reference/repos#delete-a-deployment' + ] + ) + ), + 'exception' => new \Github\Exception\ValidationFailedException('Validation Failed', 422), + ], ]; } } From a54c2233142b9f71559e0491e69f1c2c0eb78a06 Mon Sep 17 00:00:00 2001 From: Markus Staab <47448731+clxmstaab@users.noreply.github.com> Date: Tue, 30 Mar 2021 10:07:53 +0200 Subject: [PATCH 3/6] Update lib/Github/HttpClient/Plugin/GithubExceptionThrower.php Co-authored-by: Jeroen Thora --- lib/Github/HttpClient/Plugin/GithubExceptionThrower.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php index b846691042b..2e6f4722b4b 100644 --- a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php +++ b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php @@ -64,7 +64,7 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla $code = $error['code']; } - switch ($code) { + switch ($error['code'] ?? null) { case 'missing': $errors[] = sprintf('The %s %s does not exist, for resource "%s"', $error['field'], $error['value'], $error['resource']); break; From e000f001037a2d89ede35a3b8ec2d8cc2a1dac5b Mon Sep 17 00:00:00 2001 From: Markus Staab <47448731+clxmstaab@users.noreply.github.com> Date: Tue, 30 Mar 2021 10:08:08 +0200 Subject: [PATCH 4/6] Update GithubExceptionThrower.php --- lib/Github/HttpClient/Plugin/GithubExceptionThrower.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php index 2e6f4722b4b..fa3bb932188 100644 --- a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php +++ b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php @@ -59,11 +59,6 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla if (422 === $response->getStatusCode() && isset($content['errors'])) { $errors = []; foreach ($content['errors'] as $error) { - $code = null; - if (isset($error['code'])) { - $code = $error['code']; - } - switch ($error['code'] ?? null) { case 'missing': $errors[] = sprintf('The %s %s does not exist, for resource "%s"', $error['field'], $error['value'], $error['resource']); From 0e0265fab5a0592f253ff6d87e55663db4626d4a Mon Sep 17 00:00:00 2001 From: Markus Staab <47448731+clxmstaab@users.noreply.github.com> Date: Tue, 30 Mar 2021 10:13:22 +0200 Subject: [PATCH 5/6] fix CS --- .../Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php b/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php index 26d6bdfadf8..d4d7dcbe6a6 100644 --- a/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php +++ b/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php @@ -206,7 +206,7 @@ public static function responseProvider() [ 'message' => 'Validation Failed', 'errors' => ['We cannot delete an active deployment unless it is the only deployment in a given environment.'], - 'documentation_url' => 'https://docs.github.com/rest/reference/repos#delete-a-deployment' + 'documentation_url' => 'https://docs.github.com/rest/reference/repos#delete-a-deployment', ] ) ), From 1ae94ab478cac73f5465cde685caa08fdd370c73 Mon Sep 17 00:00:00 2001 From: Jeroen Thora Date: Sun, 4 Apr 2021 19:42:07 +0200 Subject: [PATCH 6/6] Improve single message errors and testcase --- lib/Github/HttpClient/Plugin/GithubExceptionThrower.php | 6 ++++++ .../Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php index fa3bb932188..7a4bc1e0ba8 100644 --- a/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php +++ b/lib/Github/HttpClient/Plugin/GithubExceptionThrower.php @@ -81,6 +81,12 @@ public function doHandleRequest(RequestInterface $request, callable $next, calla break; default: + if (is_string($error)) { + $errors[] = $error; + + break; + } + if (isset($error['message'])) { $errors[] = $error['message']; } diff --git a/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php b/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php index d4d7dcbe6a6..e57f2cce70e 100644 --- a/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php +++ b/test/Github/Tests/HttpClient/Plugin/GithubExceptionThrowerTest.php @@ -37,7 +37,7 @@ public function testHandleRequest(ResponseInterface $response, ExceptionInterfac if ($exception) { $this->expectException(get_class($exception)); $this->expectExceptionCode($exception->getCode()); - $this->expectExceptionMessage($exception->getMessage()); + $this->expectExceptionMessageRegExp('/'.preg_quote($exception->getMessage(), '/').'$/'); } $plugin->doHandleRequest( @@ -210,7 +210,7 @@ public static function responseProvider() ] ) ), - 'exception' => new \Github\Exception\ValidationFailedException('Validation Failed', 422), + 'exception' => new \Github\Exception\ValidationFailedException('Validation Failed: We cannot delete an active deployment unless it is the only deployment in a given environment.', 422), ], ]; }