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

Custom recover at connection level #287

Open
bfwbbrj opened this issue Mar 24, 2023 · 4 comments
Open

Custom recover at connection level #287

bfwbbrj opened this issue Mar 24, 2023 · 4 comments

Comments

@bfwbbrj
Copy link
Contributor

bfwbbrj commented Mar 24, 2023

During my development process, I had a panic in one of my client connection (specifically, one of the callbacks) and it brought the whole application down. I believe it's essential to add some sort of recover option to enable panic recover for each connection to make sure the failure of one connection won't affect the whole application.

@bfwbbrj
Copy link
Contributor Author

bfwbbrj commented Apr 10, 2023

@FZambia

@FZambia
Copy link
Member

FZambia commented Apr 10, 2023

Hello, I am not sure about this idea. Centrifuge itself must not panic - this should be fixed ASAP. I also have this opinion regarding running applications in production. Here with Centrifuge we have a more difficult situation then with HTTP requests - connections are stateful and in most cases we don't know what to do in case of panic. This may theoretically lead to unexpected side effects.

I suppose you can add recovery logic on your app level?

@bfwbbrj
Copy link
Contributor Author

bfwbbrj commented Apr 10, 2023

connections are stateful and in most cases we don't know what to do in case of panic. This may theoretically lead to unexpected side effects.

In case of panic, maybe we can just close the client that panics? This way we can ensure rest of the clients remain unaffected. Currently if this happens, all other connections are closed as well.

I suppose you can add recovery logic on your app level?

I did, but it didn't stop other clients from getting closed :/

@FZambia
Copy link
Member

FZambia commented Apr 10, 2023

I did, but it didn't stop other clients from getting closed :/

Could you elaborate more on this? If you added recovery then it should fully solve the case - or am I missing sth here?

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

No branches or pull requests

2 participants