amirkhiz / resellerclub-php-api

PHP SDK for ResellerClub API. Currently all of the domains, contacts and customers API are available.
MIT License
25 stars 24 forks source link

Standardize API for PSR-18 (HTTP Client) #19

Open rytisder opened 4 years ago

rytisder commented 4 years ago

As for recommendations states: A Client MUST NOT treat a well-formed HTTP request or HTTP response as an error condition. For example, response status codes in the 400 and 500 range MUST NOT cause an exception and MUST be returned to the Calling Library as normal.

So basically what we should do is refactor this part:

        try {
            return $this->parse(
                $this->guzzle->get(
                    $this->api.'/'.$prefix.$method.'.json?'.preg_replace(
                        '/%5B[0-9]+%5D/simU',
                        '',
                        http_build_query(
                            array_merge($args, $this->authentication)
                        )
                    )
                )
            );
        } catch (ClientException $e) {
            return $this->parse($e->getResponse());
        } catch (ServerException $e) {
            return $this->parse($e->getResponse());
        } catch (BadResponseException $e) {
            return $this->parse($e->getResponse());
        } catch (Exception $error) {
            return $error;
        }
    }

We should just add 'http_erros` flag on instanciating Guzzle client, and handle all 4xx, 5xx by our selfs.

$this->guzzle = new Guzzle(
 'http_errors' => false

Also, we should never ever return exception as an object from API call like this:

catch (Exception $error) {
            return $error;
        }

So we would return all responses in an array and raise an exception if LogicBoxes response contains Action status strtolower(ERROR). Since this would be a major change, I propose to release it as V.2.0.0 to not break existing libs

@see https://www.php-fig.org/psr/psr-18/

amirkhiz commented 4 years ago

@rytisder You are completely right I will go through it. I really missed returning exception part of our code it is a completely wrong way to handle errors as you mentioned. Thanks for this great issue. 👍