Skip to content

Propagate errors that were silently swallowed - #18

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1786151469-error-propagation
Open

Propagate errors that were silently swallowed#18
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1786151469-error-propagation

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Most error handling in Portreeve is already deliberate (structured RegistryError/reclamation reasons, isMissingFile guards, Promise.allSettled snapshots). This fixes the places where a failure genuinely disappeared — plus one case where the server could hang instead of reporting.

Server shutdown could hang and lose its failure (src/server/server.js). stop() set stopped = true before doing the work, so a failing unlink(socketPath) (EACCES, EPERM, …) meant stoppedPromise never resolved, serve blocked forever, and the second stop() in serveCommand's finally returned early. Shutdown is now memoized and its failure is recorded, logged as a diagnostic, and surfaced:

function stop() { stopping ??= shutdown(); return stopping; }        // memoized
async function shutdown() {
  try { server.stop(true); await unlink(socketPath) /* ignoring ENOENT */; }
  catch (error) { stopFailure = error; writeDiagnostic('error', ); resolveStopped(); throw error; }
  
}
async waitUntilStopped() { await stoppedPromise; if (stopFailure !== null) throw stopFailure; }

serveCommand therefore exits non-zero on a failed shutdown, and its finally reports a shutdown failure on stderr instead of discarding it or masking the primary error.

Failed upgrade rollback claimed success (src/supervision/manager.js). install() swallowed supervisor.stop(), uninstall(), and waitUntilHealthy() rollback failures and always said prior installation was restored, so a half-restored install looked like a clean rollback. Each rollback step is now attempted independently and its failure named in the thrown message:

Portreeve upgrade activation failed and the prior installation was not fully restored
(restarting the previous supervised service failed: systemctl start refused): <original error>

withPort hid lease abandon failures (packages/client/src/client.js). abandon(...).catch(() => {}) meant a leaked lease was invisible. Now a non-collision startup failure whose abandon also fails throws lease_abandon_failed with the startup error as cause, and bind_retry_exhausted carries cause: lastCollision plus details.abandonFailures. PortreeveClientError accepts cause and passes it to Error; documented in docs/client.md.

Desktop background failures were dropped: polling/window void coordinator.refresh(), void coordinator.checkForUpdates(), the update-manifest catch {}, update-state cache read/write, and non-ENOENT renderer asset errors now go through a new reportBackgroundFailure(scope, error) (apps/desktop/main/diagnostics.js) rather than vanishing — the resulting UI states (unavailable, 404) are unchanged.

Left alone deliberately: best-effort temp-file unlink in finally blocks, diagnostic-log tolerating a torn final line, and ephemeral-ports/stack-document fallbacks that are documented degradations.

Testing

bun run check (typecheck, lint, format, tests) passes except release metadata > renders one checksum-pinned Homebrew formula for all targets, which also fails on unmodified main in this environment. New tests cover the failed-shutdown propagation, the rollback-failure message, and lease_abandon_failed.

Link to Devin session: https://app.devin.ai/sessions/00a2e05546fa4b8082b135c0e91d0d77
Requested by: @TrentBrown

@TrentBrown TrentBrown self-assigned this Aug 8, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

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.

1 participant