php-graphql-client icon indicating copy to clipboard operation
php-graphql-client copied to clipboard

JSON decode failures

Open wucdbm opened this issue 3 years ago • 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);
        }
    }

wucdbm avatar Jan 21 '22 18:01 wucdbm

Is anyone supporting this?

wucdbm avatar Mar 24 '22 03:03 wucdbm