Skip to content

fix: ensure WebSocket notifications are sent before application quits #4381

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.

Sign up for GitHub

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

Open
cjuriartec wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: ensure WebSocket notifications are sent before application quits #4381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6c3a02
fix: ensure WebSocket notifications are sent before application quits
cjuriartec Mar 23, 2026
18c7f32
Merge branch 'master' into fix/reliable-ws-shutdown
JellyBrick Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/plugins/api-server/backend/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { registerCallback } from '@/providers/song-info';
import { createBackend } from '@/utils';

import { JWTPayloadSchema } from './scheme';
import { registerAuth, registerControl, registerWebsocket } from './routes';
import { registerAuth, registerControl, registerWebsocket, wsNotifyClose } from './routes';

import { APPLICATION_NAME } from '@/i18n';

Expand All @@ -31,8 +31,14 @@ import type {

export const backend = createBackend<BackendType, APIServerConfig>({
async start(ctx) {
const { app } = await import('electron');
Copy link
Copy Markdown
Member

@JellyBrick JellyBrick Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you used await import?

Copy link
Copy Markdown
Member

@ArjixWasTaken ArjixWasTaken Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe they were not aware that vite does tree-shaking

const config = await ctx.getConfig();

// Notify WebSocket clients that playback has stopped before quitting
app.on('before-quit', () => {
wsNotifyClose?.();
});

this.init(ctx);
registerCallback((songInfo) => {
this.songInfo = songInfo;
Expand Down Expand Up @@ -201,6 +207,7 @@ export const backend = createBackend<BackendType, APIServerConfig>({
}
},
end() {
wsNotifyClose?.();
this.server?.close();
this.server = undefined;
},
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/api-server/backend/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export { register as registerControl } from './control';
export { register as registerAuth } from './auth';
export { register as registerWebsocket } from './websocket';
export { register as registerWebsocket, wsNotifyClose } from './websocket';
16 changes: 16 additions & 0 deletions src/plugins/api-server/backend/routes/websocket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ export const register = (
);
};

// Notify all WebSocket clients that playback has stopped (e.g. app closing/hiding)
// We intentionally do NOT close or clear sockets here so that clients like
// Boring Notch keep their connection alive and can receive updates when
// Pear Desktop comes back from hiding.
wsNotifyClose = () => {
console.log(`[API Server WebSocket] NotifyClose called. Sending isPlaying:false to ${sockets.size} clients.`);
send(DataTypes.PlayerStateChanged, {
isPlaying: false,
position: lastSongInfo?.elapsedSeconds ?? 0,
});
};
Comment on lines +63 to +69
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Electron, does the before-quit event wait for asynchronous work to finish, and what is the recommended pattern for delaying quit until a WebSocket notification has been flushed (or timed out)?

💡 Result:

No, the before-quit event does not wait for asynchronous work to finish. The event handler is treated as synchronous; any returned Promise is ignored, and Electron proceeds with quitting regardless. The recommended pattern for delaying quit until a WebSocket notification is flushed (or timed out) is to use a state flag in the main process: 1. Listen for the before-quit event. 2. If cleanup (WebSocket flush) is not complete, call event.preventDefault to cancel the quit. 3. Perform the async WebSocket operation (e.g., send notification and await acknowledgment or use a timeout). 4. Set the flag to indicate completion. 5. Call app.quit manually, which will trigger before-quit again but skip prevention due to the flag. Example code: let hasFlushed = false; async function flushWebSocket { // Send notification over WebSocket // Await ack or use Promise.race with timeout, e.g.: return Promise.race([ websocket.sendAndAwaitAck('notification'), new Promise(resolve => setTimeout(resolve, 5000)) // 5s timeout ]); } app.on('before-quit', (event) => { if (!hasFlushed) { event.preventDefault; flushWebSocket.then( => { hasFlushed = true; app.quit; }).catch( => { hasFlushed = true; app.quit; // Timeout or error: still quit }); } }); This pattern is consistently recommended across sources and works for network operations like WebSocket flushes. Use IPC if the WebSocket is in a renderer process.

Citations:

🏁 Script executed:

# Find the websocket.ts file
fd -type f -name "websocket.ts"

Repository: pear-devs/pear-desktop

Length of output: 236

🏁 Script executed:

# Also check the main.ts file that was mentioned
fd -type f -name "main.ts" | grep -i "api-server\|backend"

Repository: pear-devs/pear-desktop

Length of output: 236

🏁 Script executed:

# Find the websocket.ts file with correct syntax
fd -t f "websocket.ts"

Repository: pear-devs/pear-desktop

Length of output: 116

🏁 Script executed:

# Find main.ts files in the codebase
fd -t f "main.ts" | head -20

Repository: pear-devs/pear-desktop

Length of output: 316

🏁 Script executed:

# Read the websocket.ts file to see the implementation
cat -n "src/plugins/api-server/backend/routes/websocket.ts" | head -180

Repository: pear-devs/pear-desktop

Length of output: 5877

🏁 Script executed:

# Read the main.ts file for the api-server to see how wsNotifyClose is used
cat -n "src/plugins/api-server/backend/main.ts"

Repository: pear-devs/pear-desktop

Length of output: 7381

Return a Promise from wsNotifyClose so the caller can delay shutdown until the notification is flushed.

wsNotifyClose is synchronous and fire-and-forget. Electron's before-quit event ignores Promises from handlers, but the handler can use event.preventDefault() to cancel quit, await async work, then call app.quit() manually. Without a Promise return value, there's no way for the caller to hold process teardown until the WebSocket notification has been written, so this PR still risks missing the final PLAYER_STATE_CHANGED message under the same race condition.

Make wsNotifyClose return a Promise that resolves when the notification has been sent to all clients (or after a reasonable timeout), then update the before-quit handler in main.ts to prevent default, await completion, and manually call app.quit().

Affects lines 63–69 and line 169.

🧰 Tools
🪛 ESLint

[error] 64-64: Replace [API·Server·WebSocket]·NotifyClose·called.·Sending·isPlaying:false·to·${sockets.size}·clients. with ⏎······[API·Server·WebSocket]·NotifyClose·called.·Sending·isPlaying:false·to·${sockets.size}·clients.,⏎····

(prettier/prettier)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/plugins/api-server/backend/routes/websocket.ts` around lines 63 - 69,
wsNotifyClose currently fires the PLAYER_STATE_CHANGED send synchronously and
returns nothing; change wsNotifyClose to return a Promise that resolves when
send has flushed to all clients (or on a short timeout) by tracking per-socket
send completion (use the same sockets collection and the send function that
sends DataTypes.PlayerStateChanged with { isPlaying:false, position:
lastSongInfo?.elapsedSeconds ?? 0 }) and resolving when all sends complete; then
update the main.ts before-quit handler to call event.preventDefault(), await
wsNotifyClose(), and only then call app.quit() so shutdown waits for the
notification to be sent.


const createPlayerState = ({
songInfo,
volumeState,
Expand Down Expand Up @@ -152,3 +164,7 @@ export const register = (
})) as (ctx: Context, next: Next) => Promise<Response>,
);
};

// Exposed so the backend can call it on app close/hide
export let wsNotifyClose: (() => void) | undefined;

Loading