Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement error handling #284

Merged
merged 2 commits into from
Jul 31, 2020
Merged

Implement error handling #284

merged 2 commits into from
Jul 31, 2020

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Jul 31, 2020

This PR implements support for Pusher's pusher:subscription_error event through a dedicated and generic error callback handler. It'll pass along the status as its first argument when a subscription error occurs.

I couldn't find any proper docs on Socket.io's events so I wouldn't know what the equivalent is for Socket.io.

@tillkruss noted that this is applicable for Private Channel as well. I noticed here is also only applied on PresenceChannel. I feel like like both here and error and maybe even joining, leaving and whisper should too? Maybe PrivateChannel should extend PresenceChannel? What are your thoughts @taylorotwell?

Moving just .error to the abstract PusherChannel class.

Because this adds a new method on an interface this probably needs a new major release. Although I don't know what the policy is for Echo so this might just be a minor?

Resolves #177

@driesvints
Copy link
Member Author

Pushed some changes that just allow for an error callback on both present and private channels. By default pusher has the error handling of pusher:subscription_error on both of these.

@taylorotwell taylorotwell merged commit 03002ae into master Jul 31, 2020
@driesvints driesvints deleted the subscription-errors branch July 31, 2020 17:50
@xbrunosousa
Copy link

xbrunosousa commented Aug 16, 2020

Hi! Is it possible to use it in the echo constructor?
Something like:

new Echo({ onError: e => e === 401 ? logout() : null  })

@driesvints
Copy link
Member Author

Not with this implementation.

@Patabugen
Copy link

Patabugen commented Mar 9, 2021

Hey!

I'm trying to implement this to detect two scenarios (and also just basic error handling) but struggling to figure out the syntax.

I don't fully understand the old comment which has been struck-out, it seems to have said that this works on Private, but does it being crossed out mean it doesn't?

The scenarios I'm trying to catch:
a) The laravel-echo-server is not running (e.g. invalid hostname)
b) subscription_error specifically (but also I'd like to catch all errors)

Would someone mind adding error-handling documentation to the Echo docs? Or if you can chuck some examples at me I can make a PR.

This is not valid code but these are some of the things I've tried over the past hour or two from reading various bits of source/PR messages/stack overflow questions/etc.

The only bit which seems to work is .listen('subscription_error', ..., but

    window.Echo = new Echo({
        broadcaster: 'socket.io',
        host: process.env.MIX_WEBSOCKET_HOST,
        onError: console.error,
    }).error(function(error, type) {
           console.error(error);
           console.error(type);
    }).listen('subscription_error', (e) => {
        console.log(e)
   });


    Echo.private(`my_private_channel`)
        .on('subscription_error', function (status, error) {
            console.log(status);
            console.log(error);
        })
        .error(function(error, type) {
           console.error(error);
           console.error(type);
       }).listen('subscription_error', (e) => {
            console.log(e);
        })
        .listen('DriveStarted', (e) => {
    })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support "pusher:subscription_error" event
4 participants