fix(notifications): harden retries and shutdown
This commit is contained in:
@@ -1287,7 +1287,7 @@ export class AppController {
|
|||||||
return this.manager.getItemLogPath(itemId) || getItemLogPath(itemId);
|
return this.manager.getItemLogPath(itemId) || getItemLogPath(itemId);
|
||||||
}
|
}
|
||||||
|
|
||||||
public shutdown(): void {
|
public async shutdown(): Promise<void> {
|
||||||
if (this.runtimeStatsTimer) {
|
if (this.runtimeStatsTimer) {
|
||||||
clearInterval(this.runtimeStatsTimer);
|
clearInterval(this.runtimeStatsTimer);
|
||||||
this.runtimeStatsTimer = null;
|
this.runtimeStatsTimer = null;
|
||||||
@@ -1295,7 +1295,7 @@ export class AppController {
|
|||||||
stopDebugServer();
|
stopDebugServer();
|
||||||
abortActiveUpdateDownload();
|
abortActiveUpdateDownload();
|
||||||
cancelPendingAsyncSaves();
|
cancelPendingAsyncSaves();
|
||||||
void this.notificationOutbox.drainForShutdown().catch((error) => {
|
await this.notificationOutbox.drainForShutdown(3000).catch((error) => {
|
||||||
logger.warn(`Notification-Outbox konnte beim Beenden nicht geleert werden: ${String(error)}`);
|
logger.warn(`Notification-Outbox konnte beim Beenden nicht geleert werden: ${String(error)}`);
|
||||||
});
|
});
|
||||||
this.manager.prepareForShutdown();
|
this.manager.prepareForShutdown();
|
||||||
|
|||||||
+50
-11
@@ -94,6 +94,41 @@ let controller: AppController;
|
|||||||
let pendingBackupImport: Buffer | null = null;
|
let pendingBackupImport: Buffer | null = null;
|
||||||
const CLIPBOARD_MAX_TEXT_CHARS = 50_000;
|
const CLIPBOARD_MAX_TEXT_CHARS = 50_000;
|
||||||
|
|
||||||
|
export interface BeforeQuitHandlerOptions {
|
||||||
|
cleanup: () => void;
|
||||||
|
shutdown: () => Promise<void>;
|
||||||
|
continueQuit: () => void;
|
||||||
|
onError: (error: unknown) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function createBeforeQuitHandler(options: BeforeQuitHandlerOptions): (event: { preventDefault: () => void }) => void {
|
||||||
|
let shutdownStarted = false;
|
||||||
|
let quitAllowed = false;
|
||||||
|
return (event) => {
|
||||||
|
if (quitAllowed) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
event.preventDefault();
|
||||||
|
if (shutdownStarted) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
shutdownStarted = true;
|
||||||
|
let shutdown: Promise<void>;
|
||||||
|
try {
|
||||||
|
options.cleanup();
|
||||||
|
shutdown = options.shutdown();
|
||||||
|
} catch (error) {
|
||||||
|
shutdown = Promise.reject(error);
|
||||||
|
}
|
||||||
|
void shutdown.catch((error) => {
|
||||||
|
options.onError(error);
|
||||||
|
}).finally(() => {
|
||||||
|
quitAllowed = true;
|
||||||
|
options.continueQuit();
|
||||||
|
});
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
function isDevMode(): boolean {
|
function isDevMode(): boolean {
|
||||||
return process.env.NODE_ENV === "development";
|
return process.env.NODE_ENV === "development";
|
||||||
}
|
}
|
||||||
@@ -1038,16 +1073,20 @@ app.on("window-all-closed", () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
app.on("before-quit", () => {
|
app.on("before-quit", createBeforeQuitHandler({
|
||||||
if (updateQuitTimer) { clearTimeout(updateQuitTimer); updateQuitTimer = null; }
|
cleanup: () => {
|
||||||
stopClipboardWatcher();
|
if (updateQuitTimer) { clearTimeout(updateQuitTimer); updateQuitTimer = null; }
|
||||||
destroyTray();
|
stopClipboardWatcher();
|
||||||
shutdownDaemon();
|
destroyTray();
|
||||||
if (controller) {
|
shutdownDaemon();
|
||||||
try {
|
},
|
||||||
controller.shutdown();
|
shutdown: async () => {
|
||||||
} catch (error) {
|
if (controller) {
|
||||||
logger.error(`Fehler beim Shutdown: ${String(error)}`);
|
await controller.shutdown();
|
||||||
}
|
}
|
||||||
|
},
|
||||||
|
continueQuit: () => app.quit(),
|
||||||
|
onError: (error) => {
|
||||||
|
logger.error(`Fehler beim Shutdown: ${String(error)}`);
|
||||||
}
|
}
|
||||||
});
|
}));
|
||||||
|
|||||||
@@ -132,7 +132,7 @@ function oldestIndex(events: NotificationEvent[], predicate: (event: Notificatio
|
|||||||
}
|
}
|
||||||
|
|
||||||
function retryDelayMs(attempts: number): number {
|
function retryDelayMs(attempts: number): number {
|
||||||
return Math.min(MAX_RETRY_DELAY_MS, 1000 * (2 ** Math.min(9, Math.max(0, attempts - 1))));
|
return Math.min(MAX_RETRY_DELAY_MS, 1000 * (2 ** Math.min(30, Math.max(0, attempts - 1))));
|
||||||
}
|
}
|
||||||
|
|
||||||
export class NotificationOutbox {
|
export class NotificationOutbox {
|
||||||
@@ -153,6 +153,9 @@ export class NotificationOutbox {
|
|||||||
this.clock = options.now || Date.now;
|
this.clock = options.now || Date.now;
|
||||||
this.autoDrain = Boolean(options.autoDrain);
|
this.autoDrain = Boolean(options.autoDrain);
|
||||||
this.load();
|
this.load();
|
||||||
|
if (this.autoDrain && this.events.length > 0) {
|
||||||
|
this.scheduleDrain(Math.max(0, this.events[0].nextAttemptAt - this.clock()));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public async enqueue(event: NotificationEvent): Promise<void> {
|
public async enqueue(event: NotificationEvent): Promise<void> {
|
||||||
@@ -170,11 +173,15 @@ export class NotificationOutbox {
|
|||||||
|
|
||||||
public drain(now?: number): Promise<void> {
|
public drain(now?: number): Promise<void> {
|
||||||
return this.runExclusive(async () => {
|
return this.runExclusive(async () => {
|
||||||
const drainAt = finiteInteger(now ?? this.clock());
|
let currentNow = finiteInteger(now ?? this.clock());
|
||||||
this.enforceLimits(drainAt);
|
this.enforceLimits(currentNow);
|
||||||
while (this.events.length > 0) {
|
while (this.events.length > 0) {
|
||||||
const current = this.events[0];
|
const current = this.events[0];
|
||||||
if (current.nextAttemptAt > drainAt) {
|
if (current.nextAttemptAt > currentNow) {
|
||||||
|
await this.persist(currentNow);
|
||||||
|
if (this.autoDrain) {
|
||||||
|
this.scheduleDrain(Math.max(0, current.nextAttemptAt - this.clock()));
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
let sent = false;
|
let sent = false;
|
||||||
@@ -183,23 +190,25 @@ export class NotificationOutbox {
|
|||||||
} catch {
|
} catch {
|
||||||
sent = false;
|
sent = false;
|
||||||
}
|
}
|
||||||
|
const outcomeAt = finiteInteger(this.clock(), currentNow);
|
||||||
if (!sent) {
|
if (!sent) {
|
||||||
current.attempts += 1;
|
current.attempts += 1;
|
||||||
current.nextAttemptAt = drainAt + retryDelayMs(current.attempts);
|
current.nextAttemptAt = outcomeAt + retryDelayMs(current.attempts);
|
||||||
this.lastFailureAt = drainAt;
|
this.lastFailureAt = outcomeAt;
|
||||||
await this.persist(drainAt);
|
await this.persist(outcomeAt);
|
||||||
if (this.autoDrain) {
|
if (this.autoDrain) {
|
||||||
this.scheduleDrain(Math.max(0, current.nextAttemptAt - this.clock()));
|
this.scheduleDrain(Math.max(0, current.nextAttemptAt - this.clock()));
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
this.events.shift();
|
this.events.shift();
|
||||||
this.lastSuccessAt = drainAt;
|
this.lastSuccessAt = outcomeAt;
|
||||||
await this.persist(drainAt);
|
await this.persist(outcomeAt);
|
||||||
|
currentNow = finiteInteger(this.clock(), outcomeAt);
|
||||||
}
|
}
|
||||||
if (this.events.length === 0) {
|
if (this.events.length === 0) {
|
||||||
this.clearRetryTimer();
|
this.clearRetryTimer();
|
||||||
await this.persist(drainAt);
|
await this.persist(finiteInteger(this.clock(), currentNow));
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,100 @@
|
|||||||
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
|
const electron = vi.hoisted(() => {
|
||||||
|
const handlers = new Map<string, (...args: unknown[]) => void>();
|
||||||
|
return {
|
||||||
|
handlers,
|
||||||
|
app: {
|
||||||
|
isPackaged: false,
|
||||||
|
getPath: vi.fn(() => "C:\\MDD\\Test"),
|
||||||
|
getAppPath: vi.fn(() => "C:\\MDD\\App"),
|
||||||
|
requestSingleInstanceLock: vi.fn(() => true),
|
||||||
|
on: vi.fn((name: string, handler: (...args: unknown[]) => void) => { handlers.set(name, handler); }),
|
||||||
|
whenReady: vi.fn(() => new Promise<void>(() => {})),
|
||||||
|
quit: vi.fn(),
|
||||||
|
exit: vi.fn(),
|
||||||
|
setPath: vi.fn()
|
||||||
|
}
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mock("electron", () => ({
|
||||||
|
app: electron.app,
|
||||||
|
BrowserWindow: class {
|
||||||
|
public static getAllWindows(): unknown[] { return []; }
|
||||||
|
},
|
||||||
|
clipboard: {},
|
||||||
|
dialog: {},
|
||||||
|
ipcMain: { handle: vi.fn(), on: vi.fn() },
|
||||||
|
Menu: { buildFromTemplate: vi.fn(), setApplicationMenu: vi.fn() },
|
||||||
|
safeStorage: { isEncryptionAvailable: () => false, encryptString: vi.fn(), decryptString: vi.fn() },
|
||||||
|
shell: { openExternal: vi.fn() },
|
||||||
|
Tray: class {}
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { AppController } from "../src/main/app-controller";
|
||||||
|
|
||||||
|
function deferred(): { promise: Promise<void>; resolve: () => void } {
|
||||||
|
let resolve = () => {};
|
||||||
|
const promise = new Promise<void>((done) => { resolve = done; });
|
||||||
|
return { promise, resolve };
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("main shutdown lifecycle", () => {
|
||||||
|
it("AppController waits for the bounded outbox drain before disposing runtime owners", async () => {
|
||||||
|
const drain = deferred();
|
||||||
|
const manager = { prepareForShutdown: vi.fn() };
|
||||||
|
const controller = Object.create(AppController.prototype) as any;
|
||||||
|
controller.runtimeStatsTimer = null;
|
||||||
|
controller.notificationOutbox = { drainForShutdown: vi.fn(() => drain.promise) };
|
||||||
|
controller.manager = manager;
|
||||||
|
controller.megaWebFallback = { dispose: vi.fn() };
|
||||||
|
controller.realDebridWebFallbacks = new Map();
|
||||||
|
controller.pendingRealDebridWebAccountIds = new Map();
|
||||||
|
controller.allDebridWebFallback = { dispose: vi.fn() };
|
||||||
|
controller.bestDebridWebFallback = { dispose: vi.fn() };
|
||||||
|
controller.shutdownLogStorage = vi.fn();
|
||||||
|
controller.audit = vi.fn();
|
||||||
|
controller.settings = { historyRetentionMode: "never" };
|
||||||
|
|
||||||
|
const shutdown = controller.shutdown();
|
||||||
|
|
||||||
|
expect(shutdown).toBeInstanceOf(Promise);
|
||||||
|
expect(controller.notificationOutbox.drainForShutdown).toHaveBeenCalledWith(3000);
|
||||||
|
expect(manager.prepareForShutdown).not.toHaveBeenCalled();
|
||||||
|
drain.resolve();
|
||||||
|
await shutdown;
|
||||||
|
expect(manager.prepareForShutdown).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("prevents quit once, waits for shutdown, then allows exactly one loop-free quit", async () => {
|
||||||
|
const main = await import("../src/main/main");
|
||||||
|
const shutdown = deferred();
|
||||||
|
const cleanup = vi.fn();
|
||||||
|
const continueQuit = vi.fn();
|
||||||
|
const onError = vi.fn();
|
||||||
|
const handler = main.createBeforeQuitHandler({
|
||||||
|
cleanup,
|
||||||
|
shutdown: vi.fn(() => shutdown.promise),
|
||||||
|
continueQuit,
|
||||||
|
onError
|
||||||
|
});
|
||||||
|
const first = { preventDefault: vi.fn() };
|
||||||
|
const repeated = { preventDefault: vi.fn() };
|
||||||
|
const resumed = { preventDefault: vi.fn() };
|
||||||
|
|
||||||
|
handler(first);
|
||||||
|
handler(repeated);
|
||||||
|
expect(first.preventDefault).toHaveBeenCalledTimes(1);
|
||||||
|
expect(repeated.preventDefault).toHaveBeenCalledTimes(1);
|
||||||
|
expect(cleanup).toHaveBeenCalledTimes(1);
|
||||||
|
expect(continueQuit).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
shutdown.resolve();
|
||||||
|
await vi.waitFor(() => expect(continueQuit).toHaveBeenCalledTimes(1));
|
||||||
|
handler(resumed);
|
||||||
|
expect(resumed.preventDefault).not.toHaveBeenCalled();
|
||||||
|
expect(cleanup).toHaveBeenCalledTimes(1);
|
||||||
|
expect(onError).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -4,6 +4,7 @@ import os from "node:os";
|
|||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import { NotificationEvent, NotificationOutbox } from "../src/main/notification-outbox";
|
import { NotificationEvent, NotificationOutbox } from "../src/main/notification-outbox";
|
||||||
|
import { sendNotification } from "../src/main/notify";
|
||||||
|
|
||||||
const tempDirs: string[] = [];
|
const tempDirs: string[] = [];
|
||||||
|
|
||||||
@@ -67,11 +68,12 @@ describe("NotificationOutbox", () => {
|
|||||||
|
|
||||||
it("backs off a failed event without allowing later events to overtake it", async () => {
|
it("backs off a failed event without allowing later events to overtake it", async () => {
|
||||||
const filePath = createOutboxFile();
|
const filePath = createOutboxFile();
|
||||||
|
let now = 1000;
|
||||||
const outcomes = [false, true, true];
|
const outcomes = [false, true, true];
|
||||||
const sent: string[] = [];
|
const sent: string[] = [];
|
||||||
const outbox = new NotificationOutbox({
|
const outbox = new NotificationOutbox({
|
||||||
filePath,
|
filePath,
|
||||||
now: () => 1000,
|
now: () => now,
|
||||||
send: async (queuedEvent) => {
|
send: async (queuedEvent) => {
|
||||||
sent.push(queuedEvent.id);
|
sent.push(queuedEvent.id);
|
||||||
return outcomes.shift() ?? true;
|
return outcomes.shift() ?? true;
|
||||||
@@ -80,17 +82,115 @@ describe("NotificationOutbox", () => {
|
|||||||
|
|
||||||
await outbox.enqueue(event("first"));
|
await outbox.enqueue(event("first"));
|
||||||
await outbox.enqueue(event("second"));
|
await outbox.enqueue(event("second"));
|
||||||
await outbox.drain(1000);
|
await outbox.drain();
|
||||||
expect(sent).toEqual(["first"]);
|
expect(sent).toEqual(["first"]);
|
||||||
expect(persisted(filePath).events[0]).toMatchObject({ id: "first", attempts: 1, nextAttemptAt: 2000 });
|
expect(persisted(filePath).events[0]).toMatchObject({ id: "first", attempts: 1, nextAttemptAt: 2000 });
|
||||||
expect(outbox.getStatus()).toEqual({ queued: 2, lastSuccessAt: 0, lastFailureAt: 1000 });
|
expect(outbox.getStatus()).toEqual({ queued: 2, lastSuccessAt: 0, lastFailureAt: 1000 });
|
||||||
|
|
||||||
await outbox.drain(1999);
|
now = 1999;
|
||||||
|
await outbox.drain();
|
||||||
expect(sent).toEqual(["first"]);
|
expect(sent).toEqual(["first"]);
|
||||||
await outbox.drain(2000);
|
now = 2000;
|
||||||
|
await outbox.drain();
|
||||||
expect(sent).toEqual(["first", "first", "second"]);
|
expect(sent).toEqual(["first", "first", "second"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("uses the actual failure time for retry backoff", async () => {
|
||||||
|
const filePath = createOutboxFile();
|
||||||
|
let now = 1000;
|
||||||
|
const outbox = new NotificationOutbox({
|
||||||
|
filePath,
|
||||||
|
now: () => now,
|
||||||
|
send: async () => {
|
||||||
|
now = 4500;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await outbox.enqueue(event("late-failure"));
|
||||||
|
await outbox.drain();
|
||||||
|
|
||||||
|
expect(persisted(filePath).events[0]).toMatchObject({ attempts: 1, nextAttemptAt: 5500 });
|
||||||
|
expect(outbox.getStatus().lastFailureAt).toBe(4500);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rechecks expiration after each send before delivering the next event", async () => {
|
||||||
|
const filePath = createOutboxFile();
|
||||||
|
let now = 1000;
|
||||||
|
const sent: string[] = [];
|
||||||
|
const outbox = new NotificationOutbox({
|
||||||
|
filePath,
|
||||||
|
now: () => now,
|
||||||
|
send: async (queuedEvent) => {
|
||||||
|
sent.push(queuedEvent.id);
|
||||||
|
now = 2000;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await outbox.enqueue(event("first", { expiresAt: 5000 }));
|
||||||
|
await outbox.enqueue(event("expires-during-send", { expiresAt: 1500 }));
|
||||||
|
await outbox.drain();
|
||||||
|
|
||||||
|
expect(sent).toEqual(["first"]);
|
||||||
|
expect(outbox.getStatus()).toEqual({ queued: 0, lastSuccessAt: 2000, lastFailureAt: 0 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("caps exponential retry backoff at ten minutes after many attempts", async () => {
|
||||||
|
const filePath = createOutboxFile();
|
||||||
|
let now = 1000;
|
||||||
|
const outbox = new NotificationOutbox({
|
||||||
|
filePath,
|
||||||
|
now: () => now,
|
||||||
|
send: async () => {
|
||||||
|
now = 2000;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await outbox.enqueue(event("many-attempts", { attempts: 20 }));
|
||||||
|
await outbox.drain();
|
||||||
|
|
||||||
|
expect(persisted(filePath).events[0]).toMatchObject({ attempts: 21, nextAttemptAt: 602000 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("restores a future retry timer and reads changed URL and mention only when retrying", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.setSystemTime(1000);
|
||||||
|
const filePath = createOutboxFile();
|
||||||
|
const firstUrl = "https://discord.example.test/api/webhooks/first";
|
||||||
|
const secondUrl = "https://discord.example.test/api/webhooks/second";
|
||||||
|
let settings = { url: firstUrl, mention: "111111" };
|
||||||
|
const fetchFn = vi.fn()
|
||||||
|
.mockResolvedValueOnce(new Response("", { status: 404 }))
|
||||||
|
.mockResolvedValueOnce(new Response(null, { status: 204 }));
|
||||||
|
const sender = (queuedEvent: NotificationEvent): Promise<boolean> => sendNotification(settings.url, {
|
||||||
|
title: queuedEvent.payload.title,
|
||||||
|
message: queuedEvent.payload.description || "",
|
||||||
|
mention: settings.mention,
|
||||||
|
fields: queuedEvent.payload.fields,
|
||||||
|
timestamp: queuedEvent.createdAt
|
||||||
|
}, fetchFn, async () => {});
|
||||||
|
const firstProcess = new NotificationOutbox({ filePath, send: sender });
|
||||||
|
|
||||||
|
await firstProcess.enqueue(event("restart-retry"));
|
||||||
|
await firstProcess.drain();
|
||||||
|
expect(persisted(filePath).events[0]).toMatchObject({ attempts: 1, nextAttemptAt: 2000 });
|
||||||
|
|
||||||
|
settings = { url: secondUrl, mention: "222222" };
|
||||||
|
const restartedProcess = new NotificationOutbox({ filePath, send: sender, autoDrain: true });
|
||||||
|
await vi.advanceTimersByTimeAsync(999);
|
||||||
|
expect(fetchFn).toHaveBeenCalledTimes(1);
|
||||||
|
await vi.advanceTimersByTimeAsync(1);
|
||||||
|
expect(fetchFn).toHaveBeenCalledTimes(2);
|
||||||
|
expect(fetchFn.mock.calls[0][0]).toBe(firstUrl);
|
||||||
|
expect(fetchFn.mock.calls[1][0]).toBe(secondUrl);
|
||||||
|
expect(JSON.parse(String(fetchFn.mock.calls[0][1]?.body)).content).toBe("<@111111>");
|
||||||
|
expect(JSON.parse(String(fetchFn.mock.calls[1][1]?.body)).content).toBe("<@222222>");
|
||||||
|
await restartedProcess.drain();
|
||||||
|
expect(persisted(filePath).events).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
it("automatically drains new events and retries them at the persisted deadline", async () => {
|
it("automatically drains new events and retries them at the persisted deadline", async () => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
vi.setSystemTime(1000);
|
vi.setSystemTime(1000);
|
||||||
|
|||||||
Reference in New Issue
Block a user