Skip to content

fix(timer): stop crashing when the timer expires on Apple Silicon - #634

Merged
aversini merged 2 commits into
mainfrom
fix_timer_stop_crashing_when_the_timer_expires_on_apple_silicon
Aug 4, 2026
Merged

aversini merged 2 commits into
mainfrom
fix_timer_stop_crashing_when_the_timer_expires_on_apple_silicon

Conversation

@aversini

@aversini aversini commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What was broken

Every timer crashed the moment it expired:

❯ t 2s
┌ Timer Configuration ┐
│     0h 0m 2s        │
└─────────────────────┘

Error: spawn Unknown system error -86
    at ChildProcess.spawn (node:internal/child_process:441:11)
    at module.exports.fileCommandJson (node-notifier/lib/utils.js:88:13)
    at NotificationCenter.notifyRaw (node-notifier/notifiers/notificationcenter.js:81:11)
    at Timeout._onTimeout (@node-cli/timer/dist/utilities.js:65:30)

Notifications default to on, so this made the tool unusable on affected machines.

Root cause

On Darwin, errno 86 is EBADARCH — "Bad CPU type in executable".

node-notifier displays macOS notifications by spawning a vendored binary at vendor/mac.noindex/terminal-notifier.app. That binary is x86_64-only:

$ lipo -archs .../terminal-notifier
x86_64

On an Apple Silicon host with Rosetta 2 not installed, it cannot run at all:

$ .../terminal-notifier -help
bad CPU type in executable

execFile throws that EBADARCH synchronously, straight up through the unguarded notifier.notify() call inside the setTimeout expiry callback, where nothing caught it. A cosmetic failure became an uncaught exception that killed the process.

The fix

Notifications now use the notifier each OS already ships, so nothing is vendored and nothing is coupled to a CPU architecture:

Platform Notifier
macOS osascript (arm64e, always present)
Linux notify-send
Windows PowerShell WinRT toast

notify() resolves to false on any failure — unsupported platform, missing notifier, notifier that refuses to run — instead of throwing. A notification can no longer take the timer down.

node-notifier is dropped entirely: it is unmaintained (10.0.1 is its final release), and this removes ~4MB of unrunnable vendored binaries plus 47 lockfile lines.

Verification

End-to-end, the exact failing command:

$ node dist/timer.js 2s
✔ Time's up!
EXIT_CODE=0

The notification is confirmed delivered rather than silently swallowed (notify delivered: true).

Full suite green across all 12 projects; timer at 13/13.

The regression tests are meaningful — with the guard removed, both failure tests fail (promise rejected ... instead of resolving); restored, they pass. notify.test.ts reproduces the exact EBADARCH synchronous throw (errno -86, syscall spawn), plus ENOENT, unsupported-platform, and command-construction/escaping cases.

Consumer impact

  • node-notifier is no longer a dependency.
  • wait: true is gone. The process no longer stays alive until the notification is dismissed; it exits once the notification is delivered.

Not included

Left for separate PRs, as they are unrelated to this crash:

  • 3 pre-existing lint warnings in utilities.test.ts (unused path/Duration/vi imports), identical before and after this change.
  • @types/node-notifier still arrives transitively via @versini/dev-dependencies-common — types-only and harmless, but removing it means touching another repo.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PmVBQszqooNXgLX5x6VTth

aversini and others added 2 commits August 4, 2026 15:53
The timer crashed with `spawn Unknown system error -86` every time it
expired on an Apple Silicon Mac without Rosetta 2 installed. errno 86 is
EBADARCH, "Bad CPU type in executable". Since notifications default to on,
this made the tool unusable on those machines.

node-notifier displays macOS notifications by spawning a vendored
terminal-notifier binary that is x86_64-only, so it cannot run natively on
arm64. The spawn failure was thrown synchronously through the unguarded
notify() call inside the expiry callback, where nothing caught it, turning
a cosmetic failure into an uncaught exception that killed the process.

Notifications now use the notifier each OS already ships: osascript on
macOS, notify-send on Linux, and a PowerShell toast on Windows. Nothing is
vendored, so nothing is coupled to a CPU architecture. A notification that
cannot be displayed resolves to false instead of throwing, so a failed
notification can never take the timer down again.

Consumer impact:

- node-notifier is no longer a dependency.
- `wait: true` is gone, so the process no longer stays alive until the
  notification is dismissed; it exits once the notification is delivered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PmVBQszqooNXgLX5x6VTth
Follow-up from pre-landing review. The notification path could still take
the CLI down, just by hanging instead of crashing.

- notify.ts now passes { timeout: 5000, killSignal: SIGKILL } to execFile.
  A notifier that spawns but never exits (notify-send on a dead D-Bus
  session, a stalled osascript) was the last thing holding the event loop
  open once the spinner stopped, so the CLI hung forever after printing
  "Time's up!". SIGKILL rather than the default SIGTERM, since a wedged
  child can ignore SIGTERM and hang anyway.

- buildNotifyCommand() moved inside the try block. It ran before it, so a
  malformed notification made notify() reject with a TypeError instead of
  resolving false, contradicting the "never throws" contract documented on
  the function.

- The expiry callback in utilities.ts wraps its await in try/catch. An
  async setTimeout callback has nothing to attach a rejection handler to,
  so an unhandled rejection there is fatal, which is the original crash
  shape one frame higher up.

- The EBADARCH regression test was not testing EBADARCH. Node's real
  execFile defines promisify.custom and calls execFile outside a promise
  executor, which is what lets a spawn failure throw synchronously; a bare
  vi.fn() mock has no such property, so promisify wrapped the call in
  `new Promise` and quietly turned the synchronous throw into a rejection.
  The mock now mirrors Node's shape, and the test fails if the await is
  ever moved out of the try block.

- Added guards for the spawn timeout and for malformed input, and switched
  the Timer class from `/* v8 ignore next 60 */` to the start/stop form
  used elsewhere in the package, since the line count went stale when the
  class grew.

15 tests pass, coverage 100%, full suite green across 12 projects.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PmVBQszqooNXgLX5x6VTth

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

@aversini
aversini merged commit d9e5c78 into main Aug 4, 2026
6 checks passed
@aversini
aversini deleted the fix_timer_stop_crashing_when_the_timer_expires_on_apple_silicon branch August 4, 2026 21:20
@aversini aversini mentioned this pull request Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants