ably / laravel-broadcaster

Official Laravel Ably Broadcaster
https://laravel.com/docs/broadcasting
Apache License 2.0
40 stars 7 forks source link

Using ->toOthers() results in a "Ably error: Malformed message; invalid connectionKey" #38

Closed diversitude closed 3 months ago

diversitude commented 4 months ago

Triggering an event using toOthers() with X-Socket-ID in the request header results in an "Ably error: Malformed message; invalid connectionKey"

┆Issue is synchronized with this Jira Task by Unito

sacOO7 commented 4 months ago

This is very unlikely to happen. How frequently does this happen? Also, what are the client side errors in laravel-echo ( if any )

diversitude commented 4 months ago

No client side errors as the event doesn't dispatch from Laravel- the job fails. It does work if the incoming request doesn't contain the X-SOCKET-ID header. The project I'm working on is running Laravel 8.83.27, couid that be an issue?

sacOO7 commented 4 months ago

Okay, maybe you are using old ably broadcaster. Are you using pusher-js + echo at client side. Then you will get the same error -> https://faqs.ably.com/why-isnt-the-broadcast-only-to-others-functionality-working-in-laravel-with-the-ably-broadcaster. To fix this error, you need to install explicitly

composer require ably/laravel-broadcaster

and then install

npm install @ably/laravel-echo ably

at client side. Since you are planning to migrate to new setup, follow

https://github.com/ably/laravel-broadcaster?tab=readme-ov-file#migrating-from-pusherpusher-compatible-broadcasters

diversitude commented 4 months ago

I originally followed the Pusher integration, but presence channels weren't behaving as expected. So I switched to using the ably/laravel-broadcaster and the ably fork of Echo and now presence works as expected- it's just this issue now. I have run php artisan optimize:clear since I installed the new packages and the issue persists.

sacOO7 commented 4 months ago

Ideally, it shouldn't persist. There might be something wrong with authentication being performed, wdyt?

sacOO7 commented 4 months ago

Also, make sure you are using php>=8 and followed proper setup.

diversitude commented 4 months ago

I will keep digging, it's not a show stopper at the moment as the core functionality works and we can implement a workaround for not triggering the callback on the client that fired the request.

We are running PHP 8.2 in our environment.

sacOO7 commented 4 months ago

I am not sure how it can work if it doesn't contain X-SOCKET-ID header. X-SOCKET-ID header is ably connection key. You can access the same using

window.Echo.connector.ably.connection.key

And make sure, your client side config. is

import Echo from '@ably/laravel-echo';
import * as Ably from 'ably';

window.Ably = Ably; // make globally accessible to Echo
window.Echo = new Echo({
    broadcaster: 'ably',
});
sacOO7 commented 4 months ago

Btw, happy to hear presence is working for you : )

diversitude commented 4 months ago

I am triggering message send endpoints via RapidAPI and just using the same JWT bearer token and X-SOCKET-ID as my actual UI in the headers. I will test using the actual axios instance in the app now- hopefully that's the issue.

sacOO7 commented 4 months ago

Please note that, only following interceptors are supported to capture X-SOCKET-ID as per official laravel-echo, others will not work https://github.com/ably-forks/laravel-echo/blob/ddfeba76b73028122b94e58c433e2a7fa2dc3c62/src/echo.ts#L123

diversitude commented 4 months ago

The issue persists using the axios instance in the app with all headers present.

[2024-02-28 20:09:30] local.ERROR: Ably error: Malformed message; invalid connectionKey. (See https://help.ably.io/error/40006 for help.) {"exception":"[object] (Illuminate\\Broadcasting\\BroadcastException(code: 0): Ably error: Malformed message; invalid connectionKey. (See https://help.ably.io/error/40006 for help.) at /Users/calvin/www/stratosfiaAPI/vendor/ably/laravel-broadcaster/src/AblyBroadcaster.php:187)

sacOO7 commented 4 months ago

What is the value of X-Socket-Id ? Can you double check with local connection.key ?

diversitude commented 4 months ago

Getting e91lyWF6gBZ7fm!m1zfd-NJj6AW1gTIHbt8mI-53d74

diversitude commented 4 months ago

It is present in the headers in dev tools for all XHR requests

sacOO7 commented 4 months ago

Ohkay, can you double check if it's same as window.Echo.connector.ably.connection.key

diversitude commented 4 months ago

I've wrapped echo as a vue plugin, so it matches the output of console.log("conn key", this.echo.connector.ably.connection.key);

diversitude commented 4 months ago

`import Echo from '@ably/laravel-echo'; import * as Ably from 'ably'; window.Ably = Ably;

class EchoManager {

constructor() {
    this.echo = null;
    this.apiKey = null;
    this.options = null;
}

init(apiKey, options) {
    if (this.echo) {
        console.warn("Echo instance already initialized.");
        return;
    }
    this.apiKey = apiKey;
    this.options = options;
}

connectEcho() {
    if (this.echo) {
        console.warn("Echo instance already initialized.");
        return;
    }
    this.echo = new Echo({
        broadcaster: 'ably',
        ...this.options,
        auth: {
            headers: {
                Authorization: `Bearer ${localStorage.getItem('x_token')}`,
            },
        },
    });
}

refreshToken() {
    // Update the auth headers for the Pusher connector
    this.echo.connector.options.auth.headers['Authorization'] = `Bearer ${localStorage.getItem('x_token')}`;
    // Optionally, force a disconnect and reconnect if needed to ensure the new token takes effect
    //this.echo.connector.pusher.disconnect(); //NOT SURE IF CORRECT IMPLEMENTATION FOR ABLY FORK 
    //this.echo.connector.pusher.connect();
}

subscribeToChannel(channelName, eventName, callback) {
    if (!this.echo) {
        console.error("Echo instance not initialized.");
        return;
    }

    this.echo.channel(channelName).listen(eventName, callback);
}

subscribeToPrivateChannel(channelName, eventName, callback) {
    if (!this.echo) {
        console.error("Echo instance not initialized.");
        return;
    }

    this.echo.private(channelName).listen(eventName, callback);
}

// Method to subscribe to a presence channel and listen to events
subscribeToPresenceChannel(channelName, events) {
    if (!this.echo) {
        console.error("Echo is not initialized.");
        return;
    }

    const channel = this.echo.join(channelName);

    // 'here' callback provides the initial list of users present in the channel
    if (events.onHere) {
        channel.here((users) => {
            events.onHere(users);
        });
    }

    // 'joining' callback is called when a new user joins the channel
    if (events.onJoining) {
        channel.joining((user) => {
            events.onJoining(user);
        });
    }

    // 'leaving' callback is called when a user leaves the channel
    if (events.onLeaving) {
        channel.leaving((user) => {
            events.onLeaving(user);
        });
    }

    // This example assumes you're using custom events as well
    if (events.onEvent) {
        Object.keys(events.onEvent).forEach(eventName => {
            channel.listen(eventName, (data) => {
                events.onEvent[eventName](data);
            });
        });
    }
}

// Method to leave/unsubscribe from any channel
leaveChannel(channelName) {
    if (!this.echo) {
        console.error("Echo is not initialized.");
        return;
    }
    // Unsubscribe from the channel
    this.echo.leave(channelName);
}

// Method to get the Echo socket ID
getSocketId() {
    if (!this.echo) {
        console.error("Echo is not initialized.");
        return null;
    }
    console.log("conn key", this.echo.connector.ably.connection.key);
    return this.echo.socketId();
}

}

// Vue Plugin export default { install(Vue, { apiKey, options }) { const echoManager = new EchoManager(); echoManager.init(apiKey, options); Vue.prototype.$echoManager = echoManager; }, };`

diversitude commented 4 months ago

The client side code seems to be working fine from what I can see. I'm tempted to just go through the rigmarole of upgrading the project to Laravel 10 and hope it resolves the issue.

sacOO7 commented 4 months ago

Okay, I'm not sure why you are using root api key in your client application. We haven't documented anywhere to set the api-key at client side. We have explicit note not to set it. Since, you have set it server side, you don't need to set it client side. You can remove it from your client code and it will work : )

diversitude commented 4 months ago

That is not the root api key. auth: { headers: { Authorization:Bearer ${localStorage.getItem('x_token')}, }, }, is used to set the current JWT token for the client for that my API middleware with authenticate the incoming /broadcast/auth request

sacOO7 commented 4 months ago

In the setup section, we set it server side => https://github.com/ably/laravel-broadcaster?tab=readme-ov-file#setup. Client automatically gets JWT from server using this API key. Also, don't set

auth: {
            headers: {
                Authorization: `Bearer ${localStorage.getItem('x_token')}`,
            },
        },

We have also documented not to set auth specific options -> https://github.com/ably-forks/laravel-echo?tab=readme-ov-file#installation

diversitude commented 4 months ago

oh this.apiKey is a relic from the previous pusher integration. it is redundant

sacOO7 commented 4 months ago

If you explicitly want to request token, follow section => https://github.com/ably-forks/laravel-echo?tab=readme-ov-file#working-with-laravel-sanctum-support-channel-auth-using-custom-implementation

sacOO7 commented 4 months ago

Also, this is mostly not needed. ably-laravel-echo automatically makes request to given endpoint using internal http request and retrieves jwt.

sacOO7 commented 4 months ago

Please do star the repo, if issue is resolved : )

diversitude commented 4 months ago

Will get the actual front-end devs to have a look at the UI integration in detail and let you know if we resolve the issue. Thanks for the prompt responses :) much appreciated!

sacOO7 commented 4 months ago

Btw, do star the repo. I will be available if new queries come up 👍

sacOO7 commented 4 months ago

Hi @diversitude let me know if we have update on this. Otherwise we can close the issue and open a new one if needed : )

sacOO7 commented 4 months ago

@diversitude do we have updates on this

diversitude commented 4 months ago

Have been traveling, back in the codebase today and will provide updates if we've managed to fix it.

sacOO7 commented 4 months ago

Sure, that would be great !

sacOO7 commented 4 months ago

@diversitude let me know if the fix is working for you. For reference, I recommend to go through full documentation under https://github.com/ably-forks/laravel-echo

sacOO7 commented 3 months ago

@diversitude let me know if this issue is resolved. Otherwise, we can close this and you can create a new issue if you find a bug in the future

diversitude commented 3 months ago

Hey, Sorry for the delayed response. I haven't gotten round to bumping the project to Laravel 11 yet- been in the midst of some big feature sprints. We are still just implementing workarounds on the frontend to not catch events broadcast by the current user. We can close this and I will re-open if it persists on Laravel 11.