search

jupyterlab / jupyter-collaboration

A Jupyter Server Extension Providing Support for Y Documents
https://jupyterlab-realtime-collaboration.readthedocs.io/en/latest/
Other
140 stars 29 forks source link

handle exception when websocket server start room failed #289

Closed jzhang20133 closed 2 months ago

jzhang20133 commented 2 months ago

In this PR, we handle exception when websocket server start room failed by cleaning up fileLoader and room and user won't run into the two collaboration pop up due to websocket server task group inactive failure. This PR also move authentication logic ahead of room initialization.

Resolving: https://github.com/jupyterlab/jupyter-collaboration/issues/291 https://github.com/jupyterlab/jupyter-collaboration/issues/245

github-actions[bot] commented 2 months ago

Binder :point_left: Launch a Binder on branch jzhang20133/jupyter-collaboration/main-2

jzhang20133 commented 2 months ago

@davidbrochart I have added a unit test to make sure two collaborative warning event is not thrown and room and file is deleted when both websocket requests fails due to webserver task group is no longer active.

Zsailer commented 2 months ago

Awesome work, @jzhang20133!

It looks like PR also fixes #291 by moving the prepare super call earlier in this override 🚀

Zsailer commented 2 months ago

@jzhang20133 thanks for this!

Just a minor comment about the prepare method. Does everything need to be under the condition that prepare is an awaitable?

Maybe a simpler approach is to call res = await ensure_async(super().prepare()).

jzhang20133 commented 2 months ago

Addressed comments. @davidbrochart and @Zsailer would you like to review this PR again?

Zsailer commented 2 months ago

Looks great! Thanks @jzhang20133!