always delete ydoc on disconnect

Author: dlqqq

jupyter_rtc_core/app.py

@@ -19,7 +19,7 @@ class RtcExtensionApp(ExtensionApp):
         # global awareness websocket
         # (r"api/collaboration/room/JupyterLab:globalAwareness/?", GlobalAwarenessWebsocket),
         # # ydoc websocket
-        (r"api/collaboration/room/(.*)", YRoomWebsocket),
+        (r"api/collaboration/room/(.*)/?", YRoomWebsocket),
         # # handler that just adds compatibility with Jupyter Collaboration's frontend
         # (r"api/collaboration/session/(.*)", YRoomSessionHandler),
         (r"api/fileid/index", FileIDIndexHandler)

jupyter_rtc_core/websockets/yroom_ws.py

@@ -15,14 +15,18 @@ class YRoomWebsocket(WebSocketHandler):
     yroom: YRoom
     room_id: str
     client_id: str
-    # TODO: change this. we should pass `self.log` from our
-    # `ExtensionApp` to log messages w/ "RtcCoreExtension" prefix
-    log = logging.Logger("TEMP")
 
 
     @property
     def yroom_manager(self) -> YRoomManager:
         return self.settings["yroom_manager"]
+    
+
+    @property
+    def log(self) -> logging.Logger:
+        # TODO: change this because it's kind of a hack. we should probably set
+        # up some way to reference our logger from `self.settings`
+        return self.yroom_manager.log
 
 
     @property
@@ -35,33 +39,28 @@ def contents_manager(self) -> AsyncContentsManager | ContentsManager:
         return self.settings["contents_manager"]
 
 
-    def prepare(self):
-        # Bind `room_id` attribute
-        request_path: str = self.request.path
-        self.room_id = request_path.strip("/").split("/")[-1]
+    def open(self, room_id, *_, **__):
+        # Bind `room_id` as an instance attribute
+        self.room_id = room_id
 
-        # TODO: remove this once globalawareness is implemented
-        if self.room_id == "JupyterLab:globalAwareness":
+        # TODO: remove this later
+        if room_id == "JupyterLab:globalAwareness":
             self.close(1011)
             return
+        
+        self.log.info(f"ROOM ID: {room_id}")
+        self.log.info(f"REQUEST PATH: {self.request.path}")
 
         # Verify the file ID contained in the room ID points to a valid file.
-        fileid = self.room_id.split(":")[-1]
+        fileid = room_id.split(":")[-1]
         path = self.fileid_manager.get_path(fileid)
         if not path:
             raise HTTPError(404, f"No file with ID '{fileid}'.")
-    
-
-    def open(self, *_, **__):
-        # TODO: remove this later
-        if self.room_id == "JupyterLab:globalAwareness":
-            self.close(1011)
-            return
 
         # Create the YRoom
-        yroom = self.yroom_manager.get_room(self.room_id)
+        yroom = self.yroom_manager.get_room(room_id)
         if not yroom:
-            raise HTTPError(500, f"Unable to initialize YRoom '{self.room_id}'.")
+            raise HTTPError(500, f"Unable to initialize YRoom '{room_id}'.")
         self.yroom = yroom
 
         # Add self as a client to the YRoom
@@ -71,6 +70,7 @@ def open(self, *_, **__):
     def on_message(self, message: bytes):
         # TODO: remove this later
         if self.room_id == "JupyterLab:globalAwareness":
+            self.close(1011)
             return
 
         # Route all messages to the YRoom for processing
@@ -82,5 +82,7 @@ def on_close(self):
         if self.room_id == "JupyterLab:globalAwareness":
             return
 
-        self.log.info(f"Closed Websocket to client '{self.client_id}'.")
+        self.log.info(self.close_code)
+        self.log.info(self.close_reason)
+        self.log.info(f"Closed Websocket to client '{self.client_id}' on room '{self.room_id}'.")
         self.yroom.clients.remove(self.client_id)

src/docprovider/ydrive.ts

@@ -143,7 +143,6 @@ export class RtcContentProvider
     if (typeof options.format !== 'string') {
       return;
     }
-    
 
     try {
       const provider = new WebSocketProvider({

src/docprovider/yprovider.ts

@@ -94,13 +94,10 @@ export class WebSocketProvider implements IDocumentProvider {
 
   private async _connect(): Promise<void> {
     // Fetch file ID from the file ID service.
-    let resp = await requestAPI(
-      "api/fileid/index?path=" + this._path, 
-      {
-        method: "POST"
-      }
-    )
-    const fileId = resp["id"];
+    const resp = await requestAPI(`api/fileid/index?path=${this._path}`, {
+      method: 'POST'
+    });
+    const fileId: string = resp['id'];
 
     this._yWebsocketProvider = new YWebsocketProvider(
       this._serverUrl,
@@ -131,18 +128,33 @@ export class WebSocketProvider implements IDocumentProvider {
     this._awareness.setLocalStateField('user', user.identity);
   }
 
+  /**
+   * Handles disconnections from the YRoom Websocket.
+   *
+   * TODO: handle disconnections more gracefully by reseting the YDoc to an
+   * empty state on disconnect. Unfortunately the shared model does not provide
+   * any methods for this, so we are just asking disconnected clients to
+   * refresh for now.
+   *
+   * TODO: distinguish between disconnects when server YDoc history is the same
+   * (i.e. SS1 + SS2 is sufficient), and when the history
+   * differs (client's YDoc has to be reset before SS1 + SS2).
+   */
   private _onConnectionClosed = (event: any): void => {
-    if (event.code === 1003) {
-      console.error('Document provider closed:', event.reason);
-
-      showErrorMessage(this._trans.__('Document session error'), event.reason, [
-        Dialog.okButton()
-      ]);
+    // Log error to console for debugging
+    console.error('WebSocket connection was closed. Close event: ', event);
+
+    // Show dialog to tell user to refresh the page
+    showErrorMessage(
+      this._trans.__('Document session error'),
+      'Please refresh the browser tab.',
+      [Dialog.okButton()]
+    );
 
-      // Dispose shared model immediately. Better break the document model,
-      // than overriding data on disk.
-      this._sharedModel.dispose();
-    }
+    // Delete this client's YDoc by disposing of the shared model.
+    // This is the only way we know of to stop `y-websocket` from constantly
+    // attempting to re-connect.
+    this._sharedModel.dispose();
   };
 
   private _onSync = (isSynced: boolean) => {