Skip to content

Wait for the server ack when leaving channels - #6792

Closed
patric-vinicios wants to merge 1 commit into
phoenixframework:mainfrom
patric-vinicios:fix-channel-leave-ack
Closed

Wait for the server ack when leaving channels#6792
patric-vinicios wants to merge 1 commit into
phoenixframework:mainfrom
patric-vinicios:fix-channel-leave-ack

Conversation

@patric-vinicios

Copy link
Copy Markdown
Contributor

I ran into this while looking at channel cleanup in an app

leave() was returning "ok" before the server had actually replied.

leave() changes the channel state to leaving and then checks canPush(), but canPush() only returns true when the channel is still joined so at that point it always returns false

Then, the fallback runs right away and the channel closes locally instead of waiting for the server reply or the leave timeout

The fix is just to check canPush() before changing the state to leaving

If the channel can still push, it sends the leave request and waits for either the reply or the timeout. If it cant, it keeps the existing behavior and closes immediately

I also re-enabled the tests for the leaving state and leave timeout

Tested with:

  • npm test -- --runInBand
  • npx eslint assets/js/phoenix/channel.js assets/test/channel_test.js

Comment on lines -1044 to -1047
// TODO - the following tests are skipped until Channel.leave
// behavior can be fixed; currently, 'ok' is triggered immediately
// within Channel.leave so timeout callbacks are never reached
//

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this behavior has been the same since the beginning, I'm inclined to keep it and only change it when we do a 2.0 release.

@SteffenDE

Copy link
Copy Markdown
Member

Closing as per the above. Thanks!

@SteffenDE SteffenDE closed this Aug 18, 2026
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.

2 participants