pusher / pusher-http-php

PHP library for interacting with the Pusher Channels HTTP API
https://pusher.com/docs/server_api_guide
1.42k stars 310 forks source link

How to Properly Handle json_decode Error with Guzzle Stream in Pusher Library? #390

Open joenyambura opened 5 months ago

joenyambura commented 5 months ago

I recently encountered an issue with the Pusher PHP Server library in my Laravel application.

The error I faced is:

TypeError: json_decode(): Argument #1 ($json) must be of type string, GuzzleHttp\Psr7\Stream given in /var/www/-my-app/vendor/pusher/pusher-php-server/src/Pusher.php:639

Problem The error occurs because the json_decode function is receiving a GuzzleHttp\Psr7\Stream instead of a string. This typically happens when working with HTTP responses in Guzzle.

Solution I Found To resolve this issue, I modified the post method in the Pusher library to ensure the response body is converted to a string before passing it to json_decode. Here’s the modified code:

public function post(string $path, $body, array $params = [])
{
    $path = $this->settings['base_path'] . $path;

    $params['body_md5'] = md5($body);

    $params_with_signature = $this->sign($path, 'POST', $params);

    $headers = [
        'Content-Type' => 'application/json',
        'X-Pusher-Library' => 'pusher-http-php ' . self::$VERSION,
    ];

    try {
        $response = $this->client->post(ltrim($path, '/'), [
            'query' => $params_with_signature,
            'body' => $body,
            'http_errors' => false,
            'headers' => $headers,
            'base_uri' => $this->channels_url_prefix(),
            'timeout' => $this->settings['timeout'],
        ]);
    } catch (ConnectException $e) {
        throw new ApiErrorException($e->getMessage());
    }

    $status = $response->getStatusCode();

    if ($status !== 200) {
        $body = (string) $response->getBody();
        throw new ApiErrorException($body, $status);
    }

    // Convert the response body to a string before decoding
    $responseBodyContents = $response->getBody()->getContents();

    try {
        $response_body = json_decode($responseBodyContents, false, 512, JSON_THROW_ON_ERROR);
    } catch (JsonException $e) {
        throw new PusherException('Data decoding error.');
    }

    return $response_body;
}

My Dilemma While this solution works, modifying vendor files is not recommended because these changes will be lost during package updates.

How can I fix this permanently to avoid bugs when Pusher package is updated?

braindawg commented 1 month ago

Currently the Pusher class is more opinionated than its public API indicates (e.g. constructor takes a ClientInterface, but the internals clearly require a \GuzzleHttp\Client since they rely on methods like get() and post() that aren't provided by the ClientInterface). I suspect that while Pusher::get() and Pusher::post() are exposed as public methods, they aren't actually intended to be used as such, given the total lack of mention of those functions in the docs.

We're not using the get or post methods directly in our integration. Can I ask why you're using post instead of trigger or one of the other library-specific helper functions?

If you absolutely need this behavior, you could set up your own class that extends the Pusher class and overrides the post method with your custom logic. That comes with all the attendant risks of inheritance, though.