Skip to content

refactor(worker): express cancellation scopes with fromPromise - #385

Merged
btravers merged 1 commit into
mainfrom
refactor/cancellation-from-promise
Aug 12, 2026
Merged

refactor(worker): express cancellation scopes with fromPromise#385
btravers merged 1 commit into
mainfrom
refactor/cancellation-from-promise

Conversation

@btravers

Copy link
Copy Markdown
Collaborator

cancellableScope / nonCancellableScope hand-rolled what fromPromise(thunk, qualify) already does: run a throwing async call and triage each cause into either a modeled error or a defect.

The manual version needed a try/catch, an explicit rethrow to reach the defect channel, a makeAsyncResult wrapper, and an oxlint-disable comment per site to get that rethrow past unthrown/no-throw.

-  const work = async () => {
-    try {
-      const value = await CancellationScope.cancellable(async () => fn());
-      return Ok(value);
-    } catch (error) {
-      if (isCancellation(error)) {
-        return Err(new WorkflowCancelledError(error));
-      }
-      // oxlint-disable-next-line unthrown/no-throw -- cancellation-scope rethrow: ...
-      throw error;
-    }
-  };
-  return makeAsyncResult<T, WorkflowCancelledError>(work);
+  return fromPromise(
+    () => CancellationScope.cancellable(async () => fn()),
+    (cause, defect) =>
+      isCancellation(cause) ? new WorkflowCancelledError(cause) : defect(cause),
+  );

The qualify callback is the triage: cancellation returns the modeled WorkflowCancelledError, everything else goes to the injected defect helper, whose Defect arm is subtracted from E. So E stays exactly WorkflowCancelledError — with no rethrow and no lint exemption.

Why the thunk overload, not a bare promise

fromPromise accepts Promise<T> | (() => Promise<T>). This uses the thunk form deliberately: it also captures a synchronous throw from CancellationScope.cancellable itself, which is what the old try/catch covered. Passing the promise directly would construct it outside the boundary, letting a synchronous throw escape the AsyncResult entirely instead of landing in qualify.

Behaviour

Unchanged — same modeled error, same defect routing, same AsyncResult<T, WorkflowCancelledError> signature. This is a shape change, not a semantics change.

Verification

Check Result
pnpm typecheck 12/12 projects
pnpm lint (oxlint) clean
pnpm test 555 passed
in-process integration (real Temporal test server) 71 passed — all 14 files, incl. the 9 cancellation tests

The cancellation suite is the one that matters here: it covers ending an execution Cancelled when a blocked scope is cancelled, running a nonCancellable scope to completion despite an outer cancel, and the swallowed-cancellation hazard.

Net −33 lines, and removes 2 of the repo's unthrown/no-throw exemptions.

Independent of #384 (the 5.1.0 → 5.4.0 bump) — fromPromise and its thunk overload both exist in 5.1.0, so this branches off main and merges in either order.

🤖 Generated with Claude Code

`cancellableScope` / `nonCancellableScope` hand-rolled what
`fromPromise(thunk, qualify)` already does: run a throwing async call, triage
each cause into either a modeled error or a defect. The manual version needed
a try/catch, an explicit rethrow to reach the defect channel, a
`makeAsyncResult` wrapper, and an `oxlint-disable` comment per site to get the
rethrow past `unthrown/no-throw`.

The qualify callback IS that triage: cancellation returns the modeled
`WorkflowCancelledError`, everything else goes to the injected `defect`
helper, whose `Defect` arm is subtracted from `E` — so `E` stays exactly
`WorkflowCancelledError` with no rethrow and no lint exemption.

Uses `fromPromise`'s THUNK overload (`() => Promise<T>`) rather than passing a
bare promise, deliberately: the thunk form also captures a *synchronous* throw
from `CancellationScope.cancellable` itself, matching what the old try/catch
covered. Passing the promise directly would let that throw escape the
AsyncResult.

Behaviour is unchanged — same modeled error, same defect routing, same
`AsyncResult<T, WorkflowCancelledError>` signature. Verified by the 9
cancellation in-process integration tests (real Temporal test server),
including the outer-cancel and swallowed-cancellation cases.

Net -33 lines, and removes 2 of the repo's `unthrown/no-throw` exemptions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 12, 2026 16:25

Copilot AI 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.

Pull request overview

Refactors the worker’s typed CancellationScope wrappers to use unthrown’s fromPromise(..., qualify) boundary for modeled-cancellation vs defect triage, removing the previous hand-rolled try/catch + rethrow pattern while preserving the existing AsyncResult<T, WorkflowCancelledError> API.

Changes:

  • Replaced makeAsyncResult + manual Ok/Err construction with fromPromise for cancellableScope and nonCancellableScope.
  • Removed the explicit rethrow path and associated unthrown/no-throw disable comments from these scope helpers.
  • Kept cancellation triage behavior explicit via qualify and used the thunk overload to capture synchronous throws at the boundary.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@btravers
btravers merged commit 9e31be4 into main Aug 12, 2026
13 checks passed
@btravers
btravers deleted the refactor/cancellation-from-promise branch August 12, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants