Skip to content

fix(runtime): throw typed ExitError instead of generic Error for simulated exit (#97796)#97803

Merged
openclaw-clownfish[bot] merged 1 commit into
openclaw:mainfrom
maweibin:fix/exit-error-typed-class-97796
Jul 6, 2026
Merged

fix(runtime): throw typed ExitError instead of generic Error for simulated exit (#97796)#97803
openclaw-clownfish[bot] merged 1 commit into
openclaw:mainfrom
maweibin:fix/exit-error-typed-class-97796

Conversation

@maweibin

@maweibin maweibin commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #97796.

What Problem This Solves

createNonExitingRuntime().exit() throws a generic Error, making it impossible for upstream try-catch handlers to distinguish a simulated process exit from a real runtime crash. Any code that needs to handle "exit requested but we're in a non-exiting runtime" differently from an actual error has no clean way to do so.

Why This Change Was Made

Added an internal ExitError class (extending Error) that is thrown by createNonExitingRuntime().exit() instead of a generic Error. Callers can now use instanceof ExitError to distinguish simulated exits from real errors.

ExitError is intentionally kept internal — it is exported from src/runtime.ts for direct test imports, but is not re-exported from the public plugin SDK subpaths (plugin-sdk/runtime.ts, plugin-sdk/runtime-env.ts). This avoids expanding the public plugin SDK API surface without maintainer approval.

User Impact

No user-facing impact. This is an internal runtime improvement for code that uses createNonExitingRuntime.

Evidence

Behavior addressed: createNonExitingRuntime().exit() throws generic Error — no way to distinguish simulated exit from runtime crash.

Real environment tested: Node 24.13.1, local OpenClaw checkout.

Exact steps or command run after this patch:

node --import tsx -e "
import { createNonExitingRuntime, ExitError } from './src/runtime.js';
const rt = createNonExitingRuntime();
try { rt.exit(1); } catch (e) {
  console.log(JSON.stringify({
    isExitError: e instanceof ExitError,
    isError: e instanceof Error,
    name: e.name,
    code: e.code,
    message: e.message
  }));
}
"

Evidence after fix:

{"isExitError":true,"isError":true,"name":"ExitError","code":1,"message":"exit 1"}

Before fix: e instanceof Error is true but there's no way to know it's an exit vs a crash.
After fix: e instanceof ExitError cleanly distinguishes the two.

node scripts/run-vitest.mjs src/runtime.test.ts --run
 Test Files  1 passed (1)
      Tests  8 passed (8)
  • exit function throws ExitError — verifies typed error is thrown
  • ExitError includes exit code — verifies code is preserved in message and instanceof
  • ExitError is distinguishable from generic Error — NEW: proves instanceof ExitError works independently of instanceof Error
node scripts/run-oxlint.mjs src/runtime.ts src/runtime.test.ts
(passed, exit 0)

Observed result after fix: createNonExitingRuntime().exit() throws ExitError with the exit code and a descriptive message. Callers can use instanceof ExitError to distinguish simulated exits from real errors.

What was not tested: Upstream callers that currently catch the generic Error and would benefit from switching to instanceof ExitError. Those are follow-up work and not part of this change.

Regression Test Plan

  • node scripts/run-vitest.mjs src/runtime.test.ts --run — Test Files 1 passed (1), Tests 8 passed (8)
  • node scripts/run-oxlint.mjs src/runtime.ts src/runtime.test.ts — exit 0

Root Cause

In createNonExitingRuntime(), the exit function throws new Error(\exit ${code}`)— a genericErrorinstance. Any upstream code that catches errors from the runtime cannot distinguish "the runtime simulated process.exit" from "something actually crashed." The fix adds a typedExitErrorsubclass so callers can useinstanceof` checks for clean control flow.

AI Assistance

  • AI-assisted: Yes
  • Model used: Claude Opus 4.8 (1M context)
  • Co-Authored-By: Already in commit message

@clawsweeper

This comment has been minimized.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 29, 2026
@maweibin
maweibin force-pushed the fix/exit-error-typed-class-97796 branch 2 times, most recently from b320656 to 4cc553d Compare June 29, 2026 14:25
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 29, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 29, 2026
…lated exit (openclaw#97796)

createNonExitingRuntime.exit() threw a generic Error, making it
impossible for upstream try-catch to distinguish a simulated process
exit from a real runtime crash. Added an ExitError class (internal-only,
not exported from SDK) that callers can check via instanceof.

ExitError is kept internal to avoid expanding the public plugin SDK
API surface without maintainer approval. Tests import directly from
./runtime.js.

Fixes openclaw#97796.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@maweibin
maweibin force-pushed the fix/exit-error-typed-class-97796 branch from 4cc553d to 2669115 Compare June 29, 2026 15:16
@maweibin

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 29, 2026
@maweibin

maweibin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Merge Readiness

Typed ExitError instead of generic Error — callers can now distinguish simulated exit from real crash via instanceof ExitError. Two-line source change + focused tests.

Check Status
Proof proof: sufficient + platinum hermit + ready for maintainer look
Scope src/runtime.ts (+1 internal class), src/runtime.test.ts
Backward compat ExitError extends Error — existing catch (e) still matches
Tests 2 new test cases, all pass
CI ✅ green

@cpojer @CharlesDusek — small runtime contract improvement, ready for review.

@vincentkoc vincentkoc removed the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 6, 2026
@vincentkoc vincentkoc self-assigned this Jul 6, 2026
@vincentkoc

Copy link
Copy Markdown
Member

No description provided.

@vincentkoc vincentkoc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved

@openclaw-clownfish
openclaw-clownfish Bot merged commit 5861070 into openclaw:main Jul 6, 2026
124 of 129 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
…lated exit (openclaw#97796) (openclaw#97803)

createNonExitingRuntime.exit() threw a generic Error, making it
impossible for upstream try-catch to distinguish a simulated process
exit from a real runtime crash. Added an ExitError class (internal-only,
not exported from SDK) that callers can check via instanceof.

ExitError is kept internal to avoid expanding the public plugin SDK
API surface without maintainer approval. Tests import directly from
./runtime.js.

Fixes openclaw#97796.

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
…lated exit (openclaw#97796) (openclaw#97803)

createNonExitingRuntime.exit() threw a generic Error, making it
impossible for upstream try-catch to distinguish a simulated process
exit from a real runtime crash. Added an ExitError class (internal-only,
not exported from SDK) that callers can check via instanceof.

ExitError is kept internal to avoid expanding the public plugin SDK
API surface without maintainer approval. Tests import directly from
./runtime.js.

Fixes openclaw#97796.

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
maweibin added a commit to maweibin/openclaw that referenced this pull request Jul 10, 2026
…eric Error

Replace generic Error throws in facade-loader with MissingPublicSurfaceError
so callers can use instanceof to distinguish missing optional artifacts from
real runtime errors.

Before: throw new Error('Unable to resolve...') — callers match via
  fragile error.message.startsWith(MISSING_PUBLIC_SURFACE_PREFIX)

After: throw new MissingPublicSurfaceError('Unable to resolve...') —
  callers can use error instanceof MissingPublicSurfaceError

MissingPublicSurfaceError is kept internal (not re-exported from public
plugin SDK subpaths), matching the ExitError pattern in openclaw#97803.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tech-Debt]: Use of generic Error for process exit simulation causes swallowed control-flow exceptions

2 participants