mghoneimy/php-graphql-client

JSON decode failures

wucdbm opened this issue · 1 comments

Hey,

We've been experiencing the following issue: array_key_exists(): Argument #2 ($array) must be of type array, null given

It seems that for whatever reason, the server on the other end (well outside of our control) is flaky and every now and then would fail. Then, trying to json_decode fails and null is returned in the constructor of Results, so array_key_exists then fails.

I'd suggest using strict json_decode throwing an exception, but in that case, my concern would be actually also exposing the response contents as a string, so we could at least ping our integrations with a "hey, this and that is going on with the API". Otherwise, they usually tell us "it's running fine".

Would you consider extending QueryError to also include the response as string (possibly nullable?), and in case the json_decode failed, instantiating it with an internal "fake" error array such as $errorDetails['errors'][0]['message'] = $jsonDecodeException->getMessage() and the response?

That way we'll know what happened on the other side. As it currently is, we get 500s and while we can figure out it's the other end that's faulty, we can't really help but let them know, and we do not have any sane way of handling the error gracefully besides catching all TypeError (which could potentially hide other genuine bugs).

I'm willing to do the job, as long as the proposal is OK with you.

Results constructor down here for quick reference:

    public function __construct(ResponseInterface $response, $asArray = false)
    {
        $this->responseObject = $response;
        $this->responseBody   = $this->responseObject->getBody()->getContents();
        $this->results        = json_decode($this->responseBody, $asArray);

        // Check if any errors exist, and throw exception if they do
        if ($asArray) $containsErrors = array_key_exists('errors', $this->results);
        else $containsErrors = isset($this->results->errors);

        if ($containsErrors) {

            // Reformat results to an array and use it to initialize exception object
            $this->reformatResults(true);
            throw new QueryError($this->results);
        }
    }

Is anyone supporting this?