pusher / pusher-http-php

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

Undefined property: stdClass::$channels v5.0 #295

Closed jordyvanvijfeijken closed 3 years ago

jordyvanvijfeijken commented 3 years ago

Hello,

First of all, I'm using Pusher version 5.0 "pusher/pusher-php-server": "5.0" with Lumen version Laravel Framework Lumen (8.2.3) (Laravel Components ^8.0). My PHP version is 7.4.16.

I'm making a POC with Lumen and Vue.JS. For now it just has to send a "hello world" message from the Lumen back-end to the Vue.JS front-end (which works). I have made an event which is triggered upon loading the page like this:

public function sendMessage(Request $request)
{
    event(new MessageEvent('hello world'));
}

The MessageEvent looks like this (got this from the Pusher getting started help):

class MessageEvent extends Event implements ShouldBroadcast
{
    /**
     * Create a new event instance.
     *
     * @return void
     */
    public $message;

    public function __construct($message)
    {
        $this->message = $message;
    }

    public function broadcastOn()
    {
        return ['my-channel'];
    }

    public function broadcastAs()
    {
        return 'my-event';
    }
}

This part is working, since I'm receiving this in the Vue.JS application:

Pusher :  : ["Event recd",{"event":"my-event","channel":"my-channel","data":{"message":"hello world"}}]

Now comes the problem when I check the queue log. triggered with php artisan queue:listen, I see the following:

[2021-03-14 11:57:03][Bh7373O9EETAZc39M2RCSPmUTjwSbSmL] Processing: App\Events\MessageEvent
[2021-03-14 11:57:04][Bh7373O9EETAZc39M2RCSPmUTjwSbSmL] Failed:     App\Events\MessageEvent

When I check the Lumen log files it says the following:

[2021-03-14 11:43:12] local.ERROR: Undefined property: stdClass::$channels {"exception":"[object] (ErrorException(code: 0): Undefined property: stdClass::$channels at /var/www/vendor/pusher/pusher-php-server/src/Pusher.php:538)

So I went ahead and checkt the Pusher.php file:

536:         $result = json_decode($response['body']);
537:         
538:         if ($result->channels) {
539:             $result->channels = get_object_vars($result->channels);
540:         }

I decided to check what $response was, it gives the following:

[2021-03-14 11:57:04] local.INFO: array (
  'body' => '{}',
  'status' => 200,
)  

Of course it can't get to $result->channels if response["body"]["channels"] doesn't exist.

When I go and check the Pusher API reference it says the following: image

Which would mean that the body should indeed contain a JSON response. But when I scroll a bit further I see this:

image

Which should mean you don't have to set the info parameter, since it's optional and experimental.

[EXPERIMENTAL] If the info parameter is sent, then it returns a hash of unique channels that were triggered to. The hash maps from channel name to a hash of attributes for that channel (may be empty).

The response it expects with the info parameter set is this:

{
  "channels": {
    "presence-foobar": {
      "user_count": 42,
      "subscription_count": 51
    },
    "presence-another": {
      "user_count": 123,
      "subscription_count": 140
    },
    "another": {
      "subscription_count": 13
    }
  }
}

Which is the channels object asked for. Which is not returned cause I don't set the optional info parameter.

elverkilde commented 3 years ago

Can you tell me which version of Laravel you're using? there's an incompatibility between version 5 and Laravel, fixed in Laravel 8.29.0.

elverkilde commented 3 years ago

I'm a little confused regarding the Pusher.php snippet. Line 538 on master looks like this https://github.com/pusher/pusher-http-php/blob/master/src/Pusher.php#L538 which does indeed check if the property is set.

jordyvanvijfeijken commented 3 years ago

@elverkilde Thanks for the quick reply!

I'm using the Lumen framework from Laravel which is a lightweight Microservice framework, but it follows the original Laravel docs for the most. That one is running on version 8.2.3 (Laravel Framework Lumen (8.2.3) (Laravel Components ^8.0)).

The Pusher.php snippet I'm on about is located in /vendor/pusher/pusher-php-server/src/Pusher.php.

It does look an awful lot like the one you just sent, but mine looks like this:

 $result = json_decode($response['body']);
if ($result->channels) {
    $result->channels = get_object_vars($result->channels);
}

I've checked the property_exists (like here: https://github.com/pusher/pusher-http-php/blob/master/src/Pusher.php#L538) and if that one is used, it does indeed work. But it isn't like that in my pusher.php. I don't know where that went wrong.

My composer.json looks like this:

{
    ...
    "require": {
        ...
        "pusher/pusher-php-server": "5.0"
        ...
    },
    ...
}

I've tried to run a composer update but there weren't any updates for the pusher.php package, nor were there updates for Lumen. I've followed the official Laravel docs.

So I don't know where this goes wrong. I guess for now I might create a custom pusher.php in my Docker structure so it overwrites the original.

Sorry if I'm being unclear. If you need any more info let me know.

elverkilde commented 3 years ago

sorry, I just did a test with a clean composer.json with "pusher/pusher-php-server": "5.0" and there is a bug in version 5.0.0. You can fix this by changing your composer requirement to "pusher/pusher-php-server": "^5.0", i.e. the latest patch version (currently 5.0.2).

I will keep this issue open until I have removed the buggy version from packagist.

GrahamCampbell commented 3 years ago

I will keep this issue open until I have removed the buggy version from packagist.

Deleting releases is a bad practice. Anyone who puts 5.0 in their composer.json file has made an error, since composer resolves this to the version 5.0.0.0 and not the version constraint ^5.0.

elverkilde commented 3 years ago

I was thinking the same, thanks @GrahamCampbell. I was looking for a way to mark it as broken in packagist, but it doesn't seem to be possible. I suppose leaving it is my only option?

I did test a fresh composer.json with 5.0, which did resolve to 5.0.0.

GrahamCampbell commented 3 years ago

Yes, because 5.0 is shorthand for 5.0.0.0 in composer. It is not a version constraint, but a hardcoded version.

elverkilde commented 3 years ago

Understood, thanks again!

GrahamCampbell commented 3 years ago

Yeh, there is no way to mark a package version as broken. People should assume new patch releases contain bug fixes, so should always use the latest version of a package, and not hardcode an old version in their composer.json. The purpose of the composer.lock file is to lock dependency versions at a known state for apps, so there's no reason to hardcode versions. :)

elverkilde commented 3 years ago

@jordyvanvijfeijken I am going to close this issue, assuming things will start working once you change the version to ^5.0. Feel free to re-open if this doesn't solve your problem.

jordyvanvijfeijken commented 3 years ago

@elverkilde @GrahamCampbell The issue lied with the composer file indeed.

I changed this in my composer.json file:

{
    ...
    "require": {
        ...
        "pusher/pusher-php-server": "5.0"
        ...
    },
    ...
}

To this:

{
    ...
    "require": {
        ...
        "pusher/pusher-php-server": "^5.0"
        ...
    },
    ...
}

And now it works and I get the newest version (5.0.3) after running a composer update.

Thanks for helping me out!

Just leaving this here in case anyone else encounters the same issue as me.