Add response id to request error. (bunq/sdk_php#88) #93

OGKevin · 2017-12-26T14:24:02Z

Closes #88

Tested

OGKevin · 2017-12-27T08:31:04Z

@sandervdo please 👀

sandervdo

@OGKevin all yours. 😄

sandervdo · 2017-12-27T08:50:12Z

src/Exception/ExceptionFactory.php

-    {
+    private static function generateErrorMessage(
+        int $responseCode,
+        array $messages,


sandervdo · 2017-12-27T08:50:23Z

src/Exception/ExceptionFactory.php

-    {
-        $errorMessage = static::generateErrorMessage($responseCode, $messages);
+    public static function createExceptionForResponse(
+        array $messages,


sandervdo · 2017-12-27T08:51:33Z

src/Exception/ExceptionFactory.php

@@ -20,6 +20,8 @@ class ExceptionFactory
     * Formatting constants
     */
    const FORMAT_RESPONSE_CODE_LINE = 'HTTP Response Code: %s';
+    const FORMAT_RESPONSE_ID = 'The response id to help bunq debug: %s';
+    const FORMAT_ERROR_MESSAGE_LINE = 'Error message: %s';
    const GLUE_ERROR_MESSAGES = "\n";


ERROR_MESSAGE_GLUE? ERROR_MESSAGE_SEPARATOR might even be better

sandervdo · 2017-12-27T08:52:55Z

src/Http/Handler/ResponseHandlerError.php

+     * The index of the first item in an array.
+     */
+    const INDEX_FIRST = 0;
+
    /**
     * @param ResponseInterface $response
     *


Invalid, as you can see this particular line has not been changed in this pr and therefore only a snippet of the doc block is show. If you look at the original file you see that this doc block is correct 🙃 see:

sdk_php/src/Http/Handler/ResponseHandlerError.php

Line 41 in 1a5188b

* @return ResponseInterface

Ah yes, missed that one. Ignore then. 😄

sandervdo · 2017-12-27T08:53:33Z

src/Http/Handler/ResponseHandlerError.php

@@ -65,11 +80,32 @@ private function fetchErrorDescriptions(array $errorArray): array
    {
        $errorDescriptions = [];


allErrorDescription

sandervdo · 2017-12-27T08:53:47Z

src/Http/Handler/ResponseHandlerError.php

@@ -49,9 +64,9 @@ private function fetchErrorMessages(ResponseInterface $response): array
        $responseBody = $response->getBody();
        $responseBodyInJson = json_decode($responseBody, true);

-        if ($responseBodyInJson !== false){
+        if ($responseBodyInJson !== false) {
            return $this->fetchErrorDescriptions($responseBodyInJson);


fetchAllErrorDescription

sandervdo · 2017-12-27T08:54:23Z

tests/Http/ErrorResponseIdTest.php

@@ -0,0 +1,29 @@
+<?php
+


remove empty line

sandervdo · 2017-12-27T08:55:24Z

tests/Http/ErrorResponseIdTest.php

+     */
+    public function testBadRequestWitResponseId()
+    {
+        $caughtExcretion = null;


Did you mean exception?

sandervdo · 2017-12-27T08:55:45Z

tests/Http/ErrorResponseIdTest.php

+        $caughtExcretion = null;
+
+        try {
+            UserPerson::get(static::$apiContext, 0);


Magic number

OGKevin · 2017-12-27T12:35:50Z

@sandervdo all yours again, please 👀

sandervdo

@OGKevin done. 1 more plural I noticed and see comment on the naming of your constant (might be a SDK specific thing) :)

sandervdo · 2017-12-27T12:37:24Z

src/Exception/ExceptionFactory.php


-        return static::glueMessages(array_merge([$lineResponseCode], $messages));
+        return static::glueMessages([$lineResponseCode, $lineResponseId, $lineErrorMessage]);


Oh crap, I see a plural here now too. 🙈

sandervdo · 2017-12-27T12:38:33Z

src/Exception/ExceptionFactory.php

@@ -78,6 +96,6 @@ private static function generateErrorMessage(int $responseCode, array $messages)
     */
    private static function glueMessages(array $messages): string
    {
-        return implode(self::GLUE_ERROR_MESSAGES, $messages);
+        return implode(self::SEPARATOR_ERROR_MESSAGES, $messages);


Would call it ERROR_MESSAGE_SEPARATOR then, (big -> small)

We normally group constants together, so in this case this constant belongs to the SEPARATOR_ constant group. As you can have multiple separators and then the constant names will begin with SEPARATOR_XXXXXX.

OGKevin · 2017-12-27T12:51:41Z

@sandervdo all yours again, please 👀

OGKevin · 2017-12-27T13:00:32Z

@andrederoos All yours.

andrederoos · 2018-01-02T09:26:13Z

@OGKevin branche is out-of-date

…rror message. (#88)

…#88)

…m a failed request. (#88)

…GES. (#88)

OGKevin · 2018-01-02T09:31:28Z

@andrederoos my bad, updated 🤦‍♂️

OGKevin added this to the 0.12.5 milestone Dec 26, 2017

OGKevin assigned OGKevin and sandervdo Dec 26, 2017

OGKevin requested a review from sandervdo December 27, 2017 08:30

bunq deleted a comment Dec 27, 2017

sandervdo suggested changes Dec 27, 2017

View reviewed changes

bunq deleted a comment Dec 27, 2017

sandervdo suggested changes Dec 27, 2017

View reviewed changes

bunq deleted a comment Dec 27, 2017

sandervdo previously approved these changes Dec 27, 2017

View reviewed changes

OGKevin assigned andrederoos and OGKevin and unassigned OGKevin and sandervdo Dec 27, 2017

OGKevin added the Can be merged label Dec 27, 2017

bunq deleted a comment Dec 27, 2017

OGKevin mentioned this pull request Dec 28, 2017

Add response id to error messages from failed requests bunq/sdk_csharp#63

Closed

OGKevin dismissed sandervdo’s stale review via 7b09710 December 30, 2017 11:08

bunq deleted a comment Dec 30, 2017

OGKevin changed the title ~~Feature/bunq/sdk php#88 add response id to request error~~ Add response id to request error. (bunq/sdk_php#88) Dec 30, 2017

OGKevin added 5 commits January 2, 2018 10:30

Added response id header constants. (#88)

c77832d

Added new responseId field to base exception class. (#88)

1b3aaa5

Altered generateErrorMessage to also include the response id in the e…

1bd4279

…rror message. (#88)

Pass the response id to the exceptions in ExceptionFactory. (#88)

d1ff10b

Added missing spaces for better method formatting. (#88)

872c4a0

OGKevin added 12 commits January 2, 2018 10:30

Added error constant in case response header retrieval fails. (#88)

e201559

Added method to get the response id from the failed request response. (…

33b8e35

…#88)

Added test to make sure that the response header can be retrieved fro…

1a182a6

…m a failed request. (#88)

Optimised imports. (#88)

ad04c24

Removed plural variables. (#88)

c3c5089

Changed constant name of GLUE_ERROR_MESSAGES to SEPARATOR_ERROR_MESSA…

f7178b7

…GES. (#88)

Removed unneeded empty new line. (#88)

041550b

Extracted magic value. (#88)

cd65652

Fixed typo in caughtExcretion to caughtException. (#88)

f96b6b6

Removed plural from glueMessages. (#88)

1f8fbf3

Removed plural parameter from glueAllMessage. (#88)

c9a2379

Reanmed response header constants to the propper name. (#88)

3653cf3

OGKevin force-pushed the feature/bunq/sdk_php#88-add_response_id_to_request_error branch from 7b09710 to 3653cf3 Compare January 2, 2018 09:31

bunq deleted a comment Jan 2, 2018

Merge branch 'develop' into feature/#88-add_response_id_to_request_error

f040d04

andrederoos approved these changes Jan 2, 2018

View reviewed changes

andrederoos added the Reviewed by André label Jan 2, 2018

bunq deleted a comment Jan 2, 2018

andrederoos merged commit 4b645d6 into develop Jan 2, 2018

andrederoos deleted the feature/bunq/sdk_php#88-add_response_id_to_request_error branch January 2, 2018 20:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add response id to request error. (bunq/sdk_php#88) #93

Add response id to request error. (bunq/sdk_php#88) #93

OGKevin commented Dec 26, 2017

OGKevin commented Dec 27, 2017

sandervdo left a comment

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

OGKevin Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

OGKevin commented Dec 27, 2017

sandervdo left a comment

sandervdo Dec 27, 2017

sandervdo Dec 27, 2017

OGKevin Dec 27, 2017 •

edited by sandervdo

Loading

OGKevin commented Dec 27, 2017

OGKevin commented Dec 27, 2017

andrederoos commented Jan 2, 2018

OGKevin commented Jan 2, 2018

		@@ -65,11 +80,32 @@ private function fetchErrorDescriptions(array $errorArray): array
		{
		$errorDescriptions = [];


		return static::glueMessages(array_merge([$lineResponseCode], $messages));
		return static::glueMessages([$lineResponseCode, $lineResponseId, $lineErrorMessage]);

Add response id to request error. (bunq/sdk_php#88) #93

Add response id to request error. (bunq/sdk_php#88) #93

Conversation

OGKevin commented Dec 26, 2017

OGKevin commented Dec 27, 2017

sandervdo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OGKevin commented Dec 27, 2017

sandervdo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OGKevin Dec 27, 2017 • edited by sandervdo Loading

Choose a reason for hiding this comment

OGKevin commented Dec 27, 2017

OGKevin commented Dec 27, 2017

andrederoos commented Jan 2, 2018

OGKevin commented Jan 2, 2018

OGKevin Dec 27, 2017 •

edited by sandervdo

Loading