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

Docs for broadcasting messages don't explain that channels need to be cleaned up #281

Open
mqp opened this issue Apr 1, 2024 · 4 comments
Labels
documentation Improvements or additions to documentation

Comments

@mqp
Copy link
Contributor

mqp commented Apr 1, 2024

Improve documentation

Link

https://supabase.com/docs/reference/javascript/broadcastmessage?example=send-a-message

Describe the problem

If you follow the instructions in the documentation to send a broadcast message over HTTP:

supabase.channel('room1').send({
  type: 'broadcast',
  event: 'cursor-pos',
  payload: { x: Math.random(), y: Math.random()
}})

supabase.channel will create a big puffy RealtimeChannel object and append it to the RealtimeClient.channels array, and never clean it up. So if you just ran this code in your app whenever you wanted to send a message the array would just keep growing full of channels without bounds.

Since I was about to implement a pattern like "our API server broadcasts a message every time new stuff happens", this would mean my API server would have a big memory leak, which would be very bad.

Describe the improvement

You can clean up the channel by calling RealtimeClient.removeChannel, although the code inside removeChannel is very silly looking in the case when the channel isn't connected (it constructs a fake leave push to unsubscribe and then triggers a fake OK on it.)

const chan = supabase.channel('room1');
await chan.send({ /* ... */ });
await supabase.removeChannel(chan)
@mqp mqp added the documentation Improvements or additions to documentation label Apr 1, 2024
@mqp
Copy link
Contributor Author

mqp commented Apr 2, 2024

I guess it goes without saying but it seems like probably the interface should be refactored to not use the RealtimeChannel abstraction in cases where you are just firing off a broadcast message over HTTP, since the purpose of RealtimeChannel is to manage all the state for persistent connections.

@kyeshmz
Copy link

kyeshmz commented Apr 11, 2024

I think this is just a matter of how you manage the state of the channel, whether you use a state library like zustand or something to manage it like you would a singleton.

I think its clear that they remove it automatically, (and I have read it somewhere in the docs)

@filipecabaco
Copy link
Member

Channel management will change due to the new feature of Authz ( more here https://github.com/orgs/supabase/discussions/22484 )

The idea is that if you are using Channel Authorization you will need to manage your channels (CRUD operations) using the new endpoints in our API.

If not, then the channels will auto delete as they are ephemeral in nature in our system and technically do not create any entry in any persistency layer.

@filipecabaco
Copy link
Member

Regarding the JS object itself, I'm unsure of what approach we use 🤔 cc @w3b6x9

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

No branches or pull requests

3 participants