diff --git a/packages/timer/package.json b/packages/timer/package.json index b8ac8d4b..9e5bca19 100644 --- a/packages/timer/package.json +++ b/packages/timer/package.json @@ -37,8 +37,7 @@ "dependencies": { "@node-cli/logger": "workspace:*", "@node-cli/parser": "workspace:*", - "moment": "2.30.1", - "node-notifier": "10.0.1" + "moment": "2.30.1" }, "publishConfig": { "access": "public" diff --git a/packages/timer/src/__tests__/notify.test.ts b/packages/timer/src/__tests__/notify.test.ts new file mode 100644 index 00000000..21f89cfb --- /dev/null +++ b/packages/timer/src/__tests__/notify.test.ts @@ -0,0 +1,210 @@ +import { execFile } from "node:child_process"; +import { vi } from "vitest"; +import { buildNotifyCommand, type Notification, notify } from "../notify.js"; + +/** + * Node's own execFile defines promisify.custom, and that implementation calls + * execFile outside a promise executor, which is what lets a spawn failure throw + * synchronously. A bare mock has no such property, so promisify would fall back + * to wrapping the call in `new Promise`, quietly turning every synchronous + * throw into a rejection and hiding the very failure mode these tests exist to + * pin down. + */ +vi.mock("node:child_process", () => { + const execFile = vi.fn(); + /** + * vi.mock factories are hoisted, so the symbol is resolved inline rather than + * through a top-level `promisify` import. + */ + execFile[Symbol.for("nodejs.util.promisify.custom")] = ( + ...args: unknown[] + ) => { + let resolve: (value: unknown) => void; + let reject: (reason: Error) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + /** + * deliberately outside the executor above, mirroring Node, so that a + * synchronous throw stays synchronous instead of becoming a rejection. + */ + execFile(...args, (error: Error | null) => + error ? reject(error) : resolve({ stderr: "", stdout: "" }), + ); + return promise; + }; + return { execFile }; +}); + +const mockedExecFile = vi.mocked(execFile); + +type ExecFileCallback = (error: NodeJS.ErrnoException | null) => void; + +/** + * execFile is called with an optional options argument, so the callback is + * whatever lands last rather than a fixed position. + */ +const lastArgument = (args: unknown[]): ExecFileCallback => + args.at(-1) as ExecFileCallback; + +/** + * Mimics a notifier that spawns successfully and exits cleanly. + */ +const spawnSucceeds = () => { + mockedExecFile.mockImplementation(((...args: unknown[]) => { + lastArgument(args)(null); + }) as unknown as typeof execFile); +}; + +/** + * Mimics `child_process.spawn` failing synchronously, which is how a notifier + * binary that cannot run on the host CPU surfaces: EBADARCH (errno -86, "Bad + * CPU type in executable") is thrown, not passed to the callback. + */ +const spawnThrowsBadArch = () => { + mockedExecFile.mockImplementation((() => { + const error: NodeJS.ErrnoException = new Error( + "spawn Unknown system error -86", + ); + error.errno = -86; + error.code = "Unknown system error -86"; + error.syscall = "spawn"; + throw error; + }) as unknown as typeof execFile); +}; + +/** + * Mimics a notifier that is simply not installed on the host. + */ +const spawnFailsNotFound = () => { + mockedExecFile.mockImplementation(((...args: unknown[]) => { + const error: NodeJS.ErrnoException = new Error("spawn ENOENT"); + error.code = "ENOENT"; + lastArgument(args)(error); + }) as unknown as typeof execFile); +}; + +const notification = { + message: "Time's up!", + sound: "Funk", + title: "Timer Notification", +}; + +describe("when building the platform-native notification command", () => { + it("should use osascript on macOS", async () => { + expect(buildNotifyCommand("darwin", notification)).toStrictEqual({ + command: "osascript", + args: [ + "-e", + 'display notification "Time\'s up!" with title "Timer Notification" sound name "Funk"', + ], + }); + }); + + it("should omit the sound on macOS when none is requested", async () => { + expect( + buildNotifyCommand("darwin", { message: "a", title: "b" }), + ).toStrictEqual({ + command: "osascript", + args: ["-e", 'display notification "a" with title "b"'], + }); + }); + + it("should escape quotes and backslashes in the AppleScript literal", async () => { + const { args } = buildNotifyCommand("darwin", { + message: 'say "hi" \\ bye', + title: 'a "quoted" title', + }); + expect(args[1]).toBe( + 'display notification "say \\"hi\\" \\\\ bye" with title "a \\"quoted\\" title"', + ); + }); + + it("should use notify-send on Linux", async () => { + expect(buildNotifyCommand("linux", notification)).toStrictEqual({ + command: "notify-send", + args: ["Timer Notification", "Time's up!"], + }); + }); + + it("should use a PowerShell toast on Windows", async () => { + const result = buildNotifyCommand("win32", notification); + expect(result.command).toBe("powershell"); + expect(result.args[0]).toBe("-NoProfile"); + expect(result.args[2]).toContain("ToastNotificationManager"); + expect(result.args[2]).toContain("Timer Notification"); + }); + + it("should double single quotes in the PowerShell literal", async () => { + const { args } = buildNotifyCommand("win32", { + message: "Time's up!", + title: "it's fine", + }); + expect(args[2]).toContain("'Time''s up!'"); + expect(args[2]).toContain("'it''s fine'"); + }); + + it("should return null on an unsupported platform", async () => { + expect(buildNotifyCommand("aix", notification)).toBeNull(); + }); +}); + +describe("when displaying a notification", () => { + beforeEach(() => { + mockedExecFile.mockReset(); + }); + + it("should resolve to true when the notifier runs", async () => { + spawnSucceeds(); + await expect(notify(notification, "darwin")).resolves.toBe(true); + expect(mockedExecFile).toHaveBeenCalledTimes(1); + }); + + /** + * Regression: node-notifier spawned a vendored x86_64-only terminal-notifier + * binary, so on an Apple Silicon host without Rosetta the spawn threw EBADARCH + * straight through the timer callback and crashed the process. A failed + * notification must degrade, not throw. + */ + it("should resolve to false when the notifier cannot run on this CPU", async () => { + spawnThrowsBadArch(); + await expect(notify(notification, "darwin")).resolves.toBe(false); + }); + + it("should resolve to false when the notifier is not installed", async () => { + spawnFailsNotFound(); + await expect(notify(notification, "linux")).resolves.toBe(false); + }); + + /** + * A notifier that spawns but never exits would otherwise keep the CLI alive + * indefinitely, since the pending child is the last thing holding the event + * loop open once the spinner has stopped. + */ + it("should bound the spawn so a hung notifier cannot keep the CLI alive", async () => { + spawnSucceeds(); + await notify(notification, "darwin"); + expect(mockedExecFile).toHaveBeenCalledWith( + "osascript", + expect.any(Array), + expect.objectContaining({ killSignal: "SIGKILL", timeout: 5000 }), + expect.any(Function), + ); + }); + + it("should resolve to false when a malformed notification is passed", async () => { + await expect( + notify( + { message: undefined, title: "t" } as unknown as Notification, + "darwin", + ), + ).resolves.toBe(false); + expect(mockedExecFile).not.toHaveBeenCalled(); + }); + + it("should resolve to false without spawning on an unsupported platform", async () => { + await expect(notify(notification, "aix")).resolves.toBe(false); + expect(mockedExecFile).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/timer/src/notify.ts b/packages/timer/src/notify.ts new file mode 100644 index 00000000..53cc22eb --- /dev/null +++ b/packages/timer/src/notify.ts @@ -0,0 +1,121 @@ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); + +/** + * A notifier that spawns but never exits would keep the CLI alive forever, so + * it is killed outright rather than merely asked to stop: a wedged child can + * ignore SIGTERM. + */ +const NOTIFIER_TIMEOUT_MS = 5000; + +export type Notification = { + message: string; + sound?: string; + title: string; +}; + +export type NotifyCommand = { + args: string[]; + command: string; +}; + +/** + * Escapes a value so it can be embedded in an AppleScript double-quoted string + * literal, where only backslashes and double quotes are special. + */ +const escapeAppleScript = (value: string): string => + value.replaceAll("\\", "\\\\").replaceAll('"', '\\"'); + +/** + * Escapes a value so it can be embedded in a PowerShell single-quoted string + * literal, where a single quote is escaped by doubling it. + */ +const escapePowerShell = (value: string): string => value.replaceAll("'", "''"); + +const macCommand = ({ message, sound, title }: Notification): NotifyCommand => { + const script = [ + `display notification "${escapeAppleScript(message)}"`, + `with title "${escapeAppleScript(title)}"`, + sound ? `sound name "${escapeAppleScript(sound)}"` : "", + ] + .filter(Boolean) + .join(" "); + + return { args: ["-e", script], command: "osascript" }; +}; + +const linuxCommand = ({ message, title }: Notification): NotifyCommand => ({ + args: [title, message], + command: "notify-send", +}); + +const windowsCommand = ({ message, title }: Notification): NotifyCommand => { + const script = [ + "[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null", + "$xml = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastText02)", + `$xml.GetElementsByTagName('text')[0].AppendChild($xml.CreateTextNode('${escapePowerShell(title)}')) > $null`, + `$xml.GetElementsByTagName('text')[1].AppendChild($xml.CreateTextNode('${escapePowerShell(message)}')) > $null`, + "[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier('@node-cli/timer').Show([Windows.UI.Notifications.ToastNotification]::new($xml))", + ].join("; "); + + return { args: ["-NoProfile", "-Command", script], command: "powershell" }; +}; + +/** + * Builds the platform-native command used to display a desktop notification. + * Returns null on platforms we do not know how to notify on. + * + * Every supported platform ships its notifier as part of the OS, so nothing is + * vendored and nothing has to match the host CPU architecture. + * + */ +export const buildNotifyCommand = ( + platform: string, + notification: Notification, +): NotifyCommand | null => { + switch (platform) { + case "darwin": { + return macCommand(notification); + } + case "linux": { + return linuxCommand(notification); + } + case "win32": { + return windowsCommand(notification); + } + default: { + return null; + } + } +}; + +/** + * Displays a desktop notification, resolving to true when it was shown. + * + * A notification is a courtesy, never a reason to fail: an unsupported + * platform, a missing notifier, or a notifier that refuses to run all resolve + * to false instead of throwing. + * + */ +export const notify = async ( + notification: Notification, + platform: string = process.platform, +): Promise => { + try { + const notifyCommand = buildNotifyCommand(platform, notification); + + if (notifyCommand === null) { + return false; + } + + await execFileAsync(notifyCommand.command, notifyCommand.args, { + killSignal: "SIGKILL", + timeout: NOTIFIER_TIMEOUT_MS, + }); + return true; + } catch { + return false; + } +}; diff --git a/packages/timer/src/utilities.ts b/packages/timer/src/utilities.ts index ce1e6be8..486d305e 100644 --- a/packages/timer/src/utilities.ts +++ b/packages/timer/src/utilities.ts @@ -1,7 +1,7 @@ import { Logger, Spinner } from "@node-cli/logger"; import moment, { Duration } from "moment"; -import notifier from "node-notifier"; +import { notify } from "./notify.js"; import { Configuration } from "./parse.js"; const logger = new Logger(); @@ -48,7 +48,7 @@ export const extractDuration = (config: Configuration): Duration => { } }; -/* v8 ignore next 60 */ +/* v8 ignore start */ export class Timer { config: Configuration; startTime: number; @@ -80,19 +80,31 @@ export class Timer { const timer = setInterval(this.printRemainingTime, 1000); // kill the spinner when the timer is done. - setTimeout(() => { + setTimeout(async () => { const message = "Time's up!"; - if (this.config.flags.notification) { - notifier.notify({ - message, - sound: "Funk", - title: "Timer Notification", - wait: true, - }); - } clearInterval(timer); this.printRemainingTime(message); spinner.stop(); + + /** + * notifying last, and never fatally: a desktop notification that cannot be + * displayed must not take the timer down with it. + */ + if (this.config.flags.notification) { + try { + await notify({ + message, + sound: "Funk", + title: "Timer Notification", + }); + } catch { + /** + * notify is already total, so this only ever catches a future regression + * in it. Rejecting here would be fatal: an async setTimeout callback has + * nothing to attach a handler to. + */ + } + } }, this.timerDurationMilliSeconds); } }; @@ -109,3 +121,4 @@ export class Timer { } }; } +/* v8 ignore stop */ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3ca45b1f..1feccf4a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -354,9 +354,6 @@ importers: moment: specifier: 2.30.1 version: 2.30.1 - node-notifier: - specifier: 10.0.1 - version: 10.0.1 devDependencies: '@vitest/coverage-v8': specifier: 4.1.10 @@ -2688,9 +2685,6 @@ packages: graceful-fs@4.2.11: resolution: {integrity: sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==} - growly@1.3.0: - resolution: {integrity: sha512-+xGQY0YyAWCnqy7Cd++hc2JqMYzlm0dG30Jd0beaA64sROr8C4nt8Yc9V5Ro3avlSUDTN0ulqP/VBKi1/lLygw==} - handlebars@4.7.9: resolution: {integrity: sha512-4E71E0rpOaQuJR2A3xDZ+GM1HyWYv1clR58tC8emQNeQe3RH7MAzSbat+V0wG78LQBo6m6bzSG/L4pBuCsgnUQ==} engines: {node: '>=0.4.7'} @@ -2868,11 +2862,6 @@ packages: is-arrayish@0.2.1: resolution: {integrity: sha512-zz06S8t0ozoDXMG+ube26zeCTNXcKIPJZJi8hBrF4idCLms4CG9QtK7qBl1boi5ODzFpjswb5JPmHCbMpjaYzg==} - is-docker@2.2.1: - resolution: {integrity: sha512-F+i2BKsFrH66iaUFc0woD8sLy8getkwTwtOBjvs56Cx4CgJDeKQeqfz8wAYiSb8JOprWhHH5p77PbmYCvvUuXQ==} - engines: {node: '>=8'} - hasBin: true - is-docker@3.0.0: resolution: {integrity: sha512-eljcgEDlEns/7AXFosB5K/2nCM4P7FQPkGc/DWLy5rmFEWvZayGrik1d9/QIY5nJ4f9YsVvBkA6kJpHn9rISdQ==} engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} @@ -2937,10 +2926,6 @@ packages: resolution: {integrity: sha512-mE00Gnza5EEB3Ds0HfMyllZzbBrmLOX3vfWoj9A9PEnTfratQ/BcaJOuMhnkhjXvb2+FkY3VuHqtAGpTPmglFQ==} engines: {node: '>=18'} - is-wsl@2.2.0: - resolution: {integrity: sha512-fKzAra0rGJUUBwGBgNkHZuToZcn+TtXHpeCgmkMJMMYx1sQDYaCSyjJBSCa2nH1DGm7s3n1oBnohoVTBaN7Lww==} - engines: {node: '>=8'} - is-wsl@3.1.0: resolution: {integrity: sha512-UcVfVfaK4Sc4m7X3dUSoHoozQGBEFeDC+zVo06t98xe8CzHSZZBekNXH+tu0NalHolcJ/QAGqS46Hef7QXBIMw==} engines: {node: '>=16'} @@ -3375,9 +3360,6 @@ packages: engines: {node: ^20.17.0 || >=22.9.0} hasBin: true - node-notifier@10.0.1: - resolution: {integrity: sha512-YX7TSyDukOZ0g+gmzjB6abKu+hTGvO8+8+gIFDsRCU2t8fLV/P2unmt+LGFaIa4y64aX98Qksa97rgz4vMNeLQ==} - nopt@8.1.0: resolution: {integrity: sha512-ieGu42u/Qsa4TFktmaKEwM6MQH0pOWnaB3htzh0JRtx84+Mebc0cbZYN5bC+6WTZ4+77xrL9Pn5m7CV6VIkV7A==} engines: {node: ^18.17.0 || >=20.5.0} @@ -3955,9 +3937,6 @@ packages: resolution: {integrity: sha512-Iov+JwFv/2HcTpcwNMKd8+IWNb8tboQJNQTkAY/LLVK7gGH9jy+LGkVqPxfekHl+yMmiqXszdGWXgkfml7hjqA==} engines: {node: '>= 0.4'} - shellwords@0.1.1: - resolution: {integrity: sha512-vFwSUfQvqybiICwZY5+DAWIPLKsWO31Q91JSKl3UYv+K5c2QRPzn0qzec6QPu1Qc9eHYItiP3NdJqNVqetYAww==} - siginfo@2.0.0: resolution: {integrity: sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g==} @@ -4298,11 +4277,6 @@ packages: resolution: {integrity: sha512-6ZxzVpzDXDa3bJWaHilVayA+BH/1zmxCJoVgvmqJnid/gPoKHxUrS/aC/T6LGQtNHT+XHG9fXPJB4d+IrU30Ew==} hasBin: true - uuid@8.3.2: - resolution: {integrity: sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==} - deprecated: uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028). - hasBin: true - validate-npm-package-license@3.0.4: resolution: {integrity: sha512-DpKm2Ui/xN7/HQKCtpZxoRWBhZ9Z0kqtygG8XCgNQ8ZlDnxuQmWhj566j8fN4Cu3/JmbhsDo7fcAJq4s9h27Ew==} @@ -6862,8 +6836,6 @@ snapshots: graceful-fs@4.2.11: {} - growly@1.3.0: {} - handlebars@4.7.9: dependencies: minimist: 1.2.8 @@ -7048,8 +7020,6 @@ snapshots: is-arrayish@0.2.1: {} - is-docker@2.2.1: {} - is-docker@3.0.0: {} is-fullwidth-code-point@3.0.0: {} @@ -7087,10 +7057,6 @@ snapshots: is-unicode-supported@2.1.0: {} - is-wsl@2.2.0: - dependencies: - is-docker: 2.2.1 - is-wsl@3.1.0: dependencies: is-inside-container: 1.0.0 @@ -7574,15 +7540,6 @@ snapshots: undici: 6.27.0 which: 6.0.1 - node-notifier@10.0.1: - dependencies: - growly: 1.3.0 - is-wsl: 2.2.0 - semver: 7.8.5 - shellwords: 0.1.1 - uuid: 8.3.2 - which: 2.0.2 - nopt@8.1.0: dependencies: abbrev: 3.0.1 @@ -8311,8 +8268,6 @@ snapshots: shell-quote@1.9.0: {} - shellwords@0.1.1: {} - siginfo@2.0.0: {} signal-exit@3.0.7: {} @@ -8646,8 +8601,6 @@ snapshots: uuid@14.0.1: {} - uuid@8.3.2: {} - validate-npm-package-license@3.0.4: dependencies: spdx-correct: 3.2.0