ExecutionPlan: lifecycle events for retries and execution observability - #7026
Conversation
🦋 Changeset detectedLatest commit: 7e60e62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed PR #7026 at head c2f06dde against main (b8cc53d):
- Added public
ExecutionPlan.Eventtagged union (AttemptStart,AttemptSuccess,AttemptFailure) andExecutionPlan.Optionstype. - Updated
Effect.withExecutionPlanandStream.withExecutionPlanto accept optionaloptions.onEventand union handler requirements into the resultR. - Implemented event instrumentation in
internal/executionPlan.tsfor bothEffect(uninterruptibleMask+onExit) andStream(onStart+onExitscope finalizer). - Added 10 new event tests and type tests for requirement propagation.
- Added
effectpatch changeset.
Validation run: pnpm vitest run packages/effect/test/ExecutionPlan.test.ts, pnpm test-types packages/effect/typetest/Effect.tst.ts, pnpm test-types packages/effect/typetest/Stream.tst.ts, pnpm doctest --run packages/effect/src/ExecutionPlan.ts packages/effect/src/Effect.ts packages/effect/src/Stream.ts, pnpm check, pnpm lint — all passed.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the incremental changes on PR #7026 since the prior pullfrog review at c2f06dde:
- Inlined the option shape directly into both
Effect.withExecutionPlanandStream.withExecutionPlansignatures and removed theExecutionPlan.Optionstype; moved the worked example and handler-semantics JSDoc intoEffect.withExecutionPlan. - Added an Effect test verifying that event
attemptstays in sync withExecutionPlan.CurrentMetadataacross a step-failover boundary. - Added Stream tests confirming
AttemptFailureis emitted for both interruption and defects without retrying.
The prior human review thread asking to inline ExecutionPlan.Options is addressed by the new commit. Targeted runtime tests, type tests, and doctests all pass.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Adds an optional onEvent handler to Effect.withExecutionPlan and Stream.withExecutionPlan, emitting AttemptStart, AttemptSuccess, and AttemptFailure events for each plan attempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…interruption Removes the ExecutionPlan.Options interface per review, inlining the onEvent option object into both withExecutionPlan signatures. Adds Stream test twins for interruption and defects, a CurrentMetadata parity test across a failover boundary, and documents that early downstream termination reports AttemptSuccess. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0d32de7 to
7e60e62
Compare
There was a problem hiding this comment.
Incremental review of the latest push (0d32de70 → 7e60e62d). The new commits rebase the prior ExecutionPlan work and add the onEvent lifecycle observer.
Reviewed:
- New
ExecutionPlan.AttemptStart,ExecutionPlan.AttemptSuccess, andExecutionPlan.AttemptFailure<E>event types and theExecutionPlan.Event<E>union. Effect.withExecutionPlan/Stream.withExecutionPlanoptionalonEventhandler wiring and requirement propagation.- Internal
makeEventEmitterimplementation: event ordering,stepAttemptreset per step, duration measurement, and uninterruptible start/end for effects /onStart/onExitfor streams. - Tests covering retries, failover, layer build failures, interruption, defects, and partial-stream fallback.
- Type tests for data-first/data-last requirement propagation.
Validation run:
pnpm --filter effect test --run test/ExecutionPlan.test.ts✅pnpm test-types packages/effect/typetest/Effect.tst.ts packages/effect/typetest/Stream.tst.ts✅pnpm doctest --run packages/effect/src/ExecutionPlan.ts packages/effect/src/Effect.ts packages/effect/src/Stream.ts✅pnpm --filter effect check✅pnpm lint✅
No new issues found. Approving.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
1 participant
Summary
Adds an optional
onEventhandler toEffect.withExecutionPlanandStream.withExecutionPlanso individual plan attempts can be observed from outside the effect, for logging and metrics.The handler receives an
ExecutionPlan.Event<E>, a tagged union of:AttemptStart— emitted before each attempt runsAttemptSuccess— the attempt succeeded (always the final event), withdurationAttemptFailure— the attempt failed, withdurationand the full failureCauseEach event carries
attempt(cumulative across steps, matchingExecutionPlan.CurrentMetadata.attempt),stepAttempt(1-based within the current step), andstepIndex.Design notes
ExecutionPlan.make, so a shared plan can be observed differently at different call sites.(event) => Effect<void, never, R>: it is awaited inline (events are strictly ordered), cannot fail (observation cannot change the plan's outcome), and its requirements union into the result'sR.AttemptStartis followed by exactly one terminal event. Terminal events run like finalizers, so an interrupted attempt emitsAttemptFailurewith the interruption cause; keying offCausemeans defects are reported too, even though the plan does not retry them.Effect.withExecutionPlanmoved fromdual(2, ...)todual(isEffect(args[0]), ...)to support the optional third argument, matchingStream.withExecutionPlan.Stream.onStart/Stream.onExit(scope finalizer), giving streams full parity including defects and interruption, which the existing error-channelcatch_loop cannot observe.Closes EFF-467
Closes #6699
Validation
pnpm vitest run packages/effect/test/ExecutionPlan.test.ts(16 tests, including 10 new event tests asserting full ordered event arrays)pnpm test-types Effect.tst.ts/pnpm test-types Stream.tst.ts(new handler-requirement propagation tests)pnpm doctest --run packages/effect/src/ExecutionPlan.ts packages/effect/src/Effect.ts packages/effect/src/Stream.tspnpm check,pnpm lintpnpm docgenfails on a pre-existing, unrelatedHttpStaticServerexample error (reproduced on the base commit); the new example typechecks.🤖 Generated with Claude Code