fix(realtime): subscribed state

[7ttp]

TL;DR

Delay SUBSCRIBED only for channels that bind postgres_changes callbacks exclusively
and keep the existing timing for mixed channels

Prob:

SUBSCRIBED could be emitted before the Postgres change feed was actually ready
which made postgres-only channels look ready too early

so now basically, wait for the postgres_changes system ready message before emitting SUBSCRIBED
for postgres-only channels while leaving channels with broadcast or presence callbacks unchanged

ref:

• follow up to fix(realtime): wait for postgres_changes system ready before emitting SUBSCRIBED #2029 &
  aligned with Eduardo’s suggestion :D
• closes Premature 'SUBSCRIBED' status for postgres_changes signals incorrect readiness #1599
• fix(realtime): wait for postgres_changes system ready before emitting SUBSCRIBED #2029 (comment)

[pkg-pr-new]

Open in StackBlitz

@supabase/auth-js

npm i https://pkg.pr.new/@supabase/auth-js@2438

@supabase/functions-js

npm i https://pkg.pr.new/@supabase/functions-js@2438

@supabase/postgrest-js

npm i https://pkg.pr.new/@supabase/postgrest-js@2438

@supabase/realtime-js

npm i https://pkg.pr.new/@supabase/realtime-js@2438

@supabase/storage-js

npm i https://pkg.pr.new/@supabase/storage-js@2438

@supabase/supabase-js

npm i https://pkg.pr.new/@supabase/supabase-js@2438

commit: 153d8b0

[mandarini]

@edgurgel thoughts?

[edgurgel]

Hey team I'm going to play with this branch for some days! Thanks for the PR 🙇

[mandarini]

Nice, thanks @edgurgel ! Can you please ping me once you've tested and are ok to merge?

[edgurgel]

Hey @7ttp here is my initial feedback:

• A "normal" join has a timeout that fires if it gets no response from the server. So I think we need a timeout timer here to do something similar. It should use the same timeout we optionally get from subscribe or default to this.socket.timeout
• We need to handle "status" being not "ok" and set the error state can call the callback etc
• We need to call this._resetSubscribeState() on _onError and on the happy path as well when we emitSubscribed I think?

[7ttp]

Hey @7ttp here is my initial feedback:

* A "normal" join has a timeout that fires if it gets no response from the server. So I think we need a timeout timer here to do something similar. It should use the same timeout we optional get from `subscribe` or default to `this.socket.timeout`

* We need to handle "status" being not "ok" and set the error state can call the callback etc

* We need to call `this._resetSubscribeState()` on `_onError` and on the happy path as well when we `emitSubscribed` I think?

addressed! added the pending system ready timeout....
handled non ok system statuses, and reset the subscribe state on both channel error
& successful subscribed emit :) 💚

edit: QQ: any update on this? 🙂

[edgurgel]

Thanks @7ttp ! Will play with the error states and time outs and get back to you! Looking good so far! <3

[edgurgel]

Sorry for the delay!

I've tested with a few error cases and at least one missing thing is:

When there is a timeout a channel with only pg changes does not try to "rejoin" like a normal timeout. So we would need to actually leave the channel and then start a rejoin timer to simulate what a phx_join timeout already cases.

Because this is starting to feel very complicated (and a bit hacky) we discussed with the team and we agreed that we would rather move this to the backend (and option to wait for the subscription on join) and simplify not only this client but all clients.

Appreciate the patience and the help @7ttp. Sorry if we couldn't proceed with this one!

[mandarini]

Thank you @edgurgel

[7ttp]

Sorry for the delay!

I've tested with a few error cases and at least one missing thing is:

When there is a timeout a channel with only pg changes does not try to "rejoin" like a normal timeout. So we would need to actually leave the channel and then start a rejoin timer to simulate what a phx_join timeout already cases.

Because this is starting to feel very complicated (and a bit hacky) we discussed with the team and we agreed that we would rather move this to the backend (and option to wait for the subscription on join) and simplify not only this client but all clients.

Appreciate the patience and the help @7ttp. Sorry if we couldn't proceed with this one!

makes sense, ty! 🙂💚