fix(electron-backend): lint the whole project, not one file

`nx lint electron-backend` ran `eslint apps/electron-backend/**/*.ts`
unquoted. Nx executes commands through /bin/sh, which has no `globstar`, so
`**` collapsed to `*` and the shell expanded the pattern itself — matching a
single file out of 247. Quoting hands the pattern to eslint, which expands it
recursively. Two sibling patterns were already quoted, so the breakage had
been worked around per-file rather than fixed; they are now redundant
(verified: the quoted glob alone resolves to the identical 247-file set,
including all 24 frame-copy-runtime files) and collapse into one.

That exposed five real errors the broken glob had been hiding:

- `no-unsafe-finally` x2 — `closeSession` and `disposeSession` both did
  `try { ... } finally { return ...; }`. A `return` inside `finally`
  discards an in-flight exception, so a failing close/dispose was
  indistinguishable from a clean one. Both intentionally always complete
  teardown (`shutdown()` disposes every session in turn and must not stop at
  the first failure), so the fix keeps that guarantee via an explicit `catch`
  and logs the reason instead of dropping it.
- `prefer-const` x2 — a write-once `timeoutId` and module-level `Database`.
  The other `timeoutId` in the same file is genuinely reassigned and stays
  `let`.
- `no-control-regex` — the filename sanitizer strips control characters on
  purpose; disabled inline with that rationale.

The three `max-lines` violations this would also have surfaced are already
gone: #1288 split those specs.

Regression coverage for both teardown paths, each verified to fail against
the old code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-07-27 22:34:39 +02:00
1 parent 19b592badb
commit 1609e0ff11
8 files changed
+124 -41

No files matched your search

@@ -0,0 +1,9 @@
---
type: internal
area: electron-backend
---
Lint now covers the whole Electron backend instead of a single file, and two
teardown paths that silently discarded their failure reason — closing an
external player session and disposing an embedded mpv session — now report it.
Both still finish the teardown exactly as before.
+1 -1
View File
@@ -241,7 +241,7 @@
"{workspaceRoot}/tools/embedded-mpv/runtime-probe-contract.cjs",
"{workspaceRoot}/tools/embedded-mpv/runtime-probe-contract.d.cts"
],
"command": "eslint apps/electron-backend/**/*.ts \"apps/electron-backend/src/app/services/embedded-mpv-frame-copy-runtime.ts\" \"apps/electron-backend/src/app/services/embedded-mpv-frame-copy-runtime/**/*.ts\""
"command": "eslint \"apps/electron-backend/**/*.ts\""
},
"test": {
"executor": "@nx/jest:jest",
@@ -422,7 +422,6 @@ export class EpgWorkerService {
}
let settled = false;
let timeoutId: ReturnType<typeof setTimeout>;
const settle = (fn: () => void) => {
if (settled) return;
settled = true;
@@ -430,7 +429,7 @@ export class EpgWorkerService {
fn();
};
timeoutId = setTimeout(() => {
const timeoutId = setTimeout(() => {
const errorMessage = `${options.timeoutLabel} timed out after ${
this.fetchTimeoutMs / 1000
}s`;
@@ -47,6 +47,32 @@ describe('ExternalPlayerSessionRegistry', () => {
expect(registry.getActiveSessionId()).toBeNull();
});
it('still closes the session when the player fails to close', async () => {
const warn = jest.spyOn(console, 'warn').mockImplementation();
const failure = new Error('player already exited');
const close = jest.fn().mockRejectedValue(failure);
const session = registry.beginSession({
player: 'mpv',
title: 'Example',
streamUrl: 'https://example.com/video.m3u8',
});
registry.attachCloser(session.id, close);
const closed = await registry.closeSession(session.id);
expect(closed?.status).toBe('closed');
expect(closed?.canClose).toBe(false);
expect(registry.getActiveSessionId()).toBeNull();
// The reason must survive: the old `return` inside `finally` dropped it.
expect(warn).toHaveBeenCalledWith(
`Failed to close external player session ${session.id}:`,
failure
);
warn.mockRestore();
});
it('marks runtime failures as errors without clearing the active id', () => {
const session = registry.beginSession({
player: 'mpv',
@@ -128,10 +128,18 @@ export class ExternalPlayerSessionRegistry {
return null;
}
// The session must end up closed even if the player is already gone,
// but a `return` inside `finally` would also swallow the reason — so
// catch it explicitly and surface it.
try {
await runtime.close?.();
} finally {
return this.markClosed(id);
} catch (error) {
console.warn(
`Failed to close external player session ${id}:`,
error
);
}
return this.markClosed(id);
}
}
@@ -493,7 +493,11 @@ describe('EmbeddedMpvNativeService power blocker', () => {
addon.createSession.mockReturnValueOnce('s-zoom');
addon.getSessionSnapshot.mockReturnValue(snapshot('loading'));
service.createSession({ x: 0, y: 0, width: 100, height: 100 }, '', 1);
service.createSession(
{ x: 0, y: 0, width: 100, height: 100 },
'',
1
);
expect(addon.createSession).toHaveBeenCalledWith(
expect.any(Buffer),
@@ -1112,4 +1116,33 @@ describe('EmbeddedMpvNativeService power blocker', () => {
})
);
});
it('completes teardown when the native dispose throws', () => {
const warn = jest.spyOn(console, 'warn').mockImplementation();
const failure = new Error('native session already gone');
startSession('s-throws', snapshot('playing'));
addon.disposeSession.mockImplementationOnce(() => {
throw failure;
});
// A `return` inside `finally` used to swallow this, so `shutdown()`
// could not tell a failed dispose from a clean one.
const disposed = service.disposeSession('s-throws');
expect(disposed?.id).toBe('s-throws');
expect(disposed?.status).toBe('closed');
expect(mainWindowSendMock).toHaveBeenLastCalledWith(
'EMBEDDED_MPV_SESSION_UPDATE',
expect.objectContaining({ id: 's-throws', status: 'closed' })
);
expect(warn).toHaveBeenCalledWith(
'Failed to dispose embedded mpv session s-throws:',
failure
);
// The session is gone, so a second dispose finds nothing.
expect(service.disposeSession('s-throws')).toBeNull();
warn.mockRestore();
});
});
@@ -663,32 +663,41 @@ export class EmbeddedMpvNativeService {
lastRecording = undefined;
}
addon.disposeSession(sessionId);
} finally {
this.sessions.delete(sessionId);
this.pollFailuresLogged.delete(sessionId);
const payload: EmbeddedMpvSession = {
id: session.id,
title: session.title,
streamUrl: session.streamUrl,
status: 'closed',
positionSeconds: 0,
durationSeconds: null,
volume: 1,
audioTracks: [],
selectedAudioTrackId: null,
subtitleTracks: [],
selectedSubtitleTrackId: null,
playbackSpeed: 1,
aspectOverride: 'no',
recording: this.createClosedRecordingState(lastRecording),
startedAt: session.startedAt,
updatedAt: new Date().toISOString(),
};
this.sendSessionUpdate(payload);
this.stopPollingIfIdle();
this.updatePowerBlocker();
return payload;
} catch (error) {
// Teardown below must run even when the native session is already
// gone — `shutdown()` disposes every session in turn and must not
// stop at the first failure. A `return` inside `finally` achieved
// that but also swallowed the reason, so log it instead.
console.warn(
`Failed to dispose embedded mpv session ${sessionId}:`,
error
);
}
this.sessions.delete(sessionId);
this.pollFailuresLogged.delete(sessionId);
const payload: EmbeddedMpvSession = {
id: session.id,
title: session.title,
streamUrl: session.streamUrl,
status: 'closed',
positionSeconds: 0,
durationSeconds: null,
volume: 1,
audioTracks: [],
selectedAudioTrackId: null,
subtitleTracks: [],
selectedSubtitleTrackId: null,
playbackSpeed: 1,
aspectOverride: 'no',
recording: this.createClosedRecordingState(lastRecording),
startedAt: session.startedAt,
updatedAt: new Date().toISOString(),
};
this.sendSessionUpdate(payload);
this.stopPollingIfIdle();
this.updatePowerBlocker();
return payload;
}
shutdown(): void {
@@ -979,6 +988,9 @@ export class EmbeddedMpvNativeService {
private sanitizeRecordingFileName(title: string): string {
const normalized = title
// Stripping control characters is precisely what a filename
// sanitizer must do, so the rule does not apply here.
// eslint-disable-next-line no-control-regex
.replace(/[<>:"/\\|?*\u0000-\u001f]/g, '_')
.replace(/\s+/g, ' ')
.trim();
@@ -15,17 +15,14 @@ import {
trace,
} from '../services/debug-trace';
let Database: typeof BetterSqlite3;
let drizzleFactory:
| (typeof import('drizzle-orm/better-sqlite3'))['drizzle']
| undefined;
(typeof import('drizzle-orm/better-sqlite3'))['drizzle'] | undefined;
const nativeModuleSearchPaths = [
...getWorkerDataNativeModuleSearchPaths(workerData),
...getNativeModuleSearchPaths({
resourcesPath: (
process as NodeJS.Process & { resourcesPath?: string }
).resourcesPath,
resourcesPath: (process as NodeJS.Process & { resourcesPath?: string })
.resourcesPath,
}),
];
@@ -50,14 +47,13 @@ function getDrizzleFactory(): (typeof import('drizzle-orm/better-sqlite3'))['dri
// Require drizzle only after native lookup paths have been registered.
// Its better-sqlite3 driver resolves the native package at module load time.
// eslint-disable-next-line @typescript-eslint/no-require-imports
drizzleFactory = require('drizzle-orm/better-sqlite3').drizzle as (
typeof import('drizzle-orm/better-sqlite3')
)['drizzle'];
drizzleFactory = require('drizzle-orm/better-sqlite3')
.drizzle as (typeof import('drizzle-orm/better-sqlite3'))['drizzle'];
return drizzleFactory;
}
Database = loadBetterSqlite3();
const Database: typeof BetterSqlite3 = loadBetterSqlite3();
let db: AppDatabase | null = null;
let sqlite: BetterSqlite3.Database | null = null;