ReviewOS

stacks/uplink

fix(app): an uncaught error leaves no trace, and the log grows without a cap

#34
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Part of #6.

~/Library/Logs/Uplink.log is the only record of anything the downloadable app does. There is no crash reporting, no diagnostics bundle, and no second copy of stderr anywhere, so when someone's Uplink stops answering texts that file is the whole support story. The one failure that most needs explaining is the one the file cannot describe, and the file itself is unbounded on a process that polls every two seconds.

What is there today

  • The log is set up once at startup, behind a TTY check: app/Desktop/launcher.ts#L24 only redirects when process.stdout.isTTY is false, the loop at app/Desktop/launcher.ts#L26 covers exactly log, warn and error, and each of those becomes an appendFileSync to LOG_PATH (app/Desktop/launcher.ts#L27).
  • LOG_PATH is ~/Library/Logs/Uplink.log (app/Uplink/settings.ts#L17). grep -rn LOG_PATH app/ matches only that definition and the three lines above, so nothing else in the app reads, trims or rotates the file.
  • The only handlers the launcher registers are the three shutdown signals (app/Desktop/launcher.ts#L86 and app/Desktop/launcher.ts#L87). A grep for uncaughtException and unhandledRejection across app/ returns no matches, so an uncaught throw or a rejected promise in the agent prints to a stderr nobody is reading and the process disappears with the log's last line being whatever succeeded before it.
  • At login there is no fallback either. The LaunchAgent the app writes for open at login sets RunAtLoad and KeepAlive but no StandardOutPath or StandardErrorPath (app/Uplink/desktop-agent.ts#L113 through app/Uplink/desktop-agent.ts#L123). The from-source launchd service does set both to its log (app/Uplink/service.ts#L78, app/Uplink/service.ts#L79), which is why this gap belongs to the downloadable app specifically.
  • A crash also skips cleanup. The single instance lock is released only inside agent.stop() (app/Uplink/desktop-agent.ts#L359), which shutdown calls at app/Desktop/launcher.ts#L81. Nothing calls it on an uncaught error, leaving a stale lock for the next start to reap (app/Uplink/desktop-agent.ts#L54 through app/Uplink/desktop-agent.ts#L64).
  • Nothing caps the file. The watcher reschedules itself on config.pollMs (app/Uplink/uplink.ts#L162), which defaults to 2000 (app/Uplink/config.ts#L67), and the downloadable app takes that default through loadConfig(settingsEnv(settings)) (app/Uplink/desktop-agent.ts#L158). Every run, reply and error appends forever.

Proposal

  1. In app/Desktop/launcher.ts, pull the log setup at app/Desktop/launcher.ts#L24 into a small writer function so both the console redirect and the new handlers use one code path.
  2. Register process.on('uncaughtException') and process.on('unhandledRejection') in app/Desktop/launcher.ts before startDesktopAgent is awaited at app/Desktop/launcher.ts#L46, so a failure during startup is caught too. Each handler writes one timestamped entry with the error name, message and stack, then runs the existing shutdown (app/Desktop/launcher.ts#L76) with a non-zero code.
  3. Write to the file in those two handlers whether or not stdout is a TTY. Running from source the developer still gets the stack on stderr, and the file gets the same entry.
  4. Cap the file with one rotation: before the process's first append, statSync(LOG_PATH) and, over the limit, renameSync it to Uplink.log.1 (replacing any existing .1) and start a fresh file. Re-check the size on a write counter rather than on every append.
  5. Put the size limit beside LOG_PATH in app/Uplink/settings.ts so the popover work can read the same constant instead of inventing a second one.

Requirements

  • No popover controls in this issue. Open log, Copy diagnostics and Report a problem belong to the menubar and setup track; the footer today is app/Uplink/menubar.html#L128 through app/Uplink/menubar.html#L134 and stays as it is. This issue covers the file's own reliability so those buttons have something worth opening.
  • The handler must not throw. appendFileSync fails if ~/Library/Logs was removed or is unwritable. Wrap the write in try/catch and exit regardless; a crash reporter that crashes is worse than none.
  • Exit, do not carry on. An uncaughtException handler that swallows the error leaves the agent half alive with a dead watcher and a live loopback server. Call shutdown so Craft and caffeinate are killed (app/Desktop/launcher.ts#L79, app/Desktop/launcher.ts#L80) and the lock is released, and guard it with a short timer so a hung agent.stop() cannot keep a broken process running.
  • Rotate, do not delete. Keep Uplink.log.1. The entries leading up to a crash are the ones worth reading, and they are the ones a rotation triggered by that crash's own output would otherwise throw away.
  • No new dependency. appendFileSync, statSync and renameSync from node:fs are enough, and the launcher already imports from there at app/Desktop/launcher.ts#L14.
  • Do not log message bodies in the handler. An error's stack can carry a prompt in a frame argument in some runtimes; write name, message and stack only, nothing from the run that failed.

Acceptance

  • A deliberate throw new Error('boom') inside the agent leaves a timestamped entry in ~/Library/Logs/Uplink.log containing the message and stack, and the process exits non-zero.
  • A rejected promise with no catch produces the same entry.
  • With the limit set low, appending past it leaves Uplink.log under the limit and Uplink.log.1 holding the previous contents, and the app keeps logging after the rotation.
  • Killing the app with SIGTERM still goes through the existing shutdown path and logs the menubar exit line at app/Desktop/launcher.ts#L91.
  • ./buddy lint and ./buddy typecheck pass, and the uplink tests still pass.

Sign in to comment on this issue.