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

[1.x] Allows for disconnections when broadcasting to others #122

Merged
merged 6 commits into from
Apr 4, 2024

Conversation

cryptoprof
Copy link
Contributor

@cryptoprof cryptoprof commented Mar 29, 2024

Probably will fix issue with the Exception when use toOthers()

Resolves #121

@taylorotwell
Copy link
Member

Probably? 😅

@taylorotwell taylorotwell requested a review from joedixon April 1, 2024 00:40
@taylorotwell taylorotwell marked this pull request as draft April 1, 2024 00:40
@cryptoprof
Copy link
Contributor Author

Probably? 😅

After several years of programming, I often find that code has two quantum states - it works perfectly on the developer's computer, but on the tester's and user's computers, it may or may not work =) When I'm editing someone else's code, especially that of people smarter than me, it often turns out to be not a bug, but a feature.

@joedixon joedixon changed the title probably fix toOthers() broadcast message [1.x] Allows for disconnections when broadcasting to others Apr 4, 2024
@joedixon
Copy link
Collaborator

joedixon commented Apr 4, 2024

This PR does indeed protect against a potential issue when using toOthers in the event the connection you wish to ignore has disconnected before the event has been broadcasted.

I have updated the formatting a little and introduced a test.

@joedixon joedixon marked this pull request as ready for review April 4, 2024 11:10
@RTippin
Copy link

RTippin commented Apr 4, 2024

I can confirm that all of the intermittent Pusher 500 errors I have been observing occur when using toOthers and the client has already left the channel / refreshed entire screen, by the time the job is picked up and processed.

@taylorotwell taylorotwell merged commit 0b5e383 into laravel:main Apr 4, 2024
9 checks passed
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.

Broadcast message toOthers throws ApiException in pusher
4 participants