fix: improve overleaf handshake and compile-chain compatibility#352
fix: improve overleaf handshake and compile-chain compatibility#352
Conversation
iamhyc
commented
Mar 21, 2026
iamhyc
commented
There was a problem hiding this comment.
Pull request overview
Improves Overleaf Workshop’s remote compile reliability by updating the Socket.IO project-join handshake behavior (to handle Overleaf stacks that now require projectId on the handshake) and by making the compile pipeline more tolerant so diagnostics can be surfaced even when the compile response isn’t a clean “success”.
Changes:
- Default Socket.IO connection scheme to v2 (handshake includes
projectId), with fallback between v2 and legacy v1 join flow. - Adjust Socket.IO client initialization to pass handshake query via
socket.io-clientoptions rather than appending to the URL. - Make the compile chain more resilient: treat “output files present” as a signal to proceed with log-based error checking, and add a terminal catch for failed chains.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/core/remoteFileSystemProvider.ts |
Updates compile success criteria to proceed when output files are present (enabling log parsing / diagnostics). |
src/compile/compileManager.ts |
Removes rejection-based control flow, adds early-return guard, and adds a catch to mark compile chain failures as failed. |
src/api/socketio.ts |
Switches default scheme to v2, adds bidirectional fallback logic between v1 and v2 join flows, and refactors joinProject handling. |
src/api/base.ts |
Changes Socket.IO connect to pass query via options; adjusts compile endpoint URL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Old Overleaf stacks may not support auto join via handshake. | ||
| // Fallback to legacy joinProject event to keep compatibility. | ||
| this.scheme = 'v1'; | ||
| this.init(); | ||
| return this.joinProjectByV1(project_id); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| return await this.joinProjectByV1(project_id); | ||
| } catch (err) { | ||
| if (!this.shouldFallbackToV2(err)) { | ||
| throw err; | ||
| } | ||
| // New Overleaf stacks require projectId in handshake query. | ||
| this.scheme = 'v2'; | ||
| this.init(); | ||
| return this.joinProjectByV2(); |
There was a problem hiding this comment.
When falling back between v1/v2 you call this.init() to create a new Socket.IO connection, but the existing this.socket is never disconnected first. This can leave multiple live connections and duplicated event handlers (especially if joinProject retries), leading to duplicated events and resource leaks. Consider disconnecting/cleaning up the current socket (and its listeners) before re-initializing, e.g., this.socket?.disconnect() plus removing listeners, or refactor init() to safely re-create the socket.
| const rejectPromise: Promise<ProjectEntity> = new Promise((_, reject) => { | ||
| this.socket.on('connectionRejected', (err:any) => { | ||
| reject(err?.message ?? err); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
joinProjectByV1 registers a connectionRejected listener via this.socket.on(...) but never removes it. If joinProject is called multiple times (or retries occur), these listeners will accumulate and may trigger multiple rejections for a single event. Using once here and/or explicitly removing the listener after the race resolves would prevent listener leaks.
