refactor(effect): name the lift's rejection channel; fold boundary settlements without an unknown failure - #517
Conversation
… settlements without an unknown failure lift.ts: `LiftedRejection` names the identity lift's fail channel (the raw thrown/rejected value, narrowed by callers), and `liftPromise` hands the helper Effect's interruption AbortSignal per rc.112's tryPromise contract. reconciler.ts: `waitSettledBoundary` folds a boundary's settlement inside the promise instead of catching an `unknown` off the Effect channel; the rejection reason stays data for `renderErrorFrom` (React's digest-bearing objects), so it is deliberately not normalized through `toRuntimeError`.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
Summary
The two "small ones" from the Effect-conformance audit: the reconciler's momentary
unknownon the fail channel, and the dev seam's identity lift inferring bareunknownat every call site. Behavior-preserving: no runtime path changes; everyliftPromise/liftTrycall site (82) type-checks unchanged.What changed
packages/rsc-runtime/src/reconciler.ts:288–300(audit row:293–295,Effect.tryPromise({ catch: (error) => error })→unknownInEffectCatch). The audit suggestedcatch: toRuntimeError; that would be a behavior change: a boundary's rejection reason is data thatrenderErrorFrom(:130) reads — React rejects a boundary thenable with a{ message, digest }object, andtoRuntimeErrorwould turn a non-Errorobject intonew Error('[object Object]')and lose the digest-derivedcode. InsteadwaitSettledBoundaryfolds both settlements inside the promise (Effect.promise(() => thenable.then(ok, (error) => ({ boundary, error, ok: false })))), so the Effect can never fail and nothingunknownever sits on the error channel — the reason travels as a field ofSettledBoundary, as before. Return type narrows fromEffect<SettledBoundary, Error>toEffect<SettledBoundary>.packages/agent-bundle/src/effect/lift.ts(audit row:11–15). The audit proposed typingEasCodedError | DiagnosticError | unknown; that union collapses tounknownand any narrower union would be unsound — the lift is an identity by contract (typed dev errors and rawAbortSignal.reasonvalues must cross the boundary untouched). So:export type LiftedRejection = unknownnames the channel: hovers and signatures now readEffect<A, LiftedRejection>with the doc explaining it is the raw thrown/rejected value that callers narrow (instanceof,isErrno,Effect.mapErrorinto a typed dev error) rather than a shape to assume, and how the boundary maps whatever reaches it.liftPromise'sevaluateis now(signal: AbortSignal) => PromiseLike<A>— rc.112's ownEffect.tryPromisecontract (Effect.ts:969–973): the signal aborts when the fiber is interrupted, so a cancellable Promise API can take it directly. Zero-arg thunks are unchanged.Effect.tryPromise(fn)is not used (it wraps rejections inCause.UnknownError).Idioms and citations
repos/effect/LLMS.md/ repo sectionunknown/ globalErroron the fail channel; a value that is data stays in the success channelagent-patterns/effect-errors.md"What to avoid:unknownor globalErrorin the fail channel (unknownInEffectCatch)"Effect.promisefor a promise that cannot reject;tryPromise'stry(signal)for interruption-aware liftsEffect.promiserelease), rc.112Effect.promise/tryPromisesignaturesdocs/effect-conventions.mdStage 3 "Hurt": "BareEffect.tryPromise(fn)wraps rejections inCause.UnknownError; always route throughsrc/effect/lift.ts"Tests
packages/agent-bundle/tests/effect-boundary.test.ts(effect lifts): rejected typed errors and raw non-Errorreasons are identity-preserved on the fail channel (Exit+Cause.squash), thrown values likewise vialiftTry; the Promise edge rethrows typed errors as-is and wraps a non-Errorper the mapping table; the lifted helper receives anAbortSignalthat is not aborted while running and aborts when the host signal interrupts the fiber (AbortErrorat the edge).emits a represented boundary error and still completes siblings,emits a replace or error for every boundary that settled before resnapshot) cover the fold; all 55 dispatcher/boundary tests green.pnpm typecheck,pnpm lintgreen;pnpm test:unitgreen (3186 passed).Changeset
skip-changeset: type-level and internal-structure only; no Promise-side value, message, or export changes.Review status
No PR comments are posted by the author; every review thread is answered in this section.
a9e5459