feat: distinguish cancelled steps with step:cancel event#48
Merged
Conversation
…event (#39) When a run is cancelled while a step is executing, the step is now recorded as 'cancelled' instead of 'failed' and emits a 'step:cancel' event instead of 'step:fail'. Detection uses controller.signal.aborted so user-initiated AbortErrors (e.g. fetch timeouts) are unaffected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds explicit handling for aborted steps: records step status as Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Controller as AbortController
participant Step as Step Executor
participant Storage as Storage Layer
participant Events as Event Bus
participant Subscribers as Subscribers (SSE / Hooks)
App->>Controller: abort()
Controller->>Step: signal abort on running step
Step->>Step: catch error
Step->>Controller: check signal.aborted
alt aborted == true
Step->>Storage: persist step { status: "cancelled" }
Storage-->>Step: saved
Step->>Events: emit "step:cancel" (runId, jobName, stepName, stepIndex, labels)
else
Step->>Storage: persist step { status: "failed" }
Storage-->>Step: saved
Step->>Events: emit "step:fail"
end
Events->>Subscribers: deliver event
Subscribers-->>Subscribers: refresh UI / consumers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/durably-react/src/client/use-runs.ts (1)
45-51: Alignstep:cancelclient event type with server payload.Server SSE includes
labelsforstep:cancel, but this variant drops it. Keeping the same shape avoids silent type drift and enables label-aware client logic later.Suggested fix
| { type: 'step:cancel' runId: string jobName: string stepName: string stepIndex: number + labels: Record<string, string> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably-react/src/client/use-runs.ts` around lines 45 - 51, The 'step:cancel' client event variant in use-runs.ts is missing the labels field present in the server SSE; update the union variant for type: 'step:cancel' (the object with runId, jobName, stepName, stepIndex) to include a labels property matching the server shape (e.g., labels: string[] or the same type used by the server) so the client event type stays aligned with server payloads and enables label-aware logic.packages/durably/tests/shared/step.shared.ts (1)
508-539: Add one explicitAbortError-without-run-cancel test case.This test covers generic non-cancel errors, but a direct
AbortErrorcase would better lock in the requirement that user-space abort errors still emitstep:failunless the run itself was cancelled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably/tests/shared/step.shared.ts` around lines 508 - 539, Add a new test case similar to the existing "emits step:fail event for non-cancellation errors" that specifically throws an AbortError (using the AbortError type/constructor) from inside the step callback (via defineJob -> run -> step.run) but not as a result of cancelling the run; register the job and attach listeners to 'step:fail' and 'step:cancel', trigger and start the job, and assert that a single step:fail event is emitted with the stepName matching and error message matching the AbortError while no step:cancel events are emitted. Ensure you reference the same symbols used in the file: defineJob, the job's run function calling step.run, the 'step:fail' and 'step:cancel' event handlers, and the AbortError type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/durably-react/src/types.ts`:
- Around line 96-102: The 'step:cancel' union variant in
packages/durably-react/src/types.ts is missing the labels property used by
core/server; update the 'step:cancel' payload shape (the object with type:
'step:cancel', runId, jobName, stepName, stepIndex) to include labels:
Record<string, string> (matching other event variants) so React consumers get
typed access to labels, and adjust any related type exports/usages to accept the
new property.
In `@packages/durably/src/context.ts`:
- Around line 122-146: When isCancelled is true in the catch block that emits
the 'step:cancel' event, rethrow a cancellation-specific error instead of the
original error: replace the existing "throw error" after the
eventEmitter.emit(...) in the cancellation branch with throwing a CancelledError
(or instantiating the project's CancelledError class) so upstream cancellation
handling sees the correct type; ensure other branches still rethrow the original
error. Reference: isCancelled, eventEmitter.emit (type: 'step:cancel'), and the
current "throw error" statement.
In `@website/api/events.md`:
- Around line 220-237: The docs add a `step:cancel` event but the Type
Definitions union example still omits StepCancelEvent; update the DurablyEvent
union example to include the new StepCancelEvent type (i.e., add StepCancelEvent
to the DurablyEvent union alongside existing types) and ensure the
StepCancelEvent shape (runId, jobName, stepName, stepIndex, labels, timestamp,
sequence) matches the example shown in the `step:cancel` section so the Type
Definitions and event examples remain consistent.
---
Nitpick comments:
In `@packages/durably-react/src/client/use-runs.ts`:
- Around line 45-51: The 'step:cancel' client event variant in use-runs.ts is
missing the labels field present in the server SSE; update the union variant for
type: 'step:cancel' (the object with runId, jobName, stepName, stepIndex) to
include a labels property matching the server shape (e.g., labels: string[] or
the same type used by the server) so the client event type stays aligned with
server payloads and enables label-aware logic.
In `@packages/durably/tests/shared/step.shared.ts`:
- Around line 508-539: Add a new test case similar to the existing "emits
step:fail event for non-cancellation errors" that specifically throws an
AbortError (using the AbortError type/constructor) from inside the step callback
(via defineJob -> run -> step.run) but not as a result of cancelling the run;
register the job and attach listeners to 'step:fail' and 'step:cancel', trigger
and start the job, and assert that a single step:fail event is emitted with the
stepName matching and error message matching the AbortError while no step:cancel
events are emitted. Ensure you reference the same symbols used in the file:
defineJob, the job's run function calling step.run, the 'step:fail' and
'step:cancel' event handlers, and the AbortError type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49400391-4331-4858-a1d1-9562a1fb75a2
📒 Files selected for processing (13)
packages/durably-react/src/client/use-runs.tspackages/durably-react/src/hooks/use-runs.tspackages/durably-react/src/types.tspackages/durably/docs/llms.mdpackages/durably/src/context.tspackages/durably/src/events.tspackages/durably/src/index.tspackages/durably/src/schema.tspackages/durably/src/server.tspackages/durably/src/storage.tspackages/durably/tests/shared/step.shared.tswebsite/api/events.mdwebsite/public/llms.txt
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/api/durably-react/types.md`:
- Around line 102-117: The exported StepRecord.status type in use-run-actions.ts
currently only allows 'completed' | 'failed'; update the StepRecord (or exported
type alias) to include 'cancelled' so its union becomes 'completed' | 'failed' |
'cancelled', matching the backend/storage schema and the docs; locate the
StepRecord/type definition in use-run-actions.ts (exported at the top around
line 9) and add the 'cancelled' member to the union.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b069c60d-d2b9-4f41-a3ed-3bbc33614091
📒 Files selected for processing (1)
website/api/durably-react/types.md
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add labels to step:cancel in DurablyEvent type (types.ts) - Add cancelled to StepRecord.status (use-run-actions.ts) - Rethrow CancelledError instead of original error on cancellation (context.ts) - Add StepCancelEvent to DurablyEvent union in docs (events.md) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add step:cancel event listener to server-node example and guides - Handle cancelled step status with gray styling in dashboard examples - Add example apps section to doc-check skill Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cancelledinstead offailedin the databasestep:cancelevent fires when a step is interrupted by run cancellationstep:failcontroller.signal.abortedto distinguish cancellation from other errorsCloses #39
Changes
schema.ts,storage.ts: Add'cancelled'to step status typeevents.ts,index.ts: AddStepCancelEventinterface and exportcontext.ts: Checkcontroller.signal.abortedin catch block to emit correct eventserver.ts: Streamstep:cancelvia SSE to React clientsdurably-react: Handlestep:cancelin both browser and client mode hooksllms.md,events.md,llms.txtupdatedTest plan
step:cancel(notstep:fail)step:failpnpm validatepasses (format, lint, typecheck, all tests)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests