Skip to content

fix(packaged): swallow EPIPE in console echo to stop main-process crash - #7521

Open
lorenzozanee wants to merge 1 commit into
nexu-io:mainfrom
lorenzozanee:fix/packaged-console-echo-epipe
Open

fix(packaged): swallow EPIPE in console echo to stop main-process crash#7521
lorenzozanee wants to merge 1 commit into
nexu-io:mainfrom
lorenzozanee:fix/packaged-console-echo-epipe

Conversation

@lorenzozanee

Copy link
Copy Markdown
Contributor

Fixes #6964

Why

Packaged Electron on Windows launches without a terminal, so the first console.info from did-start-loading throws EPIPE inside Writable.write and crashes the main process with an uncaught exception. The logger already writes to the desktop file but the stdout echo had no guard, so every default launch showed the native crash dialog.

What users will see

Before the dialog appeared on every packaged launch without a terminal and the app exited. After it stays running and the event is recorded only in the desktop log file. No API, env var or install layout change.

Surface area

  • UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack flag, or new OD_* env var
  • API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — added new translation keys (see TRANSLATIONS.md for the locale workflow)
  • New top-level dependency — adding any new entry to the root package.json (dependencies or devDependencies); workspace-package package.json files are out of scope. Include a paragraph on what we get vs. what bytes we ship (see CONTRIBUTING.md → Code style)
  • Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
  • None — internal refactor, docs, tests, or translation update only

Screenshots

Not applicable — the bug was a native crash dialog; the fix restores “no dialog” with the same main window.

Bug fix verification

  • Test path: apps/packaged/tests/logging.test.ts
  • Red on main, green on this branch: the new EPIPE guard cases fail without the wrapper and pass with it.

Validation

pnpm --filter @open-design/desktop build && pnpm --filter @open-design/packaged exec tsc -p tsconfig.json --noEmit && pnpm --filter @open-design/packaged exec tsc -p tsconfig.tests.json --noEmit && pnpm --filter @open-design/packaged exec vitest run -c vitest.config.ts tests/logging.test.ts — 29/29, typecheck clean, git diff --check clean.

Reviewers: @mrcfps @AmyShang-alt

Packaged Electron on Windows has no controlling terminal, so process.stdout is detached and the first console.info from did-start-loading throws EPIPE from inside Writable.write. Wrap console echo in safeEcho that swallows only EPIPE/ERR_STREAM_DESTROYED when the structured code equals those values, and install an async stdio error guard for the nextTick error path; the desktop file logger still records the event, non-harmless errors rethrow.

Fixes nexu-io#6964
@lorenzozanee
lorenzozanee requested a review from a team as a code owner August 28, 2026 01:25
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @lorenzozanee — this is a focused packaged-startup fix, and the logging/test split makes the intent easy to follow.

I’m routing the normal triage now. One quick housekeeping ask: please tick the Surface area checklist — this reads like None on the template — and I’m also marking this for QA validation because a regression here would affect packaged startup behavior. Heads-up as well: #7446 is already open against the same packaged logging path, so it’s worth comparing approaches to avoid two parallel PRs on the same fix.

@lefarcen
lefarcen requested a review from nettee August 28, 2026 01:29
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix needs-validation Runtime change detected; needs human or /explore agent validation. labels Aug 28, 2026

@nettee nettee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lorenzozanee I found one blocking startup-order issue in the new stream guard. The focused logging tests and full packaged checks pass locally, but the guard is installed after an unguarded first-launch console path, so the reported packaged startup crash can still occur.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

// safeEcho's try/catch only covers a *synchronous* throw. It does not
// cover this failure mode: verified live on a packaged Windows build
// where safeEcho alone did not stop the crash. See installStdioErrorGuard.
installStdioErrorGuard([process.stdout, process.stderr]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking: This installs the guard too late for packaged startup. apps/packaged/src/index.ts calls inspectExistingDesktopForLauncher(...) with the raw console before createPackagedDesktopLogger() runs. On the normal first launch, the desktop IPC socket is absent, so requestJsonIpc rejects and inspectExistingDesktopForLauncher logs logger.info("[open-design launcher] inspect-unavailable ...") before this listener exists. That raw console.info writes to detached stdout and can emit the same EPIPE this PR is meant to prevent, so the app may still crash before reaching the guarded renderer lifecycle call.

Install installStdioErrorGuard before launcher inspection (and avoid installing duplicate listeners), or route every pre-logger diagnostic through a guarded bootstrap logger; add a first-launch/no-terminal regression test for this sequence.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen

Copy link
Copy Markdown
Contributor

@lorenzozanee @nettee's blocking review on this head looks like the right next step to address: the stdio guard needs to cover the pre-logger launcher-inspection path as well, and the first-launch/no-terminal regression path still needs test coverage.

Once that startup-order gap is fixed, we can take another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation Runtime change detected; needs human or /explore agent validation. risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Packaged main process crashes with EPIPE on first console.info from renderer lifecycle handlers

3 participants