diff --git a/.agents/rules/handlers.md b/.agents/rules/handlers.md index 9a341817..157067d8 100644 --- a/.agents/rules/handlers.md +++ b/.agents/rules/handlers.md @@ -83,7 +83,13 @@ export const processOrder = declareWorkflow({ // context.cancellableScope / context.nonCancellableScope — see below const inventory = await context.activities.validateInventory({ orderId: args.orderId }); - return { status: inventory.available ? "confirmed" : "rejected" }; + if (inventory.isDefect()) { + throw inventory.cause; + } + if (inventory.isErr()) { + return { status: "rejected" }; + } + return { status: inventory.value.available ? "confirmed" : "rejected" }; }, }); ``` @@ -92,13 +98,24 @@ Workflow code is deterministic — see [workflow-determinism.md](./workflow-dete Typed-error semantics inside the workflow context: -- Activities **without** a declared `errors` map keep the throwing - `Promise` shape above. +- **Every** activity call returns `AsyncResult` — declared `errors` map or + not. There is no throwing `Promise` shape anymore; narrow the + result with `isOk()`/`isErr()`, or use `propagateActivityFailure` to let + the failure escape and have Temporal decide the workflow's outcome. A bare + `await context.activities.x(...)` compiles either way — it's easy to + discard the `AsyncResult` by accident and silently swallow a failure. + **Never** use unthrown's `.getOrThrow()` here: it throws the + `ActivityError`/`ActivityCancelledError` wrapper, which is a `TaggedError` + and not a `TemporalFailure` — Temporal retries that as a workflow-_task_ + failure indefinitely instead of failing the workflow. - Activities **with** a declared `errors` map return `AsyncResult` (mirroring the child-workflow API): declared failures rehydrate into typed `ContractError`s, anything else is `Err(ActivityError)` with the unwrapped - cause, cancellation is `Err(ActivityCancelledError)`. + cause, cancellation is `Err(ActivityCancelledError)`. Activities + **without** a declared `errors` map return + `AsyncResult` — same + shape, minus the declared-error members. - `context.errors` holds typed constructors for the workflow's own declared errors; `throw context.errors.X(data)` fails the execution as an `ApplicationFailure` the typed client rehydrates. Never throw a bare @@ -149,16 +166,35 @@ Workflows opt into cancellation control via `context.cancellableScope` / `contex ```typescript implementation: async (context, args) => { + // `fn`'s return value becomes the scope's `T` verbatim, so await and + // narrow the activity's own AsyncResult HERE, inside the callback — + // returning it un-awaited would make `T` the AsyncResult itself, which + // has no `isOk`/`isErr`/`.value`. const result = await context.cancellableScope(async () => { - return context.activities.processStep(args); + const step = await context.activities.processStep(args); + if (step.isDefect()) { + throw step.cause; + } + return step.isOk(); }); + if (result.isDefect()) { + throw result.cause; // a genuine bug thrown inside the scope, not a cancel + } if (result.isErr()) { // Workflow was cancelled. Cleanup that must not be cancelled itself - // goes inside `nonCancellableScope`. - await context.nonCancellableScope(async () => { - await context.activities.releaseResources(args); + // goes inside `nonCancellableScope`. Capture ITS OWN AsyncResult too — + // a bare `await` would silently discard both a defect thrown during + // cleanup and the un-awaited activity result. + const released = await context.nonCancellableScope(async () => { + const step = await context.activities.releaseResources(args); + if (step.isErr()) { + // best-effort cleanup — log and continue regardless + } }); + if (released.isDefect()) { + throw released.cause; // a genuine bug in cleanup, not a cancel + } return { status: "cancelled" }; } diff --git a/.changeset/uniform-activity-result.md b/.changeset/uniform-activity-result.md new file mode 100644 index 00000000..9b2fcc36 --- /dev/null +++ b/.changeset/uniform-activity-result.md @@ -0,0 +1,106 @@ +--- +"@temporal-contract/worker": major +--- + +Every workflow-side activity call now returns `AsyncResult`. + +Previously the call convention depended on whether the contract declared an +`errors` map: activities with declared errors returned `AsyncResult`, those +without returned a `Promise` that threw. The call site gave no indication +which applied, and the throwing path contradicted the library's own +"activities never throw" convention. + +**The dangerous part: a bare, discarded `await` compiles identically before +and after this change, and now silently swallows the failure.** Before, +`await context.activities.charge(input);` on its own line still threw on +failure (for an activity with no declared `errors` map) or, at worst, left an +unused `AsyncResult` you'd likely notice. Now every call returns +`AsyncResult`, so that exact line type-checks, discards the outcome, and lets +the workflow continue as if the call succeeded — nothing here is a type +error. Audit every activity call site: narrow the result or pass it through +`propagateActivityFailure` (below). Don't rely on the compiler to find these. + +**Migrating.** Where the workflow should handle the failure, narrow it: + +```ts +const result = await context.activities.charge(input); +if (result.isErr()) { + /* ... */ +} +``` + +Where the failure should escape and let Temporal fail the workflow, use the +new `propagateActivityFailure` helper: + +```ts +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; + +await propagateActivityFailure(context.activities.charge(input)); +``` + +**Do not use unthrown's `.getOrThrow()` for this.** It throws the +`ActivityError` wrapper, which is not a `TemporalFailure`; Temporal treats +that as a workflow-_task_ failure and retries it indefinitely, so the +workflow stalls until its execution timeout instead of failing. The named +`propagateActivityFailure` helper rethrows the preserved original failure, +which is exactly what escaped the workflow before this change. + +**Also note:** swallowing `ActivityCancelledError` makes a workflow complete +as `Completed` rather than `Cancelled`. That hazard previously applied only +to activities declaring an `errors` map; it now applies to every activity. + +**If you wrap an activity call in `cancellableScope`/`nonCancellableScope`, +this is a source break, not just a behavior change.** Both scopes are generic +over whatever `fn` returns, verbatim — they do not await it for you. Before +this change, `() => context.activities.charge(input)` returned a plain +`Promise`, so the scope's own `T` was `Output`. Now it returns an +`AsyncResult`, and `AsyncResult` is deliberately not a full +`PromiseLike` (no `.catch`/`.finally`), so `T` becomes the un-awaited +`AsyncResult` itself — a type with no `isOk`/`isErr`/`.value`. Code like: + +```ts +const scoped = await context.cancellableScope(() => context.activities.charge(input)); +if (scoped.isOk()) { + scoped.value.transactionId; // ❌ no longer compiles — scoped.value is an AsyncResult +} +``` + +stops compiling. Await and narrow the activity call _inside_ the callback +instead: + +```ts +const scoped = await context.cancellableScope(async () => { + const charged = await context.activities.charge(input); + if (charged.isDefect()) { + throw charged.cause; + } + if (charged.isErr()) { + return { ok: false as const, error: charged.error }; + } + return { ok: true as const, value: charged.value }; // now a plain value, not an AsyncResult +}); +``` + +`ActivityErrorsFor` — the error union used by +`WorkflowInferActivity`'s `AsyncResult` — is now exported from +`@temporal-contract/worker/workflow`, so consumers can name the error channel +directly (for example, to write a helper generic over an activity's error +type) instead of only the call signature. + +`ActivityError` also grows a new `originalFailure` field: the failure exactly +as caught, before `cause`'s unwrap (typically Temporal's `ActivityFailure` +wrapper). It exists so `propagateActivityFailure` can re-raise the exact +failure Temporal originally produced without changing what `cause` means for +existing consumers — see [the reference docs](/reference/errors#activityerror). + +`propagateActivityFailure` also accepts the `AsyncResult` returned by +`context.executeChildWorkflow` / `context.startChildWorkflow` and by +`context.cancellableScope` / `context.nonCancellableScope`. It re-raises +`ChildWorkflowCancelledError` / `WorkflowCancelledError` the same way it +re-raises a cancelled activity — their `cause` always holds the original +Temporal failure. `ChildWorkflowError` re-raises `cause` too when one is +present (a failed child execution), but three of its construction sites +(child input/output/signal-input validation) carry no `cause` at all; for +those, this helper converts to a terminal `ContractMisuseError` instead of +re-raising the bare `TaggedError`, so passing one of those through it does +not stall the workflow the way a bare `throw` of that `TaggedError` would. diff --git a/.changeset/v8-audit-remediation.md b/.changeset/v8-audit-remediation.md index b076264e..fca89ac9 100644 --- a/.changeset/v8-audit-remediation.md +++ b/.changeset/v8-audit-remediation.md @@ -29,7 +29,7 @@ v8 audit remediation — a second full-surface pass hardening robustness, the un - The in-workflow handler binders are renamed to the `handle*` convention: `context.defineSignal`/`defineQuery`/`defineUpdate` → `context.handleSignal`/`handleQuery`/`handleUpdate` (no collision with the contract-authoring `define*` helpers or Temporal's own functions). - Previously-internal types are now exported so implementations can be factored out of the `declareWorkflow`/`declareActivitiesHandler` calls: `WorkflowContext`, `DeclareWorkflowOptions`, `WorkflowImplementation`, the child-workflow handle types, the signal/query/update handler-implementation types, `WorkflowInferActivity`, `DeclareActivitiesHandlerOptions`, `TypedContinueAsNewOptions`, plus the new `ActivityImplementationFor` / `GlobalActivityImplementationFor` helpers. - `qualifyFailure(errorType, options)` requires an `expected` discriminator (an error class, an array of classes, a predicate, or the explicit literal `"any"`). Causes matching `expected` are wrapped into the modeled `ApplicationFailure`; everything else — a `TypeError` from a bug, say — rides the **defect** channel instead of being mislabelled a business error. A matched inner `ApplicationFailure` with `nonRetryable: true` is inherited by default. -- New `rethrowCancellation(error)` helper. When an activity declares an `errors` map, cancellation surfaces as `Err(ActivityCancelledError)`; generic error handling that folds every `Err` to a fallback would complete the workflow instead of cancelling it. The cancellation error classes' JSDoc documents the hazard and the helper. +- New `rethrowCancellation(error)` helper. Cancelling an activity call surfaces as `Err(ActivityCancelledError)`; generic error handling that folds every `Err` to a fallback would complete the workflow instead of cancelling it. The cancellation error classes' JSDoc documents the hazard and the helper. (See the separate uniform-activity-result changeset in this same release: this hazard now applies to every activity call, not only ones declaring an `errors` map.) - Async query/update schemas are rejected at bind time (`ContractMisuseError`) rather than on the first live request. - `context.continueAsNew` can no longer have its validated `workflowType`/`taskQueue` overridden through the options bag. - `ChildWorkflowError` carries a structured `workflowName`; the input/output `ValidationError` subclasses carry a `readonly direction: "input" | "output"`. diff --git a/docs/explanation/nexus.md b/docs/explanation/nexus.md index f4ac879e..ab4ed5aa 100644 --- a/docs/explanation/nexus.md +++ b/docs/explanation/nexus.md @@ -69,7 +69,7 @@ covers and drop to the SDK at the Nexus boundary: ```typescript import * as nexus from "@temporalio/nexus"; -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; const paymentService = nexus.service("PaymentService", { charge: nexus.operation<{ customerId: string; amount: number }, { transactionId: string }>(), @@ -81,7 +81,9 @@ export const processOrder = declareWorkflow({ activityOptions: { startToCloseTimeout: "1 minute" }, implementation: async (context, order) => { // Contract-typed for everything local... - const reserved = await context.activities.reserveInventory({ items: order.items }); + const reserved = await propagateActivityFailure( + context.activities.reserveInventory({ items: order.items }), + ); // ...raw SDK across the namespace boundary. const client = nexus.createNexusClient({ diff --git a/docs/explanation/the-result-model.md b/docs/explanation/the-result-model.md index 02af5f20..cd255ce1 100644 --- a/docs/explanation/the-result-model.md +++ b/docs/explanation/the-result-model.md @@ -64,50 +64,76 @@ const client = await TypedClient.create({ client: temporalClient }).get(); This is the table to internalize: -| Boundary | Shape | Why | -| --------------------------------------------------- | ------------------------------------------------------------------------------------ | --------------------------------------------- | -| **Activity implementation** returns | `AsyncResult` | You author the failure. Explicit is better | -| **Workflow calls an activity** (no declared errors) | `Promise` — throws on failure | Temporal's retry policy is the handler | -| **Workflow calls an activity** (declared errors) | `AsyncResult` | You declared these to branch on them | -| **Workflow calls a child workflow** | `AsyncResult` | A peer operation; failure is usually a branch | -| **Workflow cancellation scope** | `AsyncResult` | Cancellation is an expected outcome | -| **Client calls a workflow** | `AsyncResult` | Crossing a process boundary | +| Boundary | Shape | Why | +| ----------------------------------- | -------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------- | +| **Activity implementation** returns | `AsyncResult` | You author the failure. Explicit is better | +| **Workflow calls an activity** | `AsyncResult` — plus `ContractErrorUnion` when declared | Uniform: every call returns a Result, whether or not the contract declares `errors` | +| **Workflow calls a child workflow** | `AsyncResult` | A peer operation; failure is usually a branch | +| **Workflow cancellation scope** | `AsyncResult` | Cancellation is an expected outcome | +| **Client calls a workflow** | `AsyncResult` | Crossing a process boundary | + +### Every activity call returns a Result + +Inside a workflow, `await context.activities.chargeCard(...)` gives you an +`AsyncResult` — never a plain value, and never a call that throws through. +That is true whether or not the contract declares an `errors` map: an activity +with no declared errors still folds into `Err(ActivityError | +ActivityCancelledError)` on failure, on the same channel as one with a full +declared error union. The call convention no longer depends on reading the +contract to find out whether a given activity call throws or returns a +`Result` — it's uniform. -### Why activities unwrap by default - -Inside a workflow, `await context.activities.chargeCard(...)` gives you a plain -value. The `Result` is unwrapped for you. +```typescript +const charge = await context.activities.chargeCard({ customerId, amount }); +if (charge.isErr()) { + // charge.error: ActivityError | ActivityCancelledError +} +``` -That is not an inconsistency — it reflects who handles the failure. When an -activity fails, Temporal's retry policy takes over: it retries with backoff, and -only after exhausting the policy does the failure reach your workflow. By then -there is usually nothing sensible to do but let it propagate. +### Handle it, or let Temporal handle it -Forcing every activity call into a result fold would add ceremony to code whose -correct behaviour is almost always "let it throw": +Most activity failures still have one sensible response: let Temporal's retry +policy exhaust, then fail the workflow. Narrowing every such call site would +add ceremony to code whose correct behaviour is "let it throw" — so use +`propagateActivityFailure` to re-raise the original failure and hand the +outcome to Temporal, the same "let it throw" behaviour a bare `await` gave you +before this call convention became uniform: ```typescript -// what you write -const charge = await context.activities.chargeCard({ customerId, amount }); -const shipment = await context.activities.createShipment({ orderId }); +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; -// what a uniform Result API would force -const charge = await context.activities.chargeCard({ customerId, amount }); -if (charge.isErr()) return { status: "failed" }; -const shipment = await context.activities.createShipment({ orderId }); -if (shipment.isErr()) return { status: "failed" }; +const charge = await propagateActivityFailure( + context.activities.chargeCard({ customerId, amount }), +); +const shipment = await propagateActivityFailure(context.activities.createShipment({ orderId })); ``` -### Why declaring errors changes that +**Do not use unthrown's `.getOrThrow()` for this.** It throws the +`ActivityError`/`ActivityCancelledError` wrapper — a `TaggedError`, not a +`TemporalFailure` — and Temporal treats a non-`TemporalFailure` thrown from +workflow code as a workflow-_task_ failure, retrying it indefinitely rather +than failing the execution. `propagateActivityFailure` re-raises the +_preserved original_ Temporal failure instead, which is what actually fails +the workflow. + +### Why declaring errors still matters + +Declaring an `errors` map doesn't change the call _shape_ anymore — it changes +what's in the error _channel_. It folds the declared, rehydrated +`ContractError`s into the union alongside `ActivityError` / +`ActivityCancelledError`, and the exhaustive matcher then makes sure every +fold handles each one: -Declare an `errors` map on an activity and the call site becomes an -`AsyncResult`. That is the signal that you have failures the workflow is -_meant_ to branch on — and the exhaustive matcher then makes sure you handle -each one. +```typescript +const charged = await context.activities.chargeCard({ customerId, amount }); +if (charged.isErr()) { + // charged.error: ContractErrorUnion<...> | ActivityError | ActivityCancelledError +} +``` -It is an opt-in trade: ceremony in exchange for typed, exhaustive handling. -Declare errors on the activities whose failures drive workflow decisions; leave -the rest throwing. +Declare errors on the activities whose failures should drive workflow +decisions; for the rest, `propagateActivityFailure` keeps the call site to a +single line. ### Why child workflows never unwrap diff --git a/docs/explanation/workflow-determinism.md b/docs/explanation/workflow-determinism.md index e40eaac6..60b957a5 100644 --- a/docs/explanation/workflow-determinism.md +++ b/docs/explanation/workflow-determinism.md @@ -20,9 +20,11 @@ _recomputed_. ```typescript implementation: async (context, order) => { - let total = 0; // recomputed on replay + let total = 0; // recomputed on replay const charge = await context.activities.chargeCard({ ... }); // from history - total += charge.amount; // recomputed identically + if (charge.isOk()) { + total += charge.value.amount; // recomputed identically + } // ... } ``` @@ -101,10 +103,31 @@ Temporal's `CancellationScope` and surface cancellation as `Err(WorkflowCancelledError)`: ```typescript -const result = await context.cancellableScope(() => context.activities.processStep(args)); +const result = await context.cancellableScope(async () => { + // Narrow the activity's own AsyncResult here — it's independent of the + // scope's cancellation channel. + const step = await context.activities.processStep(args); + return step.isOk(); +}); +if (result.isDefect()) { + throw result.cause; // a genuine bug thrown inside the scope, not a cancel +} if (result.isErr()) { - await context.nonCancellableScope(() => context.activities.releaseResources(args)).getOrThrow(); + // Capture nonCancellableScope's OWN AsyncResult too — a bare `await` here + // would silently discard a defect thrown inside the cleanup callback. See + // "await is not an extractor" in /explanation/the-result-model. + const released = await context.nonCancellableScope(async () => { + // Narrow inside here too — `releaseResources(...)`'s own AsyncResult, + // left un-awaited, would otherwise be silently discarded a second time. + const step = await context.activities.releaseResources(args); + if (step.isErr()) { + // best-effort cleanup — log and continue regardless + } + }); + if (released.isDefect()) { + throw released.cause; // a genuine bug in cleanup, not a cancel + } return { status: "cancelled" }; } ``` @@ -129,11 +152,12 @@ Temporal's versioning API handles this: ```typescript import { patched } from "@temporalio/workflow"; +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; if (patched("add-fraud-check")) { - await context.activities.scoreRisk({ orderId }); // new path + await propagateActivityFailure(context.activities.scoreRisk({ orderId })); // new path } -await context.activities.chargeCard({ ... }); // both paths +await propagateActivityFailure(context.activities.chargeCard({ ... })); // both paths ``` `patched()` returns `true` for new executions and for old ones that already diff --git a/docs/how-to/continue-as-new.md b/docs/how-to/continue-as-new.md index b27204d1..6417e249 100644 --- a/docs/how-to/continue-as-new.md +++ b/docs/how-to/continue-as-new.md @@ -10,7 +10,7 @@ arguments and an empty history. ## The basic pattern ```typescript -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { sleep } from "@temporalio/workflow"; export const pollSubscription = declareWorkflow({ @@ -19,7 +19,9 @@ export const pollSubscription = declareWorkflow({ activityOptions: { startToCloseTimeout: "1 minute" }, implementation: async (context, args) => { for (let i = 0; i < 100; i += 1) { - await context.activities.chargeSubscription({ subscriptionId: args.subscriptionId }); + await propagateActivityFailure( + context.activities.chargeSubscription({ subscriptionId: args.subscriptionId }), + ); await sleep("30 days"); } @@ -51,12 +53,12 @@ implementation: async (context, args) => { let cursor = args.cursor; while (true) { - const batch = await context.activities.fetchBatch({ cursor }); + const batch = await propagateActivityFailure(context.activities.fetchBatch({ cursor })); if (batch.items.length === 0) { return { processed }; } - await context.activities.processBatch({ items: batch.items }); + await propagateActivityFailure(context.activities.processBatch({ items: batch.items })); processed += batch.items.length; cursor = batch.nextCursor; // advance, so the next fetch makes progress diff --git a/docs/how-to/handle-cancellation.md b/docs/how-to/handle-cancellation.md index 1d200254..c8db7798 100644 --- a/docs/how-to/handle-cancellation.md +++ b/docs/how-to/handle-cancellation.md @@ -39,52 +39,87 @@ export const processOrder = declareWorkflow({ contract: orderContract, activityOptions: { startToCloseTimeout: "5 minutes" }, implementation: async (context, order) => { - const charged = await context.cancellableScope(() => - context.activities.chargeCard({ customerId: order.customerId, amount: order.total }), - ); + const scoped = await context.cancellableScope(async () => { + // Narrow the activity's own AsyncResult INSIDE the scope's callback. + // `cancellableScope` is generic over whatever `fn` returns — `T` becomes + // whatever type `fn` resolves to, verbatim. `AsyncResult` is + // deliberately NOT a full `PromiseLike` (no `.catch`/`.finally`), so + // returning `context.activities.chargeCard(...)` un-awaited would make + // `T` the un-awaited `AsyncResult` itself — a type with no `isOk`/ + // `isErr`/`.value` (those live only on the plain `Result` you get by + // awaiting). Await it here, and hand the scope a plain, narrowable + // value instead. + const charged = await context.activities.chargeCard({ + customerId: order.customerId, + amount: order.total, + }); + if (charged.isDefect()) { + throw charged.cause; // an unmodeled bug — surfaces as the scope's own defect + } + if (charged.isErr()) { + // ActivityError, ActivityCancelledError, or a declared contract error. + return { ok: false as const, error: charged.error }; + } + return { ok: true as const, transactionId: charged.value.transactionId }; + }); - // Narrow positively: an `AsyncResult` has three channels (Ok/Err/Defect), - // so `charged.value` only compiles after `isOk()`. - if (charged.isOk()) { - return { status: "completed" as const, transactionId: charged.value.transactionId }; + // Narrow positively: an `AsyncResult` has three channels (Ok/Err/Defect). + if (scoped.isDefect()) { + throw scoped.cause; // a genuine bug thrown inside the scope, not a cancel } - if (charged.isDefect()) { - throw charged.cause; // a genuine bug thrown inside the scope, not a cancel + if (scoped.isErr()) { + // Err(WorkflowCancelledError): the scope itself was cancelled mid-charge + // — nothing to compensate. + return { status: "cancelled" as const }; } - // Err(WorkflowCancelledError): cancelled mid-charge — nothing to compensate. - return { status: "cancelled" as const }; + if (!scoped.value.ok) { + return handleChargeFailure(scoped.value.error); + } + + return { status: "completed" as const, transactionId: scoped.value.transactionId }; }, }); ``` -The `Err` channel of a scope is exactly one type: `WorkflowCancelledError`. -Anything _else_ thrown inside the scope is an unmodeled failure and rides the -defect channel — so a genuine bug is never mistaken for a cancellation. +The `Err` channel of the scope itself is exactly one type: +`WorkflowCancelledError`, raised when the workflow (or an ancestor scope) is +cancelled while `fn` is in flight. Anything _else_ thrown directly inside the +scope (not returned as a `Result`) is an unmodeled failure and rides the +defect channel — so a genuine bug is never mistaken for a cancellation. The +activity call's _own_ cancellation — the in-flight call itself getting +cancelled — is a separate, independent thing: it surfaces inside `charged`, +folded into the small `{ ok, ... }` envelope the callback returns, not as a +member of `scoped`'s own error union. -### Activities that declare their own errors +### Every activity call carries this hazard -When an activity declares an `errors` map, cancelling it no longer throws -through — it surfaces as `Err(ActivityCancelledError)`, one more member of that -activity's error union. Generic handling that folds _every_ `Err` to a fallback -value will therefore let the workflow **complete** instead of cancelling: +Cancelling an in-flight activity call surfaces as `Err(ActivityCancelledError)` +— one more member of that activity's error union, whether or not the contract +declares any `errors` at all. Generic handling that folds _every_ `Err` to a +fallback value will therefore let the workflow **complete** instead of +cancelling: ```typescript -import { rethrowCancellation } from "@temporal-contract/worker/workflow"; +import { ActivityCancelledError, rethrowCancellation } from "@temporal-contract/worker/workflow"; const charged = await context.activities.chargeCard({ ... }); -if (!charged.isOk()) { - if (charged.isDefect()) throw charged.cause; - // Re-raise a cancellation instead of swallowing it into the fallback path. - // For any other declared error, handle it as usual below. - rethrowCancellation(charged.error); +if (charged.isErr()) { + if (charged.error instanceof ActivityCancelledError) { + // Re-raise the cancellation instead of folding it into the fallback path. + rethrowCancellation(charged.error); + } + // Any other failure (ActivityError, or a declared contract error): handle + // it as usual. return handleChargeFailure(charged.error); } ``` -`rethrowCancellation` throws the underlying `CancelledFailure` when the error is -a cancellation and is a no-op otherwise, so the workflow ends **Cancelled** the -way the operator's `cancel()` intended. +`rethrowCancellation` throws the underlying `CancelledFailure`, so the workflow +ends **Cancelled** the way the operator's `cancel()` intended. It only accepts +a cancellation error (`ActivityCancelledError`, +`ChildWorkflowCancelledError`, or `WorkflowCancelledError`) — narrow to one of +those first, as above, rather than passing the whole error union. ## Clean up without being interrupted @@ -92,28 +127,43 @@ Once a workflow is cancelled, further activity calls are cancelled too. Cleanup must run in a `nonCancellableScope`: ```typescript +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; + implementation: async (context, order) => { let transactionId: string | undefined; const shipped = await context.cancellableScope(async () => { - const charge = await context.activities.chargeCard({ - customerId: order.customerId, - amount: order.total, - }); + // Await and narrow the activity's own AsyncResult INSIDE the scope's + // callback — `propagateActivityFailure` lets a genuine (non-cancellation) + // charge failure ride the defect channel via the scope's own throw + // handling, same as it would have without the scope. + const charge = await propagateActivityFailure( + context.activities.chargeCard({ customerId: order.customerId, amount: order.total }), + ); transactionId = charge.transactionId; - return context.activities.createShipment({ orderId: order.orderId }); + return propagateActivityFailure(context.activities.createShipment({ orderId: order.orderId })); }); + if (shipped.isDefect()) { + throw shipped.cause; // a genuine bug — or a propagated non-cancellation failure + } if (shipped.isErr()) { // Cancelled after the charge but before shipping — refund. // Without the non-cancellable scope this refund would itself be cancelled. if (transactionId !== undefined) { - // Unwrap: a refund that silently failed is worse than a loud failure, - // and a bare `await` would discard both the Err and any defect. - await context - .nonCancellableScope(() => context.activities.refundPayment({ transactionId })) - .getOrThrow(); + // Capture into a const: `transactionId` is a `let` reassigned inside + // an earlier closure, so TypeScript cannot carry the `!== undefined` + // narrowing across into this new arrow function. + const chargedTransactionId = transactionId; + const refunded = await context.nonCancellableScope(() => + propagateActivityFailure( + context.activities.refundPayment({ transactionId: chargedTransactionId }), + ), + ); + if (refunded.isDefect()) { + throw refunded.cause; // a refund that silently failed is worse than a loud failure + } } return { status: "cancelled" as const }; } @@ -165,7 +215,10 @@ processOrder: { } return ApplicationFailure.create({ type: "EXPORT_FAILED", - cause: error instanceof Error ? error : undefined, + // `exactOptionalPropertyTypes` rejects an explicit `cause: undefined` + // (the field's type is `Error`, not `Error | undefined`) — spread it + // in only when there is one. + ...(error instanceof Error ? { cause: error } : {}), }); }, ), @@ -185,27 +238,31 @@ activityOptionsByName: { ## Do not swallow cancellation -A bare `catch` around an activity call will absorb the cancellation and leave +An activity call never throws — it resolves to an `AsyncResult`, which is a +success-only thenable (see [The result model](/explanation/the-result-model)). +A `try/catch` around one does nothing: there is nothing for the `catch` block +to catch. +Generic handling that folds _every_ `Err` — including `ActivityCancelledError` +— into "log and continue" is what actually absorbs the cancellation and leaves the workflow running after a cancel request: ```typescript -// ❌ swallows cancellation -try { - await context.activities.sendNotification({ ... }); -} catch (error) { +// ❌ swallows cancellation — ActivityCancelledError falls into the same +// generic branch as an ordinary notification failure +const sent = await context.activities.sendNotification({ ... }); +if (sent.isErr()) { log.warn("notification failed, continuing"); } -// ✅ re-throws it -import { isCancellation } from "@temporalio/workflow"; +// ✅ re-raises it +import { ActivityCancelledError, rethrowCancellation } from "@temporal-contract/worker/workflow"; -try { - await context.activities.sendNotification({ ... }); -} catch (error) { - if (isCancellation(error)) { - throw error; +const sent = await context.activities.sendNotification({ ... }); +if (sent.isErr()) { + if (sent.error instanceof ActivityCancelledError) { + rethrowCancellation(sent.error); } - log.warn(`notification failed, continuing: ${error}`); + log.warn(`notification failed, continuing: ${sent.error.message}`); } ``` diff --git a/docs/how-to/model-domain-errors.md b/docs/how-to/model-domain-errors.md index 33f567d2..694e1caf 100644 --- a/docs/how-to/model-domain-errors.md +++ b/docs/how-to/model-domain-errors.md @@ -123,14 +123,16 @@ Temporal as a _task_ failure and retried forever, whereas an ## Consume one in a workflow -Declaring errors on an activity **changes its workflow-side call signature.** +Every activity call returns an `AsyncResult` — declaring an `errors` map doesn't change that shape, +it folds the declared, rehydrated errors into the same channel: -| The activity declares | The workflow call returns | -| --------------------- | ------------------------------------------------------------------------------------ | -| no `errors` map | `Promise` — Temporal's native behaviour; a failure throws | -| an `errors` map | `AsyncResult` | +| The activity declares | The workflow call's error channel | +| --------------------- | --------------------------------------------------------------- | +| no `errors` map | `ActivityError \| ActivityCancelledError` | +| an `errors` map | `ContractErrorUnion \| ActivityError \| ActivityCancelledError` | -So an errors-declaring activity is awaited as a result, not a plain value: +So every activity is awaited as a result, not a plain value: ```typescript import { CONTRACT_ERROR_TAG } from "@temporal-contract/contract"; @@ -177,9 +179,11 @@ the unwrapped actionable failure, with Temporal's `ActivityFailure` wrapper already seen through. ::: tip This is a deliberate trade -Declaring errors buys typed, exhaustive handling but changes the call site from -`await activity(...)` to a result fold. Declare errors on the activities whose -failures the workflow actually branches on, and leave the rest throwing. +Every activity call is already a `Result` — declaring errors doesn't add a +result fold, it adds typed members to the one you already have. Declare errors +on the activities whose failures the workflow actually branches on; for the +rest, `propagateActivityFailure` keeps the call site to a single line instead +of a fold. See [The result model](/explanation/the-result-model). ::: ## Consume one on the client diff --git a/docs/how-to/run-child-workflows.md b/docs/how-to/run-child-workflows.md index 1c5f4144..d7aed4e4 100644 --- a/docs/how-to/run-child-workflows.md +++ b/docs/how-to/run-child-workflows.md @@ -12,7 +12,7 @@ same-contract and cross-contract calls look identical. `executeChildWorkflow` starts the child and waits for its result: ```typescript -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { P } from "unthrown"; import { orderContract } from "./contract.js"; @@ -45,10 +45,13 @@ export const processOrder = declareWorkflow({ }); ``` -**Child workflows return a `Result`; activities do not.** That asymmetry is -deliberate — a child workflow is a peer operation whose failure is usually a -branch in your logic, whereas an activity failure is normally something -Temporal's retry policy should handle. See +Both activity and child-workflow calls return an `AsyncResult` — the +uniformity is deliberate. What differs is the usual _response_: a child +workflow is a peer operation whose failure is usually a branch in your logic +(narrow it, as above), whereas an activity failure is normally something +Temporal's retry policy should already have handled by the time it reaches +the workflow (propagate it with `propagateActivityFailure`, unless the +workflow itself needs to branch on it too). See [The result model](/explanation/the-result-model). ## Start without waiting @@ -68,7 +71,9 @@ implementation: async (context, order) => { } // Do other work while the child runs. - const shipment = await context.activities.createShipment({ orderId: order.orderId }); + const shipment = await propagateActivityFailure( + context.activities.createShipment({ orderId: order.orderId }), + ); // Then collect. const receipt = await started.value.result(); diff --git a/docs/how-to/upgrade-to-v8.md b/docs/how-to/upgrade-to-v8.md index 75277bb2..15522904 100644 --- a/docs/how-to/upgrade-to-v8.md +++ b/docs/how-to/upgrade-to-v8.md @@ -689,14 +689,108 @@ result.match({ }); ``` -### Cancellation can be swallowed by declared-error activities +### Every activity call now returns `AsyncResult` — and a bare `await` still compiles + +Before 8.0, the call convention depended on whether the contract declared an +`errors` map: activities with declared errors returned `AsyncResult`, those +without returned a plain `Promise` that threw. In 8.0, every activity +call — declared errors or not — returns `AsyncResult`, and the +throwing wrapper is gone. + +::: danger This is the branch's most dangerous hazard, and the compiler will not catch it +`await context.activities.sendEmail(input);` compiles **identically** before +and after this change. Before 8.0, an un-awaited-for-its-result activity call +still threw on failure, so the workflow failed. After 8.0, that same line +discards the `AsyncResult` — the failure is silently swallowed and the +workflow proceeds as if the call succeeded. TypeScript gives no warning: +`AsyncResult` is a valid, `await`-able value either way, so nothing is +type-incorrect about writing this. The `cancellableScope` break covered below +_does_ fail to compile; this one does not, which is exactly what makes it +easy to miss during migration. +::: + +Audit every activity call site and choose one of two shapes: + +```ts +// Narrow it — the workflow branches on the outcome itself. +const result = await context.activities.sendEmail(input); +if (result.isErr()) { + /* ... */ +} + +// Or propagate it — let a failure escape and have Temporal decide the +// workflow's fate, matching the pre-8.0 "just let it throw" behavior. +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; + +await propagateActivityFailure(context.activities.sendEmail(input)); +``` + +**Do not reach for unthrown's `.getOrThrow()` instead of `propagateActivityFailure`.** +`.getOrThrow()` throws the `ActivityError`/`ActivityCancelledError` wrapper +itself — a `TaggedError`, not a `TemporalFailure`. Temporal treats a thrown +non-`TemporalFailure` as a workflow-_task_ failure and retries it +indefinitely, so the workflow never fails — it stalls until its execution +timeout instead. `propagateActivityFailure` re-raises the preserved original +failure instead, which is exactly what would have escaped the workflow +before this change. See [The result model](/explanation/the-result-model). + +A bare `await` that discards the result is easy to introduce by habit, +especially copying a pre-8.0 call site that never needed narrowing. Grep for +`await context.activities.` / `await activities.` (or your local alias) and +confirm each hit either narrows the result or passes it through +`propagateActivityFailure` — an un-narrowed, un-propagated `AsyncResult` sitting +in an expression statement is the tell. + +### Cancellation can be swallowed by any activity call + +Every activity call now returns an `AsyncResult` — declared `errors` map or +not — so cancelling an in-flight call surfaces as `Err(ActivityCancelledError)` +on every activity, not only ones that declare errors. That is a value a +generic "map every `Err` to a fallback" handler will absorb, completing the +workflow instead of cancelling it. Re-raise with the new +`rethrowCancellation(error)` from `@temporal-contract/worker/workflow`. See +[Handle cancellation](/how-to/handle-cancellation) and [The result +model](/explanation/the-result-model). + +### `cancellableScope`/`nonCancellableScope` wrapping an activity call: source break, not just behavior + +If a scope's callback returns an activity call directly, this stops +compiling — not just behaves differently: + +```ts +// ❌ no longer compiles +const scoped = await context.cancellableScope(() => context.activities.charge(input)); +if (scoped.isOk()) { + scoped.value.transactionId; // scoped.value is now an AsyncResult, not Output +} +``` + +Both scopes are generic over whatever `fn` returns, verbatim — they do not +await it for you. Before this change, for an activity with **no** declared +`errors` map, `() => context.activities.charge(input)` returned a plain +`Promise`, so the scope's own `T` was `Output` (an errors-declaring +activity already returned an `AsyncResult` and already had this problem). Now +every activity call returns an `AsyncResult`, and `AsyncResult` is +deliberately not a full `PromiseLike` (no `.catch`/`.finally`), so `T` becomes +the un-awaited `AsyncResult` itself — a type with no `isOk`/`isErr`/`.value`. +Await and narrow the activity call _inside_ the callback instead: + +```ts +// ✅ narrow inside the callback +const scoped = await context.cancellableScope(async () => { + const charged = await context.activities.charge(input); + if (charged.isDefect()) { + throw charged.cause; + } + if (charged.isErr()) { + return { ok: false as const, error: charged.error }; + } + return { ok: true as const, value: charged.value }; +}); +``` -When an activity declares an `errors` map, cancelling it surfaces as -`Err(ActivityCancelledError)` — a value a generic "map every `Err` to a -fallback" handler will absorb, completing the workflow instead of cancelling -it. Re-raise with the new `rethrowCancellation(error)` from -`@temporal-contract/worker/workflow`. See -[Handle cancellation](/how-to/handle-cancellation). +See [Handle cancellation](/how-to/handle-cancellation) for the full pattern, +including cleanup in a `nonCancellableScope`. ### Typed errors carry a wire marker — mind the deploy order @@ -813,8 +907,13 @@ contract-error wire round-trip) so a test fails exactly where production does. - [ ] `result()` / update / query matchers handle the new modeled errors (`WorkflowCancelledError` / `Terminated` / `Timeout`, `UpdateFailedError`, `UpdateRejectedError`, `QueryFailedError`) -- [ ] Cancellation isn't swallowed by a declared-error activity — - `rethrowCancellation` where a generic `Err` fallback would complete the run +- [ ] Every `await context.activities.x(...)` (declared-error or not) either + narrows the `AsyncResult` or is wrapped in `propagateActivityFailure` — + a bare, discarded `await` compiles identically before and after 8.0 but + now silently swallows the failure +- [ ] Cancellation isn't swallowed by **any** activity call (declared-error + or not) — `rethrowCancellation` where a generic `Err` fallback would + complete the run - [ ] Shared activities implemented once (same reference or hoisted global) - [ ] `createContractTest({ contract, ... })` and `runActivity(def, { ... })` use the option bag; `testcontainers` installed only where `createContractTest` runs diff --git a/docs/how-to/use-signals-queries-and-updates.md b/docs/how-to/use-signals-queries-and-updates.md index c2e897a2..e1ef7502 100644 --- a/docs/how-to/use-signals-queries-and-updates.md +++ b/docs/how-to/use-signals-queries-and-updates.md @@ -88,7 +88,7 @@ Register handlers **inside** the implementation so they can close over workflow state: ```typescript -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { condition } from "@temporalio/workflow"; export const importCatalog = declareWorkflow({ @@ -97,7 +97,9 @@ export const importCatalog = declareWorkflow({ activityOptions: { startToCloseTimeout: "5 minutes" }, implementation: async (context, args) => { let completed = 0; - let pending = await context.activities.listSkus({ catalogId: args.catalogId }); + let pending = await propagateActivityFailure( + context.activities.listSkus({ catalogId: args.catalogId }), + ); let cancelReason: string | undefined; // Query: synchronous, reads only. @@ -120,7 +122,7 @@ export const importCatalog = declareWorkflow({ while (pending.length > 0 && cancelReason === undefined) { const [next, ...rest] = pending; pending = rest; - await context.activities.importSku({ sku: next }); + await propagateActivityFailure(context.activities.importSku({ sku: next })); completed += 1; } diff --git a/docs/index.md b/docs/index.md index f2ef1800..c271c385 100644 --- a/docs/index.md +++ b/docs/index.md @@ -92,7 +92,7 @@ export const activities = declareActivitiesHandler({ ``` ```typescript [3. Workflow] -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { orderContract } from "./contract.js"; @@ -101,11 +101,15 @@ export const processOrder = declareWorkflow({ contract: orderContract, activityOptions: { startToCloseTimeout: "1 minute" }, implementation: async (context, order) => { - // `order` is typed from the contract. So is the return value. - const { transactionId } = await context.activities.chargeCard({ - customerId: order.customerId, - amount: order.amount, - }); + // `order` is typed from the contract. So is the return value. Every + // activity call returns an AsyncResult; `propagateActivityFailure` lets + // Temporal's retry policy decide the outcome. + const { transactionId } = await propagateActivityFailure( + context.activities.chargeCard({ + customerId: order.customerId, + amount: order.amount, + }), + ); return { orderId: order.orderId, transactionId }; }, diff --git a/docs/reference/errors.md b/docs/reference/errors.md index 5a1f6a36..0f08584f 100644 --- a/docs/reference/errors.md +++ b/docs/reference/errors.md @@ -274,26 +274,37 @@ An activity name has no definition on the contract. `_tag: "@temporal-contract/ActivityError"` · channel: `err` -An errors-declaring activity failed for a reason **other** than one of its -declared errors — retries exhausted, a timeout, an undeclared -`ApplicationFailure` type, or a boundary validation failure. - -| Property | Type | -| -------------- | ------------------------------------ | -| `activityName` | `string` | -| `cause` | the **unwrapped** actionable failure | - -::: warning Only for errors-declaring activities -An activity without an `errors` map keeps Temporal's native throwing behaviour -and never produces this. -::: +Any activity call failed for a reason **other** than one of its declared +errors — retries exhausted, a timeout, an undeclared `ApplicationFailure` +type, or a boundary validation failure. This is every activity's fallback: +one with no `errors` map has no declared-error members to fall through, so +every non-cancellation failure lands here. + +| Property | Type | +| ----------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `activityName` | `string` | +| `cause` | the **unwrapped** actionable failure | +| `originalFailure` | the failure exactly as caught, **before** the unwrap (typically Temporal's `ActivityFailure` wrapper) — `undefined` when there is no separate wrapper to retain | + +`originalFailure` exists so `propagateActivityFailure` can re-raise the exact +failure Temporal originally produced without changing what `cause` means for +existing consumers that narrow on it — see [Worker +surface](/reference/worker-surface#propagateactivityfailure-result). ### `ActivityCancelledError` `_tag: "@temporal-contract/ActivityCancelledError"` · channel: `err` -A call to an errors-declaring activity was cancelled. A sibling of -`ActivityError`, not a subclass, so call sites discriminate on the tag. +A call to an activity was cancelled — declared `errors` map or not. A sibling +of `ActivityError`, not a subclass, so call sites discriminate on the tag. + +::: warning Swallowing this changes the workflow outcome +Cancellation rides this modeled `Err(...)` channel, so generic handling that +folds every `Err` to a fallback value absorbs it — the workflow completes +`Completed` instead of `Cancelled`. Re-raise it with `rethrowCancellation` +when the workflow should honor the request. See [Handle +cancellation](/how-to/handle-cancellation). +::: | Property | Type | | -------------- | --------- | @@ -368,7 +379,7 @@ the defect channel instead. | Operation | `err` channel | | ------------------------------------------ | --------------------------------------------------------------------------------- | | `TypedWorker.create` / `TypedWorker.run` | `never` | -| activity call, no declared errors | n/a — `Promise`, throws on failure | +| activity call, no declared errors | `ActivityError \| ActivityCancelledError` | | activity call, declared errors | `ContractErrorUnion \| ActivityError \| ActivityCancelledError` | | `startChildWorkflow` | `ChildWorkflowError \| ChildWorkflowCancelledError \| ChildWorkflowNotFoundError` | | `executeChildWorkflow` | same | diff --git a/docs/reference/worker-surface.md b/docs/reference/worker-surface.md index 6b62207d..48a57b47 100644 --- a/docs/reference/worker-surface.md +++ b/docs/reference/worker-surface.md @@ -60,12 +60,19 @@ The first argument to `implementation`. `Readonly<...>` map of every activity reachable from this workflow — workflow-scoped plus global — flattened to one namespace. -Each returns a **plain value**, not a `Result`. Input is validated before the -call, output after. A failure throws. +Each returns an `AsyncResult>` — never a +plain value, and never a call that throws through. That is uniform across +every activity, declared `errors` map or not: `ActivityErrorsFor` +is `ActivityError | ActivityCancelledError`, plus the activity's declared +`ContractErrorUnion` when it has one. Input is validated before the call, +output after. See [The result model](/explanation/the-result-model) and +`propagateActivityFailure` below. The map's type is `WorkflowInferWorkflowContextActivities` and a single entry's is `WorkflowInferActivity` — both exported for annotating helpers that take `context.activities`. +`ActivityErrorsFor` — the error union in that `AsyncResult` — is +exported too, for helpers generic over an activity's error type. #### `info` @@ -230,18 +237,58 @@ Each `ValidationError` subclass carries a readonly `direction: "input" | "output"` field (the class names are unchanged; they remain `ApplicationFailure` subclasses discriminated by `failure.type`). +#### `propagateActivityFailure(result)` + +```typescript +function propagateActivityFailure(result: AsyncResult): Promise; +``` + +Await an activity call and return its value, re-raising the original Temporal +failure so **Temporal** decides the workflow's outcome — the explicit +equivalent of the pre-8.0 "just let it throw" call site: + +```typescript +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; + +const { transactionId } = await propagateActivityFailure( + context.activities.chargeCard({ customerId, amount }), +); +``` + +**Do not use unthrown's `.getOrThrow()` for this.** It throws the +`ActivityError`/`ActivityCancelledError` wrapper — a `TaggedError`, not a +`TemporalFailure` — which Temporal treats as a workflow-_task_ failure and +retries indefinitely, stalling the workflow until its execution timeout +instead of failing it. `propagateActivityFailure` re-raises the preserved +original failure instead — see [The result +model](/explanation/the-result-model). + +`E` is intentionally unconstrained, so this also accepts the `AsyncResult` +returned by `context.executeChildWorkflow` / `context.startChildWorkflow` +(`ChildWorkflowError`, `ChildWorkflowCancelledError`) and by +`context.cancellableScope` / `context.nonCancellableScope` +(`WorkflowCancelledError`) — each re-raises its preserved `cause` the same +way. `ChildWorkflowNotFoundError` (no Temporal call ever happened — the +target contract doesn't declare the child workflow name) is converted to a +`ContractMisuseError` instead, since there is no prior Temporal failure to +re-raise. + #### `rethrowCancellation(error): never` Re-raise a cancellation that surfaced on the modeled `Err(...)` channel. `WorkflowCancelledError` (from `cancellableScope`), `ChildWorkflowCancelledError`, and `ActivityCancelledError` are values — generic error handling that maps every `Err` to a fallback would **complete** the workflow as `Completed` instead of -letting it end `Cancelled`. Pass the error to `rethrowCancellation` to re-raise -the original `CancelledFailure`: +letting it end `Cancelled`. Its parameter type accepts only a cancellation +error — narrow to one first — and it never returns normally: ```typescript +import { ActivityCancelledError, rethrowCancellation } from "@temporal-contract/worker/workflow"; + if (result.isErr()) { - rethrowCancellation(result.error); // re-raises a cancellation; returns for anything else + if (result.error instanceof ActivityCancelledError) { + rethrowCancellation(result.error); // never returns — re-raises the cancellation + } return { status: "failed" }; } ``` diff --git a/docs/superpowers/plans/2026-08-04-uniform-activity-result.md b/docs/superpowers/plans/2026-08-04-uniform-activity-result.md new file mode 100644 index 00000000..1a406376 --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-uniform-activity-result.md @@ -0,0 +1,811 @@ +# Uniform `AsyncResult` for Activity Calls Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make every workflow-side activity call return `AsyncResult`, so the call convention no longer depends on whether the contract declared an `errors` map — without changing how Temporal classifies a propagated activity failure. + +**Architecture:** `WorkflowInferActivity` loses its conditional and always returns `AsyncResult`; `makeThrowingActivity` is deleted so every activity flows through `makeResultShapedActivity`. Because re-throwing the `Err` value would change Temporal's failure classification, the worker package gains a named helper, `propagateActivityFailure`, that rethrows the _preserved original cause_. A characterization test written **before** any production change pins today's behavior and becomes the regression oracle. + +**Tech Stack:** TypeScript 6.0.3, unthrown 5.0.0-beta.7 (`AsyncResult`), `@temporalio/*` 1.21.1, Vitest 4.1.10, `@temporal-contract/testing`'s `testRig` + time-skipping test server, pnpm workspaces + turbo, oxlint. + +**Spec:** `docs/superpowers/specs/2026-08-04-uniform-activity-result-design.md` + +## Global Constraints + +- **No `any`.** Use `unknown` and narrow. Enforced by oxlint. +- **`.js` extensions in every import.** `./workflow.js`, never `./workflow`. +- **ESM only.** All packages are `"type": "module"`. +- **Never edit per-package `package.json` dependency versions** — the `catalog:` block in `pnpm-workspace.yaml` is the only place versions are bumped. This plan adds no dependencies. +- **Assert effects, never call shapes.** The governing test rule from workstream 1. A test whose assertion is "we called Temporal with X" is not acceptable; assert what the workflow actually did on the real server. +- **Error classification behavior must not change.** `classifyActivityError`, contract-error rehydration, and the cancellation discriminant keep their current behavior. This plan changes the _call convention_, not what errors mean. +- **Rule 2 (`AGENTS.md`)** — activities return `AsyncResult`; never throw. This plan exists to make the library obey its own rule. +- Conventional Commits are enforced by commitlint on a git hook. Use `feat:`, `fix:`, `test:`, `docs:`, `refactor:`. + +--- + +## The hazard that shapes this whole plan + +`packages/worker/src/__tests__/retry.workflows.ts` carries this comment, discovered the hard way: + +> Fold the failure into a returned status rather than rethrowing: a rethrown defect becomes a Workflow-Task retry loop that time-skipping cannot fast-forward past, turning a regression into a 120s hang. + +Two consequences you must respect: + +1. **It confirms why `propagateActivityFailure` must exist.** Throwing a non-`TemporalFailure` from workflow code does not fail the workflow — it fails the _workflow task_, which Temporal retries indefinitely. `ActivityError` is a `TaggedError` (`packages/worker/src/errors.ts`, `export class ActivityError`), not a `TemporalFailure`. So `.getOrThrow()` is the **wrong** tool here and must never be recommended as the migration path. + +2. **A wrong implementation makes tests hang, not fail.** Every test that exercises the propagation path MUST set a short `workflowExecutionTimeout` so a task-retry loop terminates quickly and surfaces as a distinguishable failure instead of a 120-second stall. Use `"10 seconds"` for propagation tests. + +--- + +## Running the tests — two separate tiers + +This trips people up, and I verified it rather than assuming: `pnpm --filter @temporal-contract/worker test` runs **only** the `unit` project. It does **not** run the in-process integration tests, and passing a filename after `--` does not filter anything — you get the whole unit suite regardless. + +| What you want | Command | +| ------------------------- | --------------------------------------------------------------------------------------------------- | +| Unit tests, filtered | `pnpm --filter @temporal-contract/worker exec vitest run --project unit ` | +| In-process tier, filtered | `pnpm --filter @temporal-contract/worker exec vitest run --project integration-inprocess ` | +| Whole unit suite | `pnpm --filter @temporal-contract/worker test` | +| Whole in-process tier | `pnpm --filter @temporal-contract/worker exec vitest run --project integration-inprocess` | + +The in-process tier boots a Temporal test server per file (~10s each), so filter to the file you are working on while iterating. There is also a third project, `integration`, driven by `test:integration`, which uses Docker — **do not run the full `test:integration` in parallel with other work**, as concurrent container boots are a known source of flakes in this repo. + +--- + +## File Structure + +| File | Responsibility | +| ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | +| `packages/worker/src/__tests__/propagation.contract.ts` | **Create.** Contract fixture with one activity that declares NO errors and one that does. | +| `packages/worker/src/__tests__/propagation.workflows.ts` | **Create.** Workflow fixtures: propagate the failure, and handle it. | +| `packages/worker/src/__tests__/propagation.inprocess.spec.ts` | **Create.** The characterization test — pins workflow status and attempt count. Written against CURRENT behavior in Task 1, unchanged thereafter. | +| `packages/worker/src/activity-failure.ts` | **Create.** `propagateActivityFailure`. Kept out of `activities-proxy.ts`, which is already the package's densest module. | +| `packages/worker/src/activities-proxy.ts` | **Modify.** `WorkflowInferActivity` loses its conditional; `makeThrowingActivity` deleted. | +| `packages/worker/src/index.ts` | **Modify.** Export `propagateActivityFailure`. | +| `examples/order-processing-worker/src/**` | **Modify.** Migrate call sites. | +| `docs/**` | **Modify.** Migrate examples and promote the cancellation warning. | + +## Sequencing rationale + +Task 1 writes the characterization test **against unchanged production code**, so it passes on the current implementation and captures what Temporal does today. That makes it a genuine regression oracle rather than a test written to match whatever the new code happens to do — the failure mode this project has hit repeatedly. + +Task 2 adds the helper. Task 3 makes the change and must leave Task 1's test passing **unmodified**. Tasks 4-6 migrate consumers. + +--- + +### Task 1: Characterization test — pin today's behavior + +**Files:** + +- Create: `packages/worker/src/__tests__/propagation.contract.ts` +- Create: `packages/worker/src/__tests__/propagation.workflows.ts` +- Create: `packages/worker/src/__tests__/propagation.inprocess.spec.ts` + +**Interfaces:** + +- Consumes: nothing. +- Produces: `propagationContract` (a `ContractDefinition` with activities `alwaysFailsNoErrors` and `alwaysFailsWithErrors`), and workflows `propagatesFailure` / `handlesFailure`. + +**Critical:** this task changes NO production code. Its tests must pass against the current implementation. If a test does not pass, the characterization is wrong — fix the test, not the library. + +- [ ] **Step 1: Write the contract fixture** + +Create `packages/worker/src/__tests__/propagation.contract.ts`: + +```ts +import { defineActivity, defineContract, defineWorkflow } from "@temporal-contract/contract"; +import { z } from "zod"; + +/** + * Fails on every attempt and declares NO `errors` map — so today it takes the + * `makeThrowingActivity` path and Temporal's original `ActivityFailure` + * propagates out of the workflow. This is the activity whose behavior must be + * identical after the uniform-`AsyncResult` change. + * + * `maximumAttempts: 2` bounds the run: enough to prove Temporal retried, + * short enough that a regression cannot stall the suite. + */ +const alwaysFailsNoErrors = defineActivity({ + input: z.object({}), + output: z.object({ ok: z.boolean() }), + activityOptions: { + startToCloseTimeout: "5 seconds", + retry: { maximumAttempts: 2, backoffCoefficient: 1, initialInterval: "1 second" }, + }, +}); + +/** The same, but declaring an error — already on the Result path today. */ +const alwaysFailsWithErrors = defineActivity({ + input: z.object({}), + output: z.object({ ok: z.boolean() }), + errors: { + Boom: { data: z.object({ at: z.number() }), nonRetryable: true }, + }, + activityOptions: { + startToCloseTimeout: "5 seconds", + retry: { maximumAttempts: 2, backoffCoefficient: 1, initialInterval: "1 second" }, + }, +}); + +/** Lets the activity failure escape, so Temporal decides the workflow outcome. */ +const propagatesFailure = defineWorkflow({ + input: z.object({}), + output: z.object({ reached: z.boolean() }), + activities: { alwaysFailsNoErrors }, +}); + +/** Catches the failure and returns normally, so the workflow completes. */ +const handlesFailure = defineWorkflow({ + input: z.object({}), + output: z.object({ outcome: z.string() }), + activities: { alwaysFailsNoErrors }, +}); + +export const propagationContract = defineContract({ + taskQueue: "propagation-tests", + workflows: { propagatesFailure, handlesFailure }, + activities: { alwaysFailsWithErrors }, +}); +``` + +- [ ] **Step 2: Write the workflow fixtures against CURRENT behavior** + +Create `packages/worker/src/__tests__/propagation.workflows.ts`. Note `alwaysFailsNoErrors` declares no errors, so **today** it returns a `Promise` and throws — that is what these fixtures must be written against: + +```ts +import { declareWorkflow } from "../workflow.js"; +import { propagationContract } from "./propagation.contract.js"; + +/** + * Awaits the activity without catching. Today `alwaysFailsNoErrors` has no + * declared errors, so this is a plain `Promise` that throws Temporal's + * `ActivityFailure`, and Temporal fails the workflow. + * + * After the uniform-`AsyncResult` change this body becomes + * `await propagateActivityFailure(context.activities.alwaysFailsNoErrors({}))` + * and the observable outcome must be IDENTICAL. + */ +export const propagatesFailure = declareWorkflow({ + workflowName: "propagatesFailure", + contract: propagationContract, + implementation: async (context) => { + await context.activities.alwaysFailsNoErrors({}); + return { reached: true }; + }, +}); + +/** + * Catches the failure so the workflow COMPLETES. Uses try/catch because today + * the call throws; after the change this becomes an `isErr()` narrow. + */ +export const handlesFailure = declareWorkflow({ + workflowName: "handlesFailure", + contract: propagationContract, + implementation: async (context) => { + try { + await context.activities.alwaysFailsNoErrors({}); + return { outcome: "unexpected-success" }; + } catch { + return { outcome: "handled" }; + } + }, +}); +``` + +- [ ] **Step 3: Write the characterization spec** + +Create `packages/worker/src/__tests__/propagation.inprocess.spec.ts`: + +```ts +import { testRig } from "@temporal-contract/testing/test-rig"; +import { it } from "@temporal-contract/testing/time-skipping"; +import { + bundleFor, + fixturePath, + nextTaskQueueId, + withTaskQueue, +} from "@temporal-contract/testing/workflow-bundle"; +import { Context } from "@temporalio/activity"; +import { describe, expect } from "vitest"; + +import { declareActivitiesHandler } from "../activity.js"; +import { propagationContract } from "./propagation.contract.js"; + +// Short, so a workflow-TASK retry loop (the failure mode a wrong propagation +// helper produces) times out fast instead of stalling the suite for 120s. +const WORKFLOW_EXECUTION_TIMEOUT = "10 seconds"; + +/** + * These tests are a CHARACTERIZATION of behavior that must survive the + * uniform-`AsyncResult` change. They are written against the pre-change + * implementation and must pass unmodified afterwards. Do not adjust them to + * match new behavior — a diff here means the change altered semantics. + */ +describe("activity failure propagation — characterization", () => { + it("fails the workflow, after Temporal's own retries, when the failure escapes", async ({ + testEnv, + }) => { + const contract = withTaskQueue(propagationContract, nextTaskQueueId("prop-escape")); + const bundle = await bundleFor(fixturePath(import.meta.url, "propagation.workflows")); + + const attempts: number[] = []; + const activities = declareActivitiesHandler({ + contract, + activities: { + propagatesFailure: { + alwaysFailsNoErrors: async () => { + attempts.push(Context.current().info.attempt); + // oxlint-disable-next-line unthrown/no-throw -- the activity under test must fail + throw new Error("activity exploded"); + }, + }, + handlesFailure: { + alwaysFailsNoErrors: async () => { + // oxlint-disable-next-line unthrown/no-throw -- the activity under test must fail + throw new Error("activity exploded"); + }, + }, + }, + }); + + const { worker, client } = await testRig(testEnv, { contract, bundle, activities }); + + const outcome = await worker.raw.runUntil( + (async () => { + const result = await client.executeWorkflow("propagatesFailure", { + workflowId: "prop-escape", + args: {}, + workflowExecutionTimeout: WORKFLOW_EXECUTION_TIMEOUT, + }); + return result.isErr() ? "failed" : "completed"; + })(), + ); + + // The workflow must FAIL — not complete, and not spin in task retries + // until the execution timeout. And Temporal must have retried the + // activity to its configured maximum, proving the retry policy reached + // the server rather than being short-circuited client-side. + expect(outcome).toBe("failed"); + expect(attempts).toEqual([1, 2]); + }); + + it("completes the workflow when the failure is caught", async ({ testEnv }) => { + const contract = withTaskQueue(propagationContract, nextTaskQueueId("prop-handled")); + const bundle = await bundleFor(fixturePath(import.meta.url, "propagation.workflows")); + + const activities = declareActivitiesHandler({ + contract, + activities: { + propagatesFailure: { + alwaysFailsNoErrors: async () => { + // oxlint-disable-next-line unthrown/no-throw -- the activity under test must fail + throw new Error("activity exploded"); + }, + }, + handlesFailure: { + alwaysFailsNoErrors: async () => { + // oxlint-disable-next-line unthrown/no-throw -- the activity under test must fail + throw new Error("activity exploded"); + }, + }, + }, + }); + + const { worker, client } = await testRig(testEnv, { contract, bundle, activities }); + + const result = await worker.raw.runUntil( + client + .executeWorkflow("handlesFailure", { + workflowId: "prop-handled", + args: {}, + workflowExecutionTimeout: WORKFLOW_EXECUTION_TIMEOUT, + }) + .getOrThrow(), + ); + + expect(result).toEqual({ outcome: "handled" }); + }); +}); +``` + +- [ ] **Step 4: Run the characterization suite against UNCHANGED production code** + +```bash +pnpm --filter @temporal-contract/worker exec vitest run --project integration-inprocess propagation +``` + +Expected: **both tests PASS.** They describe how the library behaves today. + +If either fails, your fixture is wrong — the library is not under test yet. Do not change production code to make them pass. Report what you observed, including the actual `attempts` array and outcome, because a mismatch here means the plan's premise about current behavior is wrong and later tasks need rethinking. + +- [ ] **Step 5: Commit** + +```bash +pnpm --filter @temporal-contract/worker typecheck +pnpm lint +git add packages/worker/src/__tests__/propagation.* +git commit -m "test(worker): characterize activity-failure propagation before the Result change" +``` + +--- + +### Task 2: `propagateActivityFailure` + +**Files:** + +- Create: `packages/worker/src/activity-failure.ts` +- Create: `packages/worker/src/activity-failure.spec.ts` +- Modify: `packages/worker/src/index.ts` + +**Interfaces:** + +- Consumes: `ActivityError` and `ActivityCancelledError` from `./errors.js`. +- Produces: + `export function propagateActivityFailure(result: AsyncResult): Promise` + +**Why this exists.** `ActivityError` is a `TaggedError`, not a `TemporalFailure`. Throwing it from workflow code produces a workflow-_task_ failure that Temporal retries indefinitely, instead of failing the workflow. `classifyActivityError` already preserves the unwrapped original failure on `ActivityError`'s `cause`, so rethrowing **that** reproduces today's semantics exactly. + +- [ ] **Step 1: Write the failing unit test** + +Create `packages/worker/src/activity-failure.spec.ts`: + +```ts +import { ApplicationFailure } from "@temporalio/common"; +import { ErrAsync, OkAsync } from "unthrown"; +import { describe, expect, it } from "vitest"; + +import { propagateActivityFailure } from "./activity-failure.js"; +import { ActivityCancelledError, ActivityError } from "./errors.js"; + +describe("propagateActivityFailure", () => { + it("returns the value on Ok", async () => { + await expect(propagateActivityFailure(OkAsync({ ok: true }))).resolves.toEqual({ ok: true }); + }); + + it("rethrows the PRESERVED CAUSE, not the ActivityError wrapper", async () => { + // This is the whole point. Throwing the wrapper would make Temporal treat + // the failure as a workflow-task failure and retry forever. + const cause = ApplicationFailure.create({ message: "boom", type: "Boom" }); + const wrapper = new ActivityError("charge", 'Activity "charge" failed: boom', cause); + + await expect(propagateActivityFailure(ErrAsync(wrapper))).rejects.toBe(cause); + }); + + it("rethrows the wrapper itself when no cause was preserved", async () => { + // Never lose the error identity: if there is nothing underneath, the + // wrapper is the most informative thing available. + const wrapper = new ActivityError("charge", 'Activity "charge" failed: opaque'); + + await expect(propagateActivityFailure(ErrAsync(wrapper))).rejects.toBe(wrapper); + }); + + it("rethrows the preserved cause for a cancelled activity", async () => { + const cause = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); + const cancelled = new ActivityCancelledError("charge", cause); + + await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cause); + }); + + it("rethrows a non-ActivityError error value unchanged", async () => { + const other = new Error("something else"); + await expect(propagateActivityFailure(ErrAsync(other))).rejects.toBe(other); + }); +}); +``` + +- [ ] **Step 2: Run it and verify it fails** + +```bash +pnpm --filter @temporal-contract/worker exec vitest run --project unit activity-failure +``` + +Expected: failure reporting `Cannot find module './activity-failure.js'`. + +- [ ] **Step 3: Implement** + +Create `packages/worker/src/activity-failure.ts`: + +```ts +import type { AsyncResult } from "unthrown"; + +import { ActivityCancelledError, ActivityError } from "./errors.js"; + +/** + * Await an activity call and return its value, re-raising the failure so + * **Temporal** decides the workflow's fate — the workflow-side equivalent of + * "let it fail". + * + * Use this instead of unthrown's `.getOrThrow()`. `getOrThrow` throws the + * `ActivityError` *wrapper*, which is a `TaggedError` and NOT a + * `TemporalFailure`. Temporal treats a non-`TemporalFailure` thrown from + * workflow code as a workflow-TASK failure and retries it indefinitely, so + * the workflow never fails — it stalls until its execution timeout. This + * helper rethrows the preserved original failure instead, which is exactly + * what escaped the workflow before activity calls returned `AsyncResult`. + * + * A failure with no preserved cause rethrows the wrapper, so the error + * identity is never lost. + */ +export async function propagateActivityFailure(result: AsyncResult): Promise { + const settled = await result; + if (settled.isOk()) { + return settled.value; + } + const error: unknown = settled.isErr() ? settled.error : settled.cause; + if (error instanceof ActivityError || error instanceof ActivityCancelledError) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: Temporal must see the original failure to classify the workflow outcome + throw error.cause ?? error; + } + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise, see above + throw error; +} +``` + +**If `ActivityCancelledError` does not expose `cause`**, read `packages/worker/src/errors.ts` and adapt — the requirement is "rethrow the preserved original failure, else the wrapper", not this exact property access. Report any adaptation. + +- [ ] **Step 4: Run the tests** + +```bash +pnpm --filter @temporal-contract/worker exec vitest run --project unit activity-failure +``` + +Expected: all 5 pass. + +- [ ] **Step 5: Export it** + +In `packages/worker/src/index.ts`, add `propagateActivityFailure` to the public exports, following the file's existing export style. + +- [ ] **Step 6: Commit** + +```bash +pnpm --filter @temporal-contract/worker typecheck +pnpm lint +git add packages/worker/src/activity-failure.ts packages/worker/src/activity-failure.spec.ts packages/worker/src/index.ts +git commit -m "feat(worker): add propagateActivityFailure for Temporal-faithful re-raising" +``` + +--- + +### Task 3: Uniform `AsyncResult`, and prove the characterization still holds + +**Files:** + +- Modify: `packages/worker/src/activities-proxy.ts` +- Modify: `packages/worker/src/__tests__/propagation.workflows.ts` + +**Interfaces:** + +- Consumes: `propagateActivityFailure` from Task 2. +- Produces: `WorkflowInferActivity` always returning `AsyncResult`. + +- [ ] **Step 1: Make the type unconditional** + +In `packages/worker/src/activities-proxy.ts`, replace `WorkflowInferActivity` (the `TActivity extends { errors: ... } ? ... : ...` conditional) with: + +```ts +/** + * The error channel for an activity call: declared contract errors when the + * activity declares an `errors` map, plus the two failures every activity can + * produce. + */ +export type ActivityErrorsFor = TActivity extends { + errors: infer TErrors extends Record; +} + ? ContractErrorUnion | ActivityError | ActivityCancelledError + : ActivityError | ActivityCancelledError; + +/** + * Every activity call returns an `AsyncResult` — the call convention no + * longer depends on whether the contract declared errors, only the error + * channel does. To let a failure escape and have Temporal decide the + * workflow's outcome, use `propagateActivityFailure` rather than + * unthrown's `.getOrThrow()`; see that function's documentation for why. + */ +export type WorkflowInferActivity = ( + args: ClientInferInput, +) => AsyncResult, ActivityErrorsFor>; +``` + +Keep the existing prose about rehydration and cancellation from the old doc comment — move it onto `ActivityErrorsFor` rather than deleting it. + +- [ ] **Step 2: Delete the throwing path** + +In the same file, change the wrapper selection so every activity uses the Result-shaped wrapper: + +```ts +(validatedActivities as Record)[activityName] = makeResultShapedActivity( + activityName, + activityDef, + rawActivity, +); +``` + +Then **delete the `makeThrowingActivity` function entirely.** Leaving it unreferenced would trip `knip`, which CI runs. + +- [ ] **Step 3: Update the characterization workflow fixtures** + +`packages/worker/src/__tests__/propagation.workflows.ts` — the activity now returns `AsyncResult`, so the fixtures must be rewritten. **The spec file must NOT change.** Replace the implementations with: + +```ts +import { propagateActivityFailure } from "../activity-failure.js"; +import { declareWorkflow } from "../workflow.js"; +import { propagationContract } from "./propagation.contract.js"; + +/** + * Lets the failure escape via `propagateActivityFailure`, the post-change + * equivalent of the bare `await` this fixture used before. The + * characterization spec asserts the workflow still FAILS and that Temporal + * still retried the activity to its configured maximum. + */ +export const propagatesFailure = declareWorkflow({ + workflowName: "propagatesFailure", + contract: propagationContract, + implementation: async (context) => { + await propagateActivityFailure(context.activities.alwaysFailsNoErrors({})); + return { reached: true }; + }, +}); + +/** Handles the failure by narrowing, so the workflow completes. */ +export const handlesFailure = declareWorkflow({ + workflowName: "handlesFailure", + contract: propagationContract, + implementation: async (context) => { + const result = await context.activities.alwaysFailsNoErrors({}); + if (result.isErr() || result.isDefect()) { + return { outcome: "handled" }; + } + return { outcome: "unexpected-success" }; + }, +}); +``` + +- [ ] **Step 4: Run the characterization suite — the central gate of this plan** + +```bash +pnpm --filter @temporal-contract/worker exec vitest run --project integration-inprocess propagation +``` + +Expected: **both tests pass, with the spec file byte-identical to Task 1.** + +Confirm with `git diff --stat packages/worker/src/__tests__/propagation.inprocess.spec.ts` — it must show **no changes**. If you needed to touch the spec to make it pass, the change altered observable semantics: stop and report, rather than adjusting the assertions. + +If the first test hangs rather than failing, that is the workflow-task retry loop described at the top of this plan — it means the propagation path is throwing a non-`TemporalFailure`. Report it; do not raise the timeout to mask it. + +- [ ] **Step 5: Run the whole worker suite** + +```bash +pnpm --filter @temporal-contract/worker test +``` + +Many existing tests will now fail to compile or assert wrongly — activities without declared errors return `AsyncResult` now. **Do not fix them in this task**; inventory them (file, test name, symptom) in your report. Task 4 owns them. If the suite cannot even be collected because of type errors, note that and proceed to commit only the files this task owns. + +- [ ] **Step 6: Commit** + +```bash +git add packages/worker/src/activities-proxy.ts packages/worker/src/__tests__/propagation.workflows.ts +git commit -m "feat(worker)!: return AsyncResult from every activity call" +``` + +--- + +### Task 4: Migrate the worker package's own call sites + +**Files:** + +- Modify: whichever files Task 3's inventory named (expect `packages/worker/src/__tests__/*.ts` and `packages/worker/src/*.spec.ts`) + +**Interfaces:** + +- Consumes: the uniform `WorkflowInferActivity` and `propagateActivityFailure`. +- Produces: a green `@temporal-contract/worker` suite. + +**Framing.** Roughly 71 `activities.` call sites exist in this package. Each needs one of two treatments, and choosing wrongly silently changes what the test proves: + +- The workflow **should fail** on activity failure → `await propagateActivityFailure(context.activities.x({}))`. +- The workflow **should handle** the failure → narrow with `result.isErr()` / `result.isDefect()`. + +**Do not** reach for `.getOrThrow()`. It throws the wrapper and produces the task-retry stall. + +**Do not** convert an assertion that previously proved "the workflow failed" into one that proves "the workflow returned an error string" — that is a weaker test. If a fixture folded a failure into a returned status deliberately (as `retry.workflows.ts` does, and says so in a comment), preserve that intent. + +- [ ] **Step 1: Inventory** + +```bash +pnpm --filter @temporal-contract/worker test 2>&1 | tee /tmp/worker-unit.txt +pnpm --filter @temporal-contract/worker exec vitest run --project integration-inprocess 2>&1 | tee /tmp/worker-inprocess.txt +``` + +Record every failing file and test. Write the list into your report **before** changing anything — it is the checklist you will verify against at the end. + +- [ ] **Step 2: Migrate, file by file** + +For each file, apply the two-way choice above. Read the surrounding comments first: several fixtures document _why_ they fold or rethrow, and those reasons still hold. + +- [ ] **Step 3: Verify the suite is green** + +```bash +pnpm --filter @temporal-contract/worker test +pnpm --filter @temporal-contract/worker exec vitest run --project integration-inprocess +pnpm --filter @temporal-contract/worker typecheck +``` + +Expected: all pass. + +- [ ] **Step 4: Verify no test was weakened** + +For every test you touched, confirm in your report that it still asserts the same _effect_ it asserted before — same workflow status, same attempt counts, same terminal state. A test that now passes for a different reason is a silent regression, and this project has shipped that exact defect before. + +- [ ] **Step 5: Commit** + +```bash +pnpm lint +git add packages/worker/src +git commit -m "test(worker): migrate activity call sites to the uniform Result convention" +``` + +--- + +### Task 5: Migrate the examples + +**Files:** + +- Modify: `examples/order-processing-worker/src/**` (11 `activities.` call sites, 7 `defineActivity` declarations across the example packages) + +**Interfaces:** + +- Consumes: the uniform convention and `propagateActivityFailure`. +- Produces: examples that typecheck and demonstrate the intended idiom. + +**Framing.** The examples are the library's most-read documentation. Migrating them mechanically to `propagateActivityFailure` everywhere would teach the wrong lesson: the _point_ of this change is that failures are visible and handled. Prefer narrowing where the example has a meaningful failure path, and use `propagateActivityFailure` only where "let Temporal fail the workflow" is genuinely the intent. + +- [ ] **Step 1: Find every call site** + +```bash +grep -rn 'activities\.' examples/*/src --include='*.ts' +``` + +- [ ] **Step 2: Migrate each, choosing narrow-vs-propagate deliberately** + +For each, decide which reads better as teaching material and note the choice in your report. + +- [ ] **Step 3: Verify** + +```bash +pnpm turbo run typecheck --filter='./examples/*' +``` + +Expected: green. + +- [ ] **Step 4: Commit** + +```bash +pnpm lint +git add examples +git commit -m "docs(examples): adopt the uniform activity Result convention" +``` + +--- + +### Task 6: Documentation, cancellation warning, and changeset + +**Files:** + +- Modify: documentation under `docs/` referencing `activities.` (~107 files reference it; far fewer contain call-site code) +- Modify: `packages/worker/src/errors.ts` (promote the cancellation warning) +- Create: `.changeset/uniform-activity-result.md` + +- [ ] **Step 1: Find docs with real call sites** + +```bash +grep -rln 'await context\.activities\.\|await ctx\.activities\.' docs/ +``` + +That narrower pattern finds code, not prose. Record the count in your report; update each. + +- [ ] **Step 2: Promote the cancellation warning** + +`packages/worker/src/errors.ts` documents on `ActivityCancelledError` that swallowing it makes a workflow complete as `Completed` instead of `Cancelled`. Its text currently scopes the hazard to errors-declaring activities — for example `ActivityError`'s comment says _"Only activities that declare an `errors` map surface this — activities without declared errors keep Temporal's native throwing behavior."_ **That sentence is now false.** Find every such statement in that file and correct it: the hazard applies to every activity now. + +Verify with: + +```bash +grep -n 'declare an \`errors\` map\|without declared errors' packages/worker/src/errors.ts +``` + +Every hit must be re-read and corrected if it scopes behavior to errors-declaring activities. + +- [ ] **Step 3: Write the changeset** + +Create `.changeset/uniform-activity-result.md`: + +````markdown +--- +"@temporal-contract/worker": major +--- + +Every workflow-side activity call now returns `AsyncResult`. + +Previously the call convention depended on whether the contract declared an +`errors` map: activities with declared errors returned `AsyncResult`, those +without returned a `Promise` that threw. The call site gave no indication +which applied, and the throwing path contradicted the library's own +"activities never throw" convention. + +**Migrating.** Where the workflow should handle the failure, narrow it: + +```ts +const result = await context.activities.charge(input); +if (result.isErr()) { + /* ... */ +} +``` +```` + +Where the failure should escape and let Temporal fail the workflow, use the +new `propagateActivityFailure` helper: + +```ts +import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; + +await propagateActivityFailure(context.activities.charge(input)); +``` + +**Do not use unthrown's `.getOrThrow()` for this.** It throws the +`ActivityError` wrapper, which is not a `TemporalFailure`; Temporal treats +that as a workflow-_task_ failure and retries it indefinitely, so the +workflow stalls until its execution timeout instead of failing. +`propagateActivityFailure` rethrows the preserved original failure, which is +exactly what escaped the workflow before this change. + +**Also note:** swallowing `ActivityCancelledError` makes a workflow complete +as `Completed` rather than `Cancelled`. That hazard previously applied only +to activities declaring an `errors` map; it now applies to every activity. + +```` + +- [ ] **Step 4: Full verification** + +```bash +pnpm turbo run typecheck +pnpm turbo run test +pnpm lint +```` + +Expected: all green. + +- [ ] **Step 5: Commit** + +```bash +git add docs packages/worker/src/errors.ts .changeset +git commit -m "docs: migrate activity call sites and document the uniform Result convention" +``` + +--- + +## Self-Review + +**1. Spec coverage.** + +| Spec requirement | Task | +| ----------------------------------------------------------------- | ---------------------------------------------------------------------- | +| `WorkflowInferActivity` unconditional | Task 3 Step 1 | +| `makeThrowingActivity` deleted | Task 3 Step 2 | +| Named propagation helper in the public API | Task 2 | +| Helper rethrows preserved cause, not the wrapper | Task 2 Steps 1 and 3 | +| Equivalence proven by effect: workflow status AND attempt count | Task 1 (written pre-change) + Task 3 Step 4 (unchanged spec must pass) | +| Handled case narrows to `ActivityError \| ActivityCancelledError` | Task 1 second test, Task 3 Step 3 | +| Cancellation hazard documented for all activities | Task 6 Step 2, changeset | +| Examples and docs updated, repo typecheck green | Tasks 5 and 6 | +| Changeset records the breaking change and migration | Task 6 Step 3 | +| Error classification unchanged | Global Constraints; no task touches `classifyActivityError` | + +No gaps. + +**2. Placeholder scan.** No TBDs. Tasks 4-6 are migrations whose exact edits cannot be enumerated in advance — the set of affected call sites is only knowable after Task 3 lands — so they specify the decision procedure (narrow vs propagate), the prohibition (`.getOrThrow()`), and the anti-weakening check, rather than a literal diff. + +**3. Type consistency.** `propagateActivityFailure`, `ActivityErrorsFor`, `WorkflowInferActivity`, `propagationContract`, `propagatesFailure`, `handlesFailure`, `alwaysFailsNoErrors`, `alwaysFailsWithErrors` are each named identically at definition and every use. + +**One risk the plan cannot eliminate:** Task 1 asserts `attempts` via an array captured in the _test process_, which works because `testRig` runs the activity worker in-process. If a future change moves activity execution out of process, that assertion silently stops observing retries. The `expect(attempts).toEqual([1, 2])` form at least fails loudly if the array is empty, rather than passing vacuously — but it is worth knowing. + +**A second, more likely risk:** `alwaysFailsWithErrors` is declared on the contract but exercised by no test in Task 1. It exists so Task 3 can confirm the errors-declaring path is unaffected. If Task 3 does not use it, `knip` may flag it — that is a signal the coverage is missing, not a reason to delete the fixture. diff --git a/docs/superpowers/specs/2026-08-04-uniform-activity-result-design.md b/docs/superpowers/specs/2026-08-04-uniform-activity-result-design.md new file mode 100644 index 00000000..aba3b284 --- /dev/null +++ b/docs/superpowers/specs/2026-08-04-uniform-activity-result-design.md @@ -0,0 +1,182 @@ +# Uniform `AsyncResult` for activity calls + +**Date:** 2026-08-04 +**Status:** Approved +**Scope:** Workstream 4 of the production-hardening effort, part 1 of 3 + +## Context + +`temporal-contract` is in production use where real money depends on it. The +hardening driver is preventive — no incident has occurred. + +Workstreams 1-3 shipped: mock-free test architecture (PR #359), determinism and +money-safety invariants (PR #360), compile-time contract validation (PR #369). +Workstream 4 is **pattern enforcement** — API shapes that force correct usage — +and splits into three independent pieces: + +1. **The bimodal activity proxy** (this spec) +2. Idempotency / deduplication guidance — `workflowIdReusePolicy` and + `workflowIdConflictPolicy` are currently unguided passthrough +3. Safe-by-default option shapes — appears partly satisfied already; + `internal.ts` already requires `activityOptions` once any reachable activity + lacks its own + +Each gets its own spec → plan → implementation cycle. + +## The problem + +`WorkflowInferActivity` (`packages/worker/src/activities-proxy.ts:51-60`) +switches the workflow-side call contract on whether the contract declared an +`errors` map: + +```ts +export type WorkflowInferActivity = TActivity extends { + errors: infer TErrors extends Record; +} + ? ( + args: ClientInferInput, + ) => AsyncResult< + ClientInferOutput, + ContractErrorUnion | ActivityError | ActivityCancelledError + > + : (args: ClientInferInput) => Promise>; +``` + +with the runtime counterpart at `activities-proxy.ts:141-143` choosing +`makeResultShapedActivity` or `makeThrowingActivity`. + +Two consequences: + +- **The call site cannot tell you which contract applies.** `await +context.activities.charge(x)` is either a value that throws on failure or an + `AsyncResult` that never throws, decided by a declaration in a different file. +- **The throwing branch violates the project's own rule 2** (`AGENTS.md`): + _"Activities and the typed client return `AsyncResult` from unthrown. + Never throw."_ + +This is not a typing bug — the types are accurate. It is a predictability +defect, and it is not hypothetical: it caught the library's own author during +workstream 2 planning, where sample code narrowed the call as an `AsyncResult` +against an activity that declared no `errors` map and would have thrown a +`TypeError` at runtime. + +## The design + +### Uniform return shape + +`WorkflowInferActivity` loses its conditional. Every activity returns an +`AsyncResult`: + +```ts +export type WorkflowInferActivity = ( + args: ClientInferInput, +) => AsyncResult, ActivityErrorsFor>; +``` + +where `ActivityErrorsFor` is +`ContractErrorUnion | ActivityError | ActivityCancelledError` when the +activity declares errors, and `ActivityError | ActivityCancelledError` when it +does not. The error _channel_ still varies with the contract — that is correct +and useful — but the _call convention_ no longer does. + +`makeThrowingActivity` is deleted. Every activity flows through +`makeResultShapedActivity`. + +### Propagation must preserve Temporal's failure semantics + +**This is the highest-risk part of the change and the reason it is not a +mechanical refactor.** + +Today, an activity without declared errors lets Temporal's original +`ActivityFailure` propagate out of the workflow. `ActivityFailure` and +`ApplicationFailure` both extend `TemporalFailure` +(`@temporalio/common/lib/failure.d.ts:71,108,219`), and Temporal's handling of a +workflow-code exception depends on that lineage. + +`ActivityError` is **not** a `TemporalFailure` — it is a `TaggedError` +(`packages/worker/src/errors.ts:322`). So a caller who "just rethrows" the +`Err` value is not reproducing today's behavior, and unthrown's own +`.getOrThrow()` is the wrong tool: it throws the `ActivityError` wrapper. + +`classifyActivityError` (`activities-proxy.ts`) already preserves the unwrapped +inner failure as `ActivityError`'s `cause`. The library therefore provides an +explicit propagation helper that rethrows **that preserved cause**, so Temporal +observes exactly the failure it observes today. + +Naming and exact signature are an implementation decision for the plan, but the +helper must be **named and documented in the public API** rather than left to +callers to reconstruct — reconstructing it wrongly is a silent change in +workflow failure classification. + +**The behavioral claim must be proven, not asserted.** The plan must +demonstrate by effect, on the real time-skipping test server, that for an +activity without declared errors the **workflow status and attempt count are +identical before and after this change** — both when the failure propagates and +when it is handled. Asserting merely that "an error was thrown" would not catch +a reclassification from workflow-failure to workflow-task-failure, which is the +exact defect this section exists to prevent. + +This mirrors workstream 2's governing lesson: restoring a property is not +proving a behavior. + +### Cancellation keeps its existing warning + +`errors.ts:343-347` already documents that swallowing `ActivityCancelledError` +makes a workflow complete as `Completed` instead of `Cancelled`. That hazard now +applies to **every** activity rather than only errors-declaring ones, so the +warning must be surfaced correspondingly more prominently in the docs. + +## Blast radius + +Measured on the current branch: + +| Surface | Count | +| ---------------------------------------------- | ------------------ | +| `defineActivity` declarations in `examples/` | 7 | +| `activities.` call sites in `examples/` | 11 | +| `activities.` call sites in `packages/worker/` | ~71 (mostly tests) | +| Doc files referencing `activities.` | ~107 | + +Documentation dominates. The core library change is small; the migration is +wide. + +## What is NOT changing + +- The client-side API. This spec covers the **workflow-side** activity proxy + only. +- Error classification itself — `classifyActivityError`, contract-error + rehydration, and the cancellation discriminant all keep their current + behavior. +- Input/output validation, and the validate-on-send / parse-on-receive wire + contract. +- Any runtime validation in `defineContract`. + +## Risks + +| Risk | Mitigation | +| ---------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Rethrowing the wrong error changes Temporal's failure classification — a workflow that should fail terminally instead retries forever, or vice versa | The propagation helper rethrows the preserved original cause; proven by effect against workflow status and attempt count on the real test server, before/after | +| Callers swallow `ActivityCancelledError` now that every activity returns a Result, turning `Cancelled` into `Completed` | Existing warning promoted and applied to all activities in docs; covered by an effect-based test asserting the workflow's terminal status | +| Migration ceremony on activities that never fail meaningfully | Accepted. The uniform convention is the point; the named propagation helper keeps the one-line case one line | +| ~107 doc files to update, with drift risk | Docs are part of the deliverable, not a follow-up. Examples must typecheck, which the repo already enforces via `turbo run typecheck` | + +## Success criteria + +1. `WorkflowInferActivity` has no conditional; every activity call returns + `AsyncResult`. +2. `makeThrowingActivity` no longer exists. +3. An activity **without** declared errors, whose failure is propagated via the + library's helper, produces the **same workflow status and attempt count** as + before this change — proven on the real time-skipping server. +4. An activity without declared errors whose failure is _handled_ narrows + correctly to `ActivityError | ActivityCancelledError`. +5. Cancellation still yields a `Cancelled` workflow when re-raised, and the + swallow-hazard is documented for all activities. +6. Examples and docs updated; `pnpm turbo run typecheck` green repo-wide. +7. A changeset records the breaking change and the migration. + +## Out of scope + +- Idempotency / dedup guidance (workstream 4, part 2). +- Safe-by-default option shapes (workstream 4, part 3). +- The client-side typed API. diff --git a/docs/tutorial/adding-signals-and-queries.md b/docs/tutorial/adding-signals-and-queries.md index a7f5f81c..756827db 100644 --- a/docs/tutorial/adding-signals-and-queries.md +++ b/docs/tutorial/adding-signals-and-queries.md @@ -110,7 +110,7 @@ Edit `src/workflows.ts`. The workflow now holds mutable state, registers three handlers, and waits for approval before charging: ```typescript -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { condition } from "@temporalio/workflow"; import { orderContract } from "./contract.js"; @@ -146,15 +146,16 @@ export const processOrder = declareWorkflow({ state = "charging"; - const { transactionId } = await context.activities.chargeCard({ - customerId: order.customerId, - amount, - }); + const { transactionId } = await propagateActivityFailure( + context.activities.chargeCard({ + customerId: order.customerId, + amount, + }), + ); - await context.activities.sendReceipt({ - customerId: order.customerId, - transactionId, - }); + await propagateActivityFailure( + context.activities.sendReceipt({ customerId: order.customerId, transactionId }), + ); state = "done"; diff --git a/docs/tutorial/your-first-workflow.md b/docs/tutorial/your-first-workflow.md index 91be5114..8c738747 100644 --- a/docs/tutorial/your-first-workflow.md +++ b/docs/tutorial/your-first-workflow.md @@ -221,7 +221,7 @@ The workflow orchestrates. It must be deterministic — no `Date.now()`, no Create `src/workflows.ts`: ```typescript -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { orderContract } from "./contract.js"; @@ -236,15 +236,16 @@ export const processOrder = declareWorkflow({ retry: { maximumAttempts: 3 }, }, implementation: async (context, order) => { - const { transactionId } = await context.activities.chargeCard({ - customerId: order.customerId, - amount: order.amount, - }); + const { transactionId } = await propagateActivityFailure( + context.activities.chargeCard({ + customerId: order.customerId, + amount: order.amount, + }), + ); - await context.activities.sendReceipt({ - customerId: order.customerId, - transactionId, - }); + await propagateActivityFailure( + context.activities.sendReceipt({ customerId: order.customerId, transactionId }), + ); return { orderId: order.orderId, transactionId }; }, @@ -255,15 +256,16 @@ export const processOrder = declareWorkflow({ without you writing that type. So is the return value — change the returned object and TypeScript will tell you it no longer satisfies the contract. -Notice that `context.activities.chargeCard(...)` returns a **plain value**, not -a `Result`. Inside a workflow the framework unwraps the activity's `Result` for -you: success gives you the value, failure throws, and Temporal's retry policy -handles the rest. That asymmetry is deliberate and explained in -[The result model](/explanation/the-result-model). +Notice that `context.activities.chargeCard(...)` returns an **`AsyncResult`**, +not a plain value — every activity call does, whether or not the contract +declares any `errors`. `propagateActivityFailure` unwraps the success value +and re-raises the original failure on the way out, so Temporal's retry policy +still handles it — the same "let it throw" behavior as before, made explicit +at the call site. See [The result model](/explanation/the-result-model). -(An activity that declares contract errors is the exception — it returns an -`AsyncResult` so the workflow can branch on the declared failures. See -[Model domain errors](/how-to/model-domain-errors).) +(When the workflow itself should branch on a failure instead of letting +Temporal decide, narrow the `AsyncResult` with `isErr()` instead of +propagating it. See [Model domain errors](/how-to/model-domain-errors).) ## Step 5 — Run a worker @@ -425,7 +427,8 @@ boundary the data crosses. - [Adding signals and queries](/tutorial/adding-signals-and-queries) picks up this exact project and makes the workflow interactive while it runs. -- [The result model](/explanation/the-result-model) explains why activities - return `AsyncResult` but read as plain values inside a workflow. +- [The result model](/explanation/the-result-model) explains why every + activity call returns an `AsyncResult`, and when to narrow it instead of + propagating it. - [Model domain errors](/how-to/model-domain-errors) replaces the generic `WorkflowFailedError` above with typed, schema-validated failures. diff --git a/examples/README.md b/examples/README.md index cb2669fc..ea8229cc 100644 --- a/examples/README.md +++ b/examples/README.md @@ -10,7 +10,7 @@ Shared contract package — domain schemas plus the workflow, activity, signal, ### [order-processing-worker](./order-processing-worker) -Worker with Clean Architecture; activities return `AsyncResult` from unthrown, the workflow handles signals/queries via `context.defineSignal`/`defineQuery`, and a schedule-driven cleanup workflow shows the activity-less workflow shape +Worker with Clean Architecture; activities return `AsyncResult` from unthrown, the workflow handles signals/queries via `context.handleSignal`/`handleQuery`, and a schedule-driven cleanup workflow shows the activity-less workflow shape ### [order-processing-client](./order-processing-client) diff --git a/examples/order-processing-client/src/client.ts b/examples/order-processing-client/src/client.ts index a000a2ae..c56709c4 100644 --- a/examples/order-processing-client/src/client.ts +++ b/examples/order-processing-client/src/client.ts @@ -187,9 +187,22 @@ async function run() { const cancelHandle = fetchedHandle.value; // Payload-less signal — `defineSignal()` in the contract, sent with no - // arguments. - await cancelHandle.signals.cancelRequested(); - logger.info("🛑 Cancellation signal sent"); + // arguments. Narrow the Result like every other client call in this file — + // a bare `await` would collapse the AsyncResult and silently drop a failed + // send. + const cancelSent = await cancelHandle.signals.cancelRequested(); + cancelSent.match({ + ok: () => logger.info("🛑 Cancellation signal sent"), + errCases: (matcher) => + matcher + .with(P.tag(SIGNAL_VALIDATION_ERROR_TAG), (err) => + logger.error({ error: err }, "❌ Signal payload rejected by the contract"), + ) + .with(P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), (err) => + logger.error({ error: err }, "❌ Workflow execution not found"), + ), + defect: (cause) => logger.error({ cause }, "❌ Unexpected failure sending signal"), + }); const cancelOutcome = await cancelHandle.result(); cancelOutcome.match({ diff --git a/examples/order-processing-worker/src/application/workflows.ts b/examples/order-processing-worker/src/application/workflows.ts index 646af43d..8fa0bc87 100644 --- a/examples/order-processing-worker/src/application/workflows.ts +++ b/examples/order-processing-worker/src/application/workflows.ts @@ -6,8 +6,8 @@ import { ACTIVITY_CANCELLED_ERROR_TAG, ACTIVITY_ERROR_TAG, declareWorkflow, + propagateActivityFailure, rethrowCancellation, - WORKFLOW_CANCELLED_ERROR_TAG, } from "@temporal-contract/worker/workflow"; import { condition, log } from "@temporalio/workflow"; import { P } from "unthrown"; @@ -165,11 +165,36 @@ export const processOrder = declareWorkflow({ status = "failed"; log.error(`Payment declined for order ${order.orderId}: ${failure.data.reason}`); - await activities.sendNotification({ - customerId: order.customerId, - subject: "Order Failed", - message: `We're sorry, but your order ${order.orderId} could not be processed. Your payment was declined (${failure.data.reason}).`, - }); + // Best-effort notification: the declined-payment outcome below + // is what matters, so an undeclared notification failure only + // gets a warning — it must not swallow (or block) the rethrow. + // Real cancellation is the exception: it must still propagate. + await activities + .sendNotification({ + customerId: order.customerId, + subject: "Order Failed", + message: `We're sorry, but your order ${order.orderId} could not be processed. Your payment was declined (${failure.data.reason}).`, + }) + .match({ + ok: () => undefined, + errCases: (matcher) => + matcher + .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => + rethrowCancellation(cancelled), + ) + .with(P.tag(ACTIVITY_ERROR_TAG), (notifyFailure) => { + log.warn( + `Failed to notify customer of declined payment: ${notifyFailure.message}`, + ); + }), + // A defect here is still just a failed email — the + // PaymentDeclined outcome about to be rethrown below is + // already authoritative and must not be blocked by a + // notification bug. + defect: (cause) => { + log.warn(`Failed to notify customer of declined payment: ${cause}`); + }, + }); // Rethrow as this workflow's own declared contract error: the // execution fails with `ApplicationFailure(type: "PaymentDeclined")` @@ -217,68 +242,149 @@ export const processOrder = declareWorkflow({ // ------------------------------------------------------------------ status = "reserving_inventory"; log.info("Reserving inventory"); - const inventoryResult = await activities.reserveInventory(order.items); - if (!inventoryResult.reserved) { + // `reserveInventory` declares no contract errors, but every activity call + // now returns an `AsyncResult`, so a technical failure (retries + // exhausted, timeout) still needs a decision. Fold it into the same + // "not reserved" business outcome handled below — the rollback (refund + + // notify) should run regardless of *why* inventory couldn't be reserved — + // but carry `unavailable` through so the returned order result still + // tells the truth about *which* outcome happened (see the `errorCode` + // selection below): a technical fault must never be reported to the + // client as a business "out of stock" decline, mirroring how + // `processPayment` above keeps `PAYMENT_UNAVAILABLE` distinct from + // `PaymentDeclined`. Real cancellation is the exception: it must still + // propagate. + const inventoryOutcome = await activities.reserveInventory(order.items).match({ + ok: (reservation) => ({ ...reservation, unavailable: false as const }), + errCases: (matcher) => + matcher + .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => rethrowCancellation(cancelled)) + .with(P.tag(ACTIVITY_ERROR_TAG), (failure) => { + log.error(`Inventory reservation activity failed: ${failure.message}`); + return { reserved: false as const, unavailable: true as const }; + }), + // Unmodeled failure (a bug, not an anticipated outcome) — rethrow at + // the edge so Temporal surfaces the Workflow Task failure. + defect: (cause) => { + // oxlint-disable-next-line unthrown/no-throw -- defect-cause rethrow at the edge: an unmodeled failure must surface as a Workflow Task failure + throw cause; + }, + }); + + if (!inventoryOutcome.reserved) { status = "failed"; - log.error("Inventory reservation failed"); + // `unavailable` is only `true` on the technical-failure arm above — a + // business decline (no stock) reports the activity's own `Ok` value, + // which carries `unavailable: false`. + const inventoryUnavailable = inventoryOutcome.unavailable === true; + log.error( + inventoryUnavailable + ? "Inventory reservation activity failed" + : "Inventory reservation failed", + ); - // Rollback: Refund payment + // Rollback: refund the payment. Unlike the notifications in this + // workflow, a failed refund is NOT worth a warning-and-continue: the + // customer would be charged for an order that both failed and was + // never refunded. That is exactly the case where Temporal should fail + // the workflow loudly (visible, alertable) instead of completing it + // with a routine "failed" order status. `propagateActivityFailure` + // restores the exact pre-uniform-`AsyncResult` behavior: before every + // activity call returned a `Result`, an unhandled `refundPayment` + // failure threw and failed this workflow outright — this is that same + // outcome, made explicit instead of accidental. log.info("Rolling back: refunding payment"); - await activities.refundPayment(payment.transactionId); + await propagateActivityFailure(activities.refundPayment(payment.transactionId)); log.info(`Payment refunded: ${payment.transactionId}`); - await activities.sendNotification({ - customerId: order.customerId, - subject: "Order Failed", - message: `We're sorry, but your order ${order.orderId} could not be processed. One or more items are out of stock. Any charges have been refunded.`, - }); + // Best-effort notification — see the PaymentDeclined branch above for + // the same reasoning: don't let an undeclared notification failure + // block the "failed" order result, but do honor real cancellation. + const rollbackMessage = inventoryUnavailable + ? `We're sorry, but your order ${order.orderId} could not be processed. Our inventory service is temporarily unavailable. Any charges have been refunded.` + : `We're sorry, but your order ${order.orderId} could not be processed. One or more items are out of stock. Any charges have been refunded.`; + + await activities + .sendNotification({ + customerId: order.customerId, + subject: "Order Failed", + message: rollbackMessage, + }) + .match({ + ok: () => undefined, + errCases: (matcher) => + matcher + .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => + rethrowCancellation(cancelled), + ) + .with(P.tag(ACTIVITY_ERROR_TAG), (failure) => { + log.warn(`Failed to notify customer of out-of-stock order: ${failure.message}`); + }), + // A defect here is still just a failed email — the order outcome + // above (and about to be returned below) is already authoritative + // and must not be blocked by a notification bug. + defect: (cause) => { + log.warn(`Failed to notify customer of out-of-stock order: ${cause}`); + }, + }); return { orderId: order.orderId, status: "failed" as const, - failureReason: "One or more items are out of stock", - errorCode: "OUT_OF_STOCK", + failureReason: inventoryUnavailable + ? "Inventory could not be reserved" + : "One or more items are out of stock", + errorCode: inventoryUnavailable ? "INVENTORY_UNAVAILABLE" : "OUT_OF_STOCK", }; } - log.info(`Inventory reserved: ${inventoryResult.reservationId}`); + log.info(`Inventory reserved: ${inventoryOutcome.reservationId}`); // ------------------------------------------------------------------ // Step 4: create shipment // ------------------------------------------------------------------ status = "creating_shipment"; log.info("Creating shipment"); - const shippingResult = await activities.createShipment({ - orderId: order.orderId, - customerId: order.customerId, - }); + + // No rollback path exists for a failed shipment creation (unlike + // inventory reservation above) — that is a genuine "let Temporal fail + // the workflow" case, not a business outcome this example models. + const shippingResult = await propagateActivityFailure( + activities.createShipment({ + orderId: order.orderId, + customerId: order.customerId, + }), + ); log.info(`Shipment created: ${shippingResult.trackingNumber}`); - // Step 5: Send success notification (non-critical). `sendNotification` - // declares no errors, so it is a throwing Promise — `cancellableScope` - // folds it into the Result discipline instead of a `try/catch`: - // cancellation surfaces as `Err(WorkflowCancelledError)`, anything else - // it throws is a defect. - await context - .cancellableScope(() => - activities.sendNotification({ - customerId: order.customerId, - subject: "Order Confirmed", - message: `Your order ${order.orderId} has been confirmed and will be shipped. Tracking: ${shippingResult.trackingNumber}`, - }), - ) + // Step 5: Send success notification (non-critical). Every activity call + // now returns an `AsyncResult` — including cancellation, via + // `ActivityCancelledError` — so narrowing the call's own result is enough + // and no longer needs a wrapping `cancellableScope` just to observe it. + await activities + .sendNotification({ + customerId: order.customerId, + subject: "Order Confirmed", + message: `Your order ${order.orderId} has been confirmed and will be shipped. Tracking: ${shippingResult.trackingNumber}`, + }) .match({ ok: () => undefined, // Cancellation must propagate — absorbing it here would complete the // workflow after a cancel request instead of ending it `Cancelled`. errCases: (matcher) => - matcher.with(P.tag(WORKFLOW_CANCELLED_ERROR_TAG), (cancelled) => - rethrowCancellation(cancelled), - ), - // Non-critical: the order is already shipped, so even an unmodeled - // notification failure is only worth a warning. + matcher + .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => + rethrowCancellation(cancelled), + ) + // Non-critical: the order is already shipped, so even an + // undeclared notification failure is only worth a warning. + .with(P.tag(ACTIVITY_ERROR_TAG), (failure) => { + log.warn(`Failed to send confirmation notification: ${failure.message}`); + }), + // A defect here is still just a failed email — the order already + // completed successfully above, and that outcome is authoritative. defect: (cause) => { log.warn(`Failed to send confirmation notification: ${cause}`); }, @@ -331,7 +437,13 @@ export const cleanupExpiredOrders = declareWorkflow({ implementation: async (context, { olderThanDays }) => { log.info(`Starting order cleanup (older than ${olderThanDays} days)`); - const { purgedCount } = await context.activities.purgeExpiredOrders({ olderThanDays }); + // A scheduled cleanup job with no recovery path: a failed purge should + // fail loudly (visible in the Temporal UI, the schedule runs again next + // time) rather than being silently swallowed into a fake "0 purged" + // success. + const { purgedCount } = await propagateActivityFailure( + context.activities.purgeExpiredOrders({ olderThanDays }), + ); log.info(`Order cleanup finished: purged ${purgedCount} orders`); return { purgedCount }; diff --git a/packages/contract/src/errors-impl.ts b/packages/contract/src/errors-impl.ts index d2a6b04d..346547a4 100644 --- a/packages/contract/src/errors-impl.ts +++ b/packages/contract/src/errors-impl.ts @@ -27,8 +27,6 @@ import { TaggedError } from "unthrown"; import { CONTRACT_ERROR_TAG, TECHNICAL_ERROR_TAG } from "./error-tags.js"; import type { AnySchema, ErrorDefinition, InferErrorData, InferErrorDataInput } from "./types.js"; -export { CONTRACT_ERROR_TAG, TECHNICAL_ERROR_TAG } from "./error-tags.js"; - /** * Error for technical/runtime failures that cannot be prevented by * TypeScript — connection failures, missing runtime capabilities, worker diff --git a/packages/worker/README.md b/packages/worker/README.md index 614b1d1e..5e43d3b4 100644 --- a/packages/worker/README.md +++ b/packages/worker/README.md @@ -37,7 +37,7 @@ export const activities = declareActivitiesHandler({ ```typescript // workflows.ts -import { declareWorkflow } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; import { myContract } from "./contract.js"; @@ -46,8 +46,10 @@ export const processOrder = declareWorkflow({ contract: myContract, activityOptions: { startToCloseTimeout: "1 minute" }, implementation: async ({ activities }, input) => { - // Activities return plain values (Result is unwrapped internally) - await activities.sendEmail({ to: "user@example.com", body: "Done!" }); + // Every activity call returns an AsyncResult — narrow it, or use + // `propagateActivityFailure` to let Temporal decide the workflow's fate. + // A bare `await` here compiles but silently discards a failed call. + await propagateActivityFailure(activities.sendEmail({ to: "user@example.com", body: "Done!" })); return { success: true }; }, }); @@ -197,8 +199,13 @@ export const extractLayout = declareWorkflow({ // Activities not listed here fall through to the default queue/options. }, implementation: async ({ activities }, input) => { - // Each call still validates input/output against the contract. - const layout = await activities.extractLayoutChunk({ docId: input.docId }); + // Each call still validates input/output against the contract, and + // still returns an AsyncResult — propagate it to let a failure fail + // the workflow instead of returning an AsyncResult where `{ layout }` + // expects the unwrapped value. + const layout = await propagateActivityFailure( + activities.extractLayoutChunk({ docId: input.docId }), + ); return { layout }; }, }); diff --git a/packages/worker/src/__tests__/cancellation.contract.ts b/packages/worker/src/__tests__/cancellation.contract.ts index 0a48a8ae..23fc0425 100644 --- a/packages/worker/src/__tests__/cancellation.contract.ts +++ b/packages/worker/src/__tests__/cancellation.contract.ts @@ -10,14 +10,16 @@ import { z } from "zod"; * re-listed per workflow (mirrors `globalTimeoutActivity` in * `activity-options.contract.ts`). * - * The `errors` map — even empty — is what puts this activity's calls on the - * `AsyncResult` path (`activities-proxy.ts`'s `makeResultShapedActivity`). - * That is precisely the condition that creates the swallowed-cancellation - * hazard under test: a cancelled in-flight call to an errors-declaring - * activity resolves to `Err(ActivityCancelledError)` — a value on the SAME - * modeled channel as an ordinary declared failure, and therefore - * indistinguishable from one to a handler that maps every `Err` to a - * generic fallback. + * Every activity call — this one included — is on the `AsyncResult` path + * (`activities-proxy.ts`'s `makeResultShapedActivity` wraps all of them, not + * just ones with a declared `errors` map). That uniform path is precisely + * the condition that creates the swallowed-cancellation hazard under test: a + * cancelled in-flight activity call resolves to `Err(ActivityCancelledError)` + * — a value on the SAME modeled channel as an ordinary declared failure, and + * therefore indistinguishable from one to a handler that maps every `Err` to + * a generic fallback. The empty `errors: {}` here is incidental (it only + * types the declared-error member of the union as `never`), not what puts + * the call on the `AsyncResult` path. */ const slowActivity = defineActivity({ input: z.object({ sleepMs: z.number() }), diff --git a/packages/worker/src/__tests__/cancellation.inprocess.spec.ts b/packages/worker/src/__tests__/cancellation.inprocess.spec.ts index ed700bb3..6ce92915 100644 --- a/packages/worker/src/__tests__/cancellation.inprocess.spec.ts +++ b/packages/worker/src/__tests__/cancellation.inprocess.spec.ts @@ -22,12 +22,12 @@ import { inprocessContract } from "./inprocess.contract.js"; * (the old `cancellation.spec.ts`) could only assert that * `cancellableScope`/`nonCancellableScope` CALLED the mocked primitives; it * could never reproduce Temporal's actual cancellation propagation, nor the - * swallowed-cancellation hazard this file exists to prove: an activity that - * declares an `errors` map turns a cancellation into an `Err(...)` on the - * SAME modeled channel as an ordinary declared failure, so a generic - * "map every Err to a fallback" handler absorbs it — the workflow completes - * `Completed` instead of `Cancelled`, silently overriding the cancel - * request. + * swallowed-cancellation hazard this file exists to prove: every activity + * call turns a cancellation into an `Err(ActivityCancelledError)` on the + * SAME modeled channel as an ordinary declared failure — declared `errors` + * map or not — so a generic "map every Err to a fallback" handler absorbs + * it — the workflow completes `Completed` instead of `Cancelled`, silently + * overriding the cancel request. */ /** * Bounds every workflow started below. A regression in the `context` diff --git a/packages/worker/src/__tests__/propagation.contract.ts b/packages/worker/src/__tests__/propagation.contract.ts new file mode 100644 index 00000000..aa2133d7 --- /dev/null +++ b/packages/worker/src/__tests__/propagation.contract.ts @@ -0,0 +1,56 @@ +import { defineActivity, defineContract, defineWorkflow } from "@temporal-contract/contract"; +import { z } from "zod"; + +/** + * Fails on every attempt and declares NO `errors` map. Before the + * uniform-`AsyncResult` change, this activity's workflow-side call took the + * (now-deleted) `makeThrowingActivity` path, and Temporal's original + * `ActivityFailure` propagated out of the workflow via a bare `await`. This + * is the activity whose observable behavior — now reached through + * `propagateActivityFailure`, or handled by narrowing `isErr()` — must stay + * IDENTICAL to that pre-change throwing behavior. + * + * `maximumAttempts: 2` bounds the run: enough to prove Temporal retried, + * short enough that a regression cannot stall the suite. + */ +const alwaysFailsNoErrors = defineActivity({ + input: z.object({}), + output: z.object({ ok: z.boolean() }), + activityOptions: { + startToCloseTimeout: "5 seconds", + retry: { maximumAttempts: 2, backoffCoefficient: 1, initialInterval: "1 second" }, + }, +}); + +/** The same, but declaring an error — already on the Result path today. */ +const alwaysFailsWithErrors = defineActivity({ + input: z.object({}), + output: z.object({ ok: z.boolean() }), + errors: { + Boom: { data: z.object({ at: z.number() }), nonRetryable: true }, + }, + activityOptions: { + startToCloseTimeout: "5 seconds", + retry: { maximumAttempts: 2, backoffCoefficient: 1, initialInterval: "1 second" }, + }, +}); + +/** Lets the activity failure escape, so Temporal decides the workflow outcome. */ +const propagatesFailure = defineWorkflow({ + input: z.object({}), + output: z.object({ reached: z.boolean() }), + activities: { alwaysFailsNoErrors }, +}); + +/** Catches the failure and returns normally, so the workflow completes. */ +const handlesFailure = defineWorkflow({ + input: z.object({}), + output: z.object({ outcome: z.string() }), + activities: { alwaysFailsNoErrors }, +}); + +export const propagationContract = defineContract({ + taskQueue: "propagation-tests", + workflows: { propagatesFailure, handlesFailure }, + activities: { alwaysFailsWithErrors }, +}); diff --git a/packages/worker/src/__tests__/propagation.inprocess.spec.ts b/packages/worker/src/__tests__/propagation.inprocess.spec.ts new file mode 100644 index 00000000..cb74e321 --- /dev/null +++ b/packages/worker/src/__tests__/propagation.inprocess.spec.ts @@ -0,0 +1,164 @@ +import { testRig } from "@temporal-contract/testing/test-rig"; +import { it } from "@temporal-contract/testing/time-skipping"; +import { + bundleFor, + fixturePath, + nextTaskQueueId, + withTaskQueue, +} from "@temporal-contract/testing/workflow-bundle"; +import { Context } from "@temporalio/activity"; +import { ActivityFailure } from "@temporalio/common"; +import { OkAsync } from "unthrown"; +import { describe, expect } from "vitest"; + +import { declareActivitiesHandler } from "../activity.js"; +import { propagationContract } from "./propagation.contract.js"; + +// Short, so a workflow-TASK retry loop (the failure mode a wrong propagation +// helper produces) times out fast instead of stalling the suite for 120s. +const WORKFLOW_EXECUTION_TIMEOUT = "10 seconds"; + +// `alwaysFailsWithErrors` is declared on the contract's global `activities` +// block (Task 3's fixture, unused here — see propagation.contract.ts) but +// `declareActivitiesHandler` requires an implementation for every declared +// activity regardless of whether this task's tests invoke it. It is never +// called by either workflow under test. +const alwaysFailsWithErrors = () => OkAsync({ ok: true }); + +/** + * These tests are a CHARACTERIZATION of behavior that must survive the + * uniform-`AsyncResult` change. They are written against the pre-change + * implementation and must pass unmodified afterwards. Do not adjust them to + * match new behavior — a diff here means the change altered semantics. + */ +describe("activity failure propagation — characterization", () => { + it("fails the workflow, after Temporal's own retries, when the failure escapes", async ({ + testEnv, + }) => { + const contract = withTaskQueue(propagationContract, nextTaskQueueId("prop-escape")); + const bundle = await bundleFor(fixturePath(import.meta.url, "propagation.workflows")); + + const attempts: number[] = []; + // `alwaysFailsNoErrors` is declared on both `propagatesFailure` and + // `handlesFailure`, so it shares one flat runtime namespace name. + // `declareActivitiesHandler` requires the exact same function reference + // from every scope that declares it (see activity.ts's `shouldRegister`) + // — passing two different closures throws a declaration-time config + // error, even though only one workflow is exercised per test. + // + // Synchronous (not `async`), not `AsyncResult`-returning: the handler's + // TYPE always demands `AsyncResult` (`ResultActivityImplementation`), but + // an `async () => { throw }` infers `Promise`, which doesn't + // satisfy it. A plain `() => never` does, because `never` is assignable + // to any type — and at runtime a synchronous throw inside this callback + // rejects the wrapper's promise identically to an async throw, so the + // Activity Task failure Temporal observes is unchanged. + const alwaysFailsNoErrors = (): never => { + attempts.push(Context.current().info.attempt); + // oxlint-disable-next-line unthrown/no-throw -- the activity under test must fail + throw new Error("activity exploded"); + }; + const activities = declareActivitiesHandler({ + contract, + activities: { + alwaysFailsWithErrors, + propagatesFailure: { alwaysFailsNoErrors }, + handlesFailure: { alwaysFailsNoErrors }, + }, + }); + + const { worker, client } = await testRig(testEnv, { contract, bundle, activities }); + + const outcome = await worker.raw.runUntil( + (async () => { + const result = await client.executeWorkflow("propagatesFailure", { + workflowId: "prop-escape", + args: {}, + workflowExecutionTimeout: WORKFLOW_EXECUTION_TIMEOUT, + }); + + // `isErr()` alone does NOT discriminate this test's regression: a + // workflow-TASK retry loop (what a rethrown non-`TemporalFailure` + // produces — see `retry.workflows.ts`'s doc comment) runs out the + // `workflowExecutionTimeout` above, and the resulting + // `WorkflowTimeoutError` is ALSO an `Err`. So `isErr()` is true in + // both the correct world and the regression world; only the error's + // identity (`WorkflowFailedError` vs. `WorkflowTimeoutError`) tells + // them apart. Folding a `defect` into its own branch (rather than + // lumping it into "completed") is the same house style as + // `retry.workflows.ts`. + if (result.isDefect()) return `defect:${String(result.cause)}`; + if (!result.isErr()) return "completed"; + + const error = result.error; + // Not every member of `WorkflowResultErrorsOf` carries `cause` + // (`WorkflowValidationError` doesn't), so narrow with `in` rather + // than assuming the property exists on the whole union. + const cause = "cause" in error ? error.cause : undefined; + // Pin WHAT propagated, not just that something did: Temporal's own + // `ActivityFailure` wrapper, whose inner cause carries the + // activity's original message. If a regression re-wrapped this into + // e.g. a bare `ApplicationFailure`, this string changes. + const innerCause = cause instanceof ActivityFailure ? cause.cause : undefined; + const innerMessage = innerCause instanceof Error ? innerCause.message : String(innerCause); + return `failed:${error.name}:${cause instanceof Error ? cause.constructor.name : String(cause)}:${innerMessage}`; + })(), + ); + + // The workflow must FAIL with Temporal's own `WorkflowFailedError` + // wrapping the original `ActivityFailure` (whose cause message is the + // activity's own thrown message) — not complete, not surface a defect, + // and specifically not a `WorkflowTimeoutError`, which is what a + // workflow-TASK retry loop spinning out the execution timeout would + // produce. And Temporal must have retried the activity to its + // configured maximum, proving the retry policy reached the server + // rather than being short-circuited client-side. + expect(outcome).toBe("failed:WorkflowFailedError:ActivityFailure:activity exploded"); + expect(attempts).toEqual([1, 2]); + }); + + it("completes the workflow when the failure is caught", async ({ testEnv }) => { + const contract = withTaskQueue(propagationContract, nextTaskQueueId("prop-handled")); + const bundle = await bundleFor(fixturePath(import.meta.url, "propagation.workflows")); + + // Same flat-namespace constraint, and same sync-throw-typechecks-as-never + // reasoning, as the previous test. Also tracks `attempts` via Temporal's + // own `Context.current().info.attempt`, same as the previous test: a + // bare `catch { return { outcome: "handled" } }` in the workflow (now + // fixed to fold the caught error's identity — see + // `propagation.workflows.ts`) would still go green if a regression never + // dispatched the activity at all. Asserting the server-reported attempt + // numbers proves the activity actually ran to its configured maximum. + const attempts: number[] = []; + const alwaysFailsNoErrors = (): never => { + attempts.push(Context.current().info.attempt); + // oxlint-disable-next-line unthrown/no-throw -- the activity under test must fail + throw new Error("activity exploded"); + }; + const activities = declareActivitiesHandler({ + contract, + activities: { + alwaysFailsWithErrors, + propagatesFailure: { alwaysFailsNoErrors }, + handlesFailure: { alwaysFailsNoErrors }, + }, + }); + + const { worker, client } = await testRig(testEnv, { contract, bundle, activities }); + + const result = await worker.raw.runUntil( + client + .executeWorkflow("handlesFailure", { + workflowId: "prop-handled", + args: {}, + workflowExecutionTimeout: WORKFLOW_EXECUTION_TIMEOUT, + }) + .getOrThrow(), + ); + + // `handled:ActivityFailure` pins WHAT the workflow's `catch` observed, + // not just that it caught something. + expect(result).toEqual({ outcome: "handled:ActivityFailure" }); + expect(attempts).toEqual([1, 2]); + }); +}); diff --git a/packages/worker/src/__tests__/propagation.workflows.ts b/packages/worker/src/__tests__/propagation.workflows.ts new file mode 100644 index 00000000..04303494 --- /dev/null +++ b/packages/worker/src/__tests__/propagation.workflows.ts @@ -0,0 +1,50 @@ +import { ActivityFailure } from "@temporalio/workflow"; + +import { ActivityError } from "../errors.js"; +import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; +import { propagationContract } from "./propagation.contract.js"; + +/** + * Lets the failure escape via `propagateActivityFailure`, the post-change + * equivalent of the bare `await` this fixture used before. The + * characterization spec asserts the workflow still FAILS and that Temporal + * still retried the activity to its configured maximum. + */ +export const propagatesFailure = declareWorkflow({ + workflowName: "propagatesFailure", + contract: propagationContract, + implementation: async (context) => { + await propagateActivityFailure(context.activities.alwaysFailsNoErrors({})); + return { reached: true }; + }, +}); + +/** + * Catches the failure by narrowing, so the workflow COMPLETES. Uses + * `isErr()`/`isDefect()` because the call now returns an `AsyncResult`; before + * the uniform-`AsyncResult` change this was a bare try/catch. + */ +export const handlesFailure = declareWorkflow({ + workflowName: "handlesFailure", + contract: propagationContract, + implementation: async (context) => { + const result = await context.activities.alwaysFailsNoErrors({}); + + if (result.isOk()) { + return { outcome: "unexpected-success" }; + } + + // Fold the caught error's identity into the returned status rather than + // a bare "handled": a bare success-path swallows ANYTHING (an + // input-validation failure at the proxy boundary, a proxy-construction + // error), so a regression that never even dispatched the activity would + // still report "handled". Naming `ActivityFailure` — Temporal's own + // wrapper for the call under test, retained pre-unwrap as + // `ActivityError.originalFailure` — pins WHAT was caught, not just that + // something was. + const error: unknown = result.isErr() ? result.error : result.cause; + const originalFailure = error instanceof ActivityError ? error.originalFailure : undefined; + const errorName = originalFailure instanceof ActivityFailure ? originalFailure.name : "unknown"; + return { outcome: `handled:${errorName}` }; + }, +}); diff --git a/packages/worker/src/__tests__/routing.workflows.ts b/packages/worker/src/__tests__/routing.workflows.ts index 14b8256c..db8689fa 100644 --- a/packages/worker/src/__tests__/routing.workflows.ts +++ b/packages/worker/src/__tests__/routing.workflows.ts @@ -1,4 +1,4 @@ -import { declareWorkflow } from "../workflow.js"; +import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; import { ROUTED_ACTIVITY_QUEUE, routingContract } from "./routing.contract.js"; /** @@ -18,7 +18,11 @@ export const routedFlow = declareWorkflow({ reportQueue: { taskQueue: ROUTED_ACTIVITY_QUEUE }, }, implementation: async (context) => { - const { handledBy } = await context.activities.reportQueue({}); + // The spec only ever exercises the success path (the dedicated activity + // worker always resolves); a failure here would be an unmodeled routing + // bug, so let Temporal decide the workflow's fate rather than folding it + // into a returned status. + const { handledBy } = await propagateActivityFailure(context.activities.reportQueue({})); return { handledBy }; }, }); diff --git a/packages/worker/src/__tests__/test.workflows.ts b/packages/worker/src/__tests__/test.workflows.ts index 7e452c84..c159544a 100644 --- a/packages/worker/src/__tests__/test.workflows.ts +++ b/packages/worker/src/__tests__/test.workflows.ts @@ -1,13 +1,13 @@ import { sleep } from "@temporalio/workflow"; -import { declareWorkflow } from "../workflow.js"; +import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; import { testContract } from "./test.contract.js"; export const simpleWorkflow = declareWorkflow({ workflowName: "simpleWorkflow", contract: testContract, implementation: async ({ activities }, args) => { - await activities.logMessage({ message: `Processing: ${args.value}` }); + await propagateActivityFailure(activities.logMessage({ message: `Processing: ${args.value}` })); return { result: `Processed: ${args.value}`, }; @@ -30,8 +30,17 @@ export const workflowWithActivities = declareWorkflow({ }, }, implementation: async ({ activities }, args) => { + // Both activities below always succeed technically in this fixture's + // tests — `valid`/`success` are business outcomes carried in the Ok + // value, not activity failures. A *technical* failure here (neither + // test exercises one) should still fail the workflow rather than being + // folded into the "failed" business status, so unwrap with + // propagateActivityFailure and only branch on the business fields. + // Validate order - const validationResult = await activities.validateOrder({ orderId: args.orderId }); + const validationResult = await propagateActivityFailure( + activities.validateOrder({ orderId: args.orderId }), + ); if (!validationResult.valid) { return { @@ -42,7 +51,9 @@ export const workflowWithActivities = declareWorkflow({ } // Process payment - const paymentResult = await activities.processPayment({ amount: args.amount }); + const paymentResult = await propagateActivityFailure( + activities.processPayment({ amount: args.amount }), + ); if (!paymentResult.success) { return { @@ -53,9 +64,11 @@ export const workflowWithActivities = declareWorkflow({ } // Log success - await activities.logMessage({ - message: `Order ${args.orderId} completed with transaction ${paymentResult.transactionId}`, - }); + await propagateActivityFailure( + activities.logMessage({ + message: `Order ${args.orderId} completed with transaction ${paymentResult.transactionId}`, + }), + ); return { orderId: args.orderId, @@ -136,7 +149,9 @@ export const childWorkflow = declareWorkflow({ workflowName: "childWorkflow", contract: testContract, implementation: async ({ activities }, args) => { - await activities.logMessage({ message: `Child workflow ${args.id} running` }); + await propagateActivityFailure( + activities.logMessage({ message: `Child workflow ${args.id} running` }), + ); return { message: `Child ${args.id} completed`, }; @@ -151,10 +166,13 @@ export const workflowWithFailableActivity = declareWorkflow({ workflowName: "workflowWithFailableActivity", contract: testContract, implementation: async ({ activities }, args) => { - const result = await activities.failableActivity({ shouldFail: args.shouldFail }); - return { - success: result.success, - }; + // The (skipped) "Error Handling" spec in worker.spec.ts expects the + // workflow itself to FAIL when the activity fails — not to fold the + // failure into a returned status — so let it escape via + // propagateActivityFailure rather than narrowing. + return await propagateActivityFailure( + activities.failableActivity({ shouldFail: args.shouldFail }), + ); }, activityOptions: { startToCloseTimeout: "1 minute", diff --git a/packages/worker/src/__tests__/timeouts.workflows.ts b/packages/worker/src/__tests__/timeouts.workflows.ts index a15ae1ac..cbc31aba 100644 --- a/packages/worker/src/__tests__/timeouts.workflows.ts +++ b/packages/worker/src/__tests__/timeouts.workflows.ts @@ -1,4 +1,4 @@ -import { declareWorkflow } from "../workflow.js"; +import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; import { timeoutsContract } from "./timeouts.contract.js"; export const reportsLayered = declareWorkflow({ @@ -13,10 +13,11 @@ export const reportsLayered = declareWorkflow({ // activity-options.contract.ts. activityOptionsByName: { reportsTimeouts: { startToCloseTimeout: "9 seconds" } }, implementation: async (context) => { - // `reportsTimeouts` declares no contract `errors`, so the workflow-side - // proxy is the plain-throwing wrapper (matching Temporal's native - // behavior) rather than an AsyncResult — there is no `.isErr()`/ - // `.isDefect()` to narrow here. - return await context.activities.reportsTimeouts({}); + // `reportsTimeouts` declares no contract `errors`, but every activity + // call is uniformly `AsyncResult`-shaped now regardless of a declared + // `errors` map. The spec only exercises the success path (each merge + // layer contributing its value), so let a technical failure escape via + // propagateActivityFailure and have Temporal decide the workflow's fate. + return await propagateActivityFailure(context.activities.reportsTimeouts({})); }, }); diff --git a/packages/worker/src/activities-proxy.spec.ts b/packages/worker/src/activities-proxy.spec.ts index 2889a7f4..29bc19e8 100644 --- a/packages/worker/src/activities-proxy.spec.ts +++ b/packages/worker/src/activities-proxy.spec.ts @@ -1,13 +1,18 @@ import type { ActivityDefinition } from "@temporal-contract/contract"; import { type AnyContractError, ContractError } from "@temporal-contract/contract/errors"; -import { ApplicationFailure, CancelledFailure } from "@temporalio/common"; +import { + ActivityFailure, + ApplicationFailure, + CancelledFailure, + RetryState, +} from "@temporalio/common"; import type { AsyncResult } from "unthrown"; /** - * Runtime coverage for `createValidatedActivities` — specifically the - * Result-shaped wrapper that activities with a declared `errors` map get on - * the workflow side: rehydration of declared `ApplicationFailure`s into - * typed `ContractError`s, cancellation discrimination, and the - * `ActivityError` fallback for everything else. + * Runtime coverage for `createValidatedActivities` — the Result-shaped + * wrapper every activity gets on the workflow side, declared `errors` map or + * not: rehydration of declared `ApplicationFailure`s into typed + * `ContractError`s (when the activity declares errors), cancellation + * discrimination, and the `ActivityError` fallback for everything else. */ import { describe, expect, it } from "vitest"; import { z } from "zod"; @@ -22,6 +27,14 @@ import { ActivityCancelledError, type ActivityError } from "./errors.js"; */ type ProxyError = AnyContractError | ActivityError | ActivityCancelledError; +/** + * The error union for activities with NO declared `errors` map — mirrors + * `ActivityErrorsFor`'s else-branch in activities-proxy.ts: the same two + * classification fallbacks as `ProxyError`, minus the typed rehydration + * member that can't occur without a declared `errors` map. + */ +type NoDeclaredErrors = ActivityError | ActivityCancelledError; + const erroredDefinition = { input: z.object({ amount: z.number() }), output: z.object({ transactionId: z.string() }), @@ -46,19 +59,21 @@ const buildProxy = (raw: (...args: unknown[]) => Promise) => ) as unknown as Record AsyncResult>; describe("createValidatedActivities — activities without declared errors", () => { - it("keeps the historical throwing Promise shape", async () => { + it("is Result-shaped too — no more special throwing shape for undeclared errors", async () => { const activities = createValidatedActivities( { chargePayment: async () => ({ transactionId: "tx" }) }, { chargePayment: plainDefinition }, undefined, - ) as unknown as Record Promise>; + ) as unknown as Record AsyncResult>; - await expect(activities["chargePayment"]!({ amount: 1 })).resolves.toEqual({ - transactionId: "tx", - }); - await expect(activities["chargePayment"]!({ amount: "bad" })).rejects.toThrow( - /input validation failed/, - ); + const okResult = await activities["chargePayment"]!({ amount: 1 }); + expect(okResult).toBeOkWith({ transactionId: "tx" }); + + const errResult = await activities["chargePayment"]!({ amount: "bad" }); + expect(errResult).toBeErrTagged("@temporal-contract/ActivityError"); + if (errResult.isErr()) { + expect(errResult.error.message).toContain("input validation failed"); + } }); }); @@ -80,7 +95,7 @@ describe("createValidatedActivities — wire format (validate on send, parse on }, } as unknown as ActivityDefinition; - it("throwing shape: sends the ORIGINAL input over the wire and parses the output once", async () => { + it("no declared errors: sends the ORIGINAL input over the wire and parses the output once", async () => { const seen: unknown[] = []; const activities = createValidatedActivities( { @@ -91,7 +106,7 @@ describe("createValidatedActivities — wire format (validate on send, parse on }, { transformer: transformDefinition }, undefined, - ) as unknown as Record Promise>; + ) as unknown as Record AsyncResult>; const result = await activities["transformer"]!({ text: "hi" }); @@ -99,10 +114,10 @@ describe("createValidatedActivities — wire format (validate on send, parse on // parsed `{ text: "hi!" }` — the activity worker parses on receive. expect(seen).toEqual([{ text: "hi" }]); // The activity's wire value (pre-transform) is parsed exactly once here. - expect(result).toEqual({ n: 42 }); + expect(result).toBeOkWith({ n: 42 }); }); - it("Result shape: sends the ORIGINAL input over the wire and parses the output once", async () => { + it("declared errors: sends the ORIGINAL input over the wire and parses the output once", async () => { const seen: unknown[] = []; const activities = createValidatedActivities( { @@ -167,6 +182,37 @@ describe("createValidatedActivities — activities with declared errors", () => } }); + it("preserves the original ActivityFailure wrapper as ActivityError.originalFailure, alongside the unwrapped cause", async () => { + // classifyActivityError unwraps Temporal's ActivityFailure to build + // `cause` (the failure below only throws a bare ApplicationFailure, never + // exercising that unwrap branch or the originalFailure argument at all — + // this test drives a REAL ActivityFailure wrapper through the raw + // activity so both are covered). + const inner = ApplicationFailure.create({ type: "GATEWAY_5XX", message: "boom" }); + const wrapper = new ActivityFailure( + "activity failed", + "chargePayment", + "1", + RetryState.MAXIMUM_ATTEMPTS_REACHED, + undefined, + inner, + ); + const activities = buildProxy(async () => { + throw wrapper; + }); + + const result = await activities["chargePayment"]!({ amount: 1 }); + expect(result).toBeErrTagged("@temporal-contract/ActivityError"); + if (result.isErr()) { + const error = result.error as InstanceType; + // cause stays the UNWRAPPED failure (documented, unchanged behavior). + expect(error.cause).toBe(inner); + // originalFailure is the wrapper Temporal actually threw, retained + // specifically so propagateActivityFailure can re-raise it faithfully. + expect(error.originalFailure).toBe(wrapper); + } + }); + it("surfaces a declared type with a mismatching payload as ActivityError (no wrong typed error)", async () => { const failure = ApplicationFailure.create({ type: "PaymentDeclined", @@ -192,6 +238,33 @@ describe("createValidatedActivities — activities with declared errors", () => } }); + it("preserves the original ActivityFailure wrapper on ActivityCancelledError.cause when cancellation is wrapped", async () => { + // isCancellation() recognizes an ActivityFailure whose cause is a + // CancelledFailure — the shape a real cancelled activity actually throws + // (not the bare CancelledFailure the test above uses). Cancellation is + // detected BEFORE classifyActivityError's unwrap, so cause here should be + // the wrapper itself, not the inner CancelledFailure. + const cancelledFailure = new CancelledFailure("activity cancelled"); + const wrapper = new ActivityFailure( + "activity failed", + "chargePayment", + "1", + RetryState.CANCEL_REQUESTED, + undefined, + cancelledFailure, + ); + const activities = buildProxy(async () => { + throw wrapper; + }); + + const result = await activities["chargePayment"]!({ amount: 1 }); + expect(result).toBeErrTagged("@temporal-contract/ActivityCancelledError"); + if (result.isErr()) { + const error = result.error as InstanceType; + expect(error.cause).toBe(wrapper); + } + }); + it("folds workflow-side input validation failure into Err(ActivityError)", async () => { const activities = buildProxy(async () => ({ transactionId: "tx" })); diff --git a/packages/worker/src/activities-proxy.ts b/packages/worker/src/activities-proxy.ts index cfb6f85b..ddb963b7 100644 --- a/packages/worker/src/activities-proxy.ts +++ b/packages/worker/src/activities-proxy.ts @@ -26,6 +26,24 @@ import { import { makeAsyncResult } from "./internal.js"; import type { ClientInferInput, ClientInferOutput } from "./types.js"; +/** + * The error channel for an activity call: declared contract errors when the + * activity declares an `errors` map, plus the two failures every activity can + * produce. + * + * - **With an `errors` map** — declared failures are rehydrated from the + * `ApplicationFailure` wire shape into typed {@link ContractErrorUnion} + * members (data re-validated against the declared schema). + * - **Always** — any other failure surfaces as {@link ActivityError} (with + * Temporal's `ActivityFailure` wrapper unwrapped to its actionable cause) + * or {@link ActivityCancelledError} (mirroring the child-workflow API). + */ +export type ActivityErrorsFor = TActivity extends { + errors: infer TErrors extends Record; +} + ? ContractErrorUnion | ActivityError | ActivityCancelledError + : ActivityError | ActivityCancelledError; + /** * Activity function signature from workflow execution perspective. * @@ -34,30 +52,15 @@ import type { ClientInferInput, ClientInferOutput } from "./types.js"; * the input but transmits the original value (the activity worker parses it * on receive), and parses the activity's result on receive. * - * The shape depends on whether the activity declares contract errors: - * - * - **No `errors` map** — plain `Promise`, matching Temporal's native - * behavior: a failure (retries exhausted, timeout, cancellation) throws and - * propagates unless caught / scoped. - * - **With an `errors` map** — `AsyncResult`. Declared failures are - * rehydrated from the `ApplicationFailure` wire shape into typed - * {@link ContractErrorUnion} members (data re-validated against the - * declared schema); any other failure surfaces as - * {@link ActivityError} (with Temporal's `ActivityFailure` wrapper - * unwrapped to its actionable cause) or {@link ActivityCancelledError} - * (mirroring the child-workflow API). + * Every activity call returns an `AsyncResult` — the call convention no + * longer depends on whether the contract declared errors, only the error + * channel does. To let a failure escape and have Temporal decide the + * workflow's outcome, use `propagateActivityFailure` rather than + * unthrown's `.getOrThrow()`; see that function's documentation for why. */ -export type WorkflowInferActivity = TActivity extends { - errors: infer TErrors extends Record; -} - ? ( - args: ClientInferInput, - ) => AsyncResult< - ClientInferOutput, - ContractErrorUnion | ActivityError | ActivityCancelledError - > - : (args: ClientInferInput) => Promise>; +export type WorkflowInferActivity = ( + args: ClientInferInput, +) => AsyncResult, ActivityErrorsFor>; /** * All global activities from a contract (workflow execution perspective). @@ -102,9 +105,10 @@ export type WorkflowInferWorkflowContextActivities< * its return and transmitted the original value — so a transforming * output schema is applied exactly once, here. * - * Activities that declare contract errors additionally get failure - * classification: their wrapper returns an `AsyncResult` whose error channel - * carries the rehydrated typed errors (see {@link WorkflowInferActivity}). + * Every wrapper returns an `AsyncResult` whose error channel carries failure + * classification — the rehydrated typed contract errors when the activity + * declares an `errors` map, plus the two failures every activity can produce + * (see {@link WorkflowInferActivity}). */ export function createValidatedActivities< TContract extends ContractDefinition, @@ -138,46 +142,18 @@ export function createValidatedActivities< ); } - (validatedActivities as Record)[activityName] = activityDef.errors - ? makeResultShapedActivity(activityName, activityDef, rawActivity) - : makeThrowingActivity(activityName, activityDef, rawActivity); + (validatedActivities as Record)[activityName] = makeResultShapedActivity( + activityName, + activityDef, + rawActivity, + ); } return validatedActivities; } /** - * Validation-only wrapper for activities without declared errors — the - * historical shape: validate input (send the original), invoke, parse - * output, let failures throw through to Temporal's native handling. - */ -function makeThrowingActivity( - activityName: string, - activityDef: ActivityDefinition, - rawActivity: (...args: unknown[]) => Promise, -) { - return async (input: unknown) => { - const inputResult = await activityDef.input["~standard"].validate(input); - if (inputResult.issues) { - // oxlint-disable-next-line unthrown/no-throw -- sanctioned ValidationError/ApplicationFailure model: terminal failure Temporal must see thrown (CLAUDE.md rule 2 exception) - throw new ActivityInputValidationError(activityName, inputResult.issues); - } - - // Send the ORIGINAL input — the activity worker parses on receive. - const result = await rawActivity(input); - - const outputResult = await activityDef.output["~standard"].validate(result); - if (outputResult.issues) { - // oxlint-disable-next-line unthrown/no-throw -- sanctioned ValidationError/ApplicationFailure model: terminal failure Temporal must see thrown (CLAUDE.md rule 2 exception) - throw new ActivityOutputValidationError(activityName, outputResult.issues); - } - - return outputResult.value; - }; -} - -/** - * Result-shaped wrapper for activities that declare contract errors. + * Result-shaped wrapper for every activity — declared `errors` map or not. * Classification mirrors the child-workflow API (`classifyChildWorkflowError`): * * - cancellation → `Err(ActivityCancelledError)` (checked first, so a @@ -234,7 +210,14 @@ function makeResultShapedActivity( /** * Map a failure thrown by a workflow-side activity call into the typed error - * union of an errors-declaring activity. + * union of the activity — every activity now, not only ones that declare an + * `errors` map. When the activity declares no `errors`, `activityDef.errors` + * is `undefined` and the rehydration branch below is a deliberate no-op: + * `_internal_rehydrateContractError` returns `undefined` for an `undefined` + * map (see `packages/contract/src/errors.spec.ts`'s "returns undefined when + * no errors are declared or the failure has no type" case), so classification + * falls straight through to the `ActivityError` fallback. Do not reintroduce + * a guard around this call for the errors-less case — it is already handled. */ async function classifyActivityError( activityName: string, @@ -268,5 +251,11 @@ async function classifyActivityError( activityName, `Activity "${activityName}" failed: ${innerMessage}`, inner, + // Retain the value exactly as caught (pre-unwrap) as `originalFailure`, + // alongside the unwrapped `cause` above. `propagateActivityFailure` + // re-raises `originalFailure` so Temporal classifies the workflow + // outcome exactly as it would if this activity call still threw + // directly — see the field's doc comment on `ActivityError`. + error, ); } diff --git a/packages/worker/src/activity-failure.spec.ts b/packages/worker/src/activity-failure.spec.ts new file mode 100644 index 00000000..ee479e53 --- /dev/null +++ b/packages/worker/src/activity-failure.spec.ts @@ -0,0 +1,204 @@ +import { ContractError } from "@temporal-contract/contract/errors"; +import { ApplicationFailure, ActivityFailure, RetryState } from "@temporalio/common"; +import { ErrAsync, OkAsync } from "unthrown"; +import { describe, expect, it } from "vitest"; + +import { propagateActivityFailure } from "./activity-failure.js"; +import { + ActivityCancelledError, + ActivityError, + ChildWorkflowCancelledError, + ChildWorkflowError, + ChildWorkflowNotFoundError, + ContractMisuseError, + WorkflowCancelledError, +} from "./errors.js"; + +describe("propagateActivityFailure", () => { + it("returns the value on Ok", async () => { + await expect(propagateActivityFailure(OkAsync({ ok: true }))).resolves.toEqual({ ok: true }); + }); + + it("rethrows the ORIGINAL ActivityFailure wrapper, not the unwrapped cause", async () => { + // This is the whole point (see activity-failure.ts's doc comment). Real + // ActivityFailure/ApplicationFailure instances, mirroring exactly what + // classifyActivityError constructs: `cause` is the unwrapped + // ApplicationFailure, `originalFailure` is the wrapper Temporal actually + // threw. Rethrowing `cause` instead would hand Temporal a bare + // ApplicationFailure where it previously saw an ActivityFailure — + // changing the client-visible WorkflowFailedError.cause type. + const innerCause = ApplicationFailure.create({ message: "boom", type: "Boom" }); + const wrapper = new ActivityFailure( + "activity failed", + "charge", + "1", + RetryState.MAXIMUM_ATTEMPTS_REACHED, + undefined, + innerCause, + ); + const activityError = new ActivityError( + "charge", + 'Activity "charge" failed: boom', + innerCause, + wrapper, + ); + + await expect(propagateActivityFailure(ErrAsync(activityError))).rejects.toBe(wrapper); + }); + + it("falls back to cause when no originalFailure was preserved", async () => { + const cause = ApplicationFailure.create({ message: "boom", type: "Boom" }); + const activityError = new ActivityError("charge", 'Activity "charge" failed: boom', cause); + + await expect(propagateActivityFailure(ErrAsync(activityError))).rejects.toBe(cause); + }); + + it("rethrows the wrapper itself when neither cause nor originalFailure was preserved", async () => { + // Never lose the error identity: if there is nothing underneath, the + // ActivityError itself is the most informative thing available. + const activityError = new ActivityError("charge", 'Activity "charge" failed: opaque'); + + await expect(propagateActivityFailure(ErrAsync(activityError))).rejects.toBe(activityError); + }); + + it("rethrows the preserved cause for a cancelled activity", async () => { + // ActivityCancelledError has no separate originalFailure: cancellation is + // detected before classifyActivityError's unwrap, so cause already holds + // the pre-unwrap original failure — see the class's doc comment. + const cancelledFailure = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); + const cancelled = new ActivityCancelledError("charge", cancelledFailure); + + await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); + }); + + it("rethrows a cancelled activity's wrapper when no cause was preserved", async () => { + const cancelled = new ActivityCancelledError("charge"); + + await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelled); + }); + + it("rethrows a non-ActivityError error value unchanged", async () => { + const other = new Error("something else"); + await expect(propagateActivityFailure(ErrAsync(other))).rejects.toBe(other); + }); + + it("rethrows the ApplicationFailure cause for a declared ContractError, not the TaggedError wrapper", async () => { + // A declared contract error is ALSO a TaggedError, not a TemporalFailure. + // Rethrown bare it wouldn't stall (declareWorkflow's own catch converts + // any ContractError it sees), but it WOULD misclassify: that catch looks + // the error name up on the workflow's declared errors, not the + // activity's, so the common case (name declared only on the activity) + // produces a misleading "not declared on workflow" failure instead of + // the activity's real error (Fix round 2, Important — see + // activity-failure.ts's doc comment). `cause` is the original + // ApplicationFailure, which is what must escape instead. + const wireFailure = ApplicationFailure.create({ + type: "PaymentDeclined", + message: "Card declined", + nonRetryable: true, + details: [{ reason: "insufficient_funds" }], + }); + const contractError = new ContractError({ + errorName: "PaymentDeclined", + data: { reason: "insufficient_funds" }, + message: "Card declined", + cause: wireFailure, + }); + + expect(contractError).not.toBeInstanceOf(ApplicationFailure); + await expect(propagateActivityFailure(ErrAsync(contractError))).rejects.toBe(wireFailure); + }); + + it("rethrows a ContractError itself when no cause was set", async () => { + const contractError = new ContractError({ + errorName: "PaymentDeclined", + data: undefined, + message: "Card declined", + }); + + await expect(propagateActivityFailure(ErrAsync(contractError))).rejects.toBe(contractError); + }); + + it("rethrows the preserved cause for a failed child workflow", async () => { + // Mirrors ActivityError: `cause` is the unwrapped actionable failure. + const cause = ApplicationFailure.create({ message: "child failed", type: "Boom" }); + const childError = new ChildWorkflowError("processPayment", "Child workflow failed", cause); + + await expect(propagateActivityFailure(ErrAsync(childError))).rejects.toBe(cause); + }); + + it("converts a causeless child workflow error to a terminal ContractMisuseError, not a bare TaggedError rethrow", async () => { + // The three child-workflow.ts sites that construct ChildWorkflowError + // without a cause (input/output/signal-input validation) fire BEFORE any + // Temporal call — there is no pre-existing TemporalFailure to re-raise. + // Rethrowing the bare TaggedError would stall the workflow exactly like + // ChildWorkflowNotFoundError would; it must convert to a terminal + // ApplicationFailure instead (see activity-failure.ts's doc comment). + const childError = new ChildWorkflowError("processPayment", "Child workflow failed"); + + await expect(propagateActivityFailure(ErrAsync(childError))).rejects.toThrow( + ContractMisuseError, + ); + await expect(propagateActivityFailure(ErrAsync(childError))).rejects.toMatchObject({ + message: childError.message, + nonRetryable: true, + }); + }); + + it("rethrows the preserved cause for a cancelled child workflow", async () => { + const cancelledFailure = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); + const cancelled = new ChildWorkflowCancelledError("processPayment", cancelledFailure); + + await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); + }); + + it("rethrows the preserved cause for a cancelled cancellation scope", async () => { + // WorkflowCancelledError from context.cancellableScope/nonCancellableScope + // — mirrors ActivityCancelledError's cause handling. + const cancelledFailure = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); + const cancelled = new WorkflowCancelledError(cancelledFailure); + + await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); + }); + + it("rethrows a cancelled scope's wrapper when no cause was preserved", async () => { + const cancelled = new WorkflowCancelledError(); + + await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelled); + }); + + it("converts a not-found child workflow to a terminal ContractMisuseError, not a bare TaggedError rethrow", async () => { + // ChildWorkflowNotFoundError fires before any Temporal call (the child + // workflow name isn't declared on the target contract) — there is no + // pre-existing TemporalFailure to re-raise. Rethrowing it bare would + // stall the workflow (TaggedError, not TemporalFailure); it must be + // converted to a terminal ApplicationFailure instead. + const notFound = new ChildWorkflowNotFoundError("processPayment", ["processOrder"]); + + await expect(propagateActivityFailure(ErrAsync(notFound))).rejects.toThrow(ContractMisuseError); + await expect(propagateActivityFailure(ErrAsync(notFound))).rejects.toMatchObject({ + message: notFound.message, + nonRetryable: true, + }); + }); + + // Item 8: the defect channel has no dedicated coverage above — every case + // drives ErrAsync/OkAsync. Prove this test discriminates by temporarily + // deleting the `: settled.cause` fallback in activity-failure.ts (hardcode + // `settled.error`) and observing ONLY this test fail; the eight Err-based + // tests above stay green because none of them exercise the Defect branch. + it("classifies a defect's cause through the same chain as an Err — not settled.error", async () => { + const cause = ApplicationFailure.create({ message: "boom", type: "Boom" }); + const activityError = new ActivityError("charge", 'Activity "charge" failed: boom', cause); + + // A genuine defect: something inside the AsyncResult pipeline THREW + // `activityError` rather than returning `Err(activityError)`. A defect + // has no public constructor — a throw inside a combinator is the only + // way to mint one (see unthrown's `defectOf` test helper). + const defect = OkAsync(0).map(() => { + throw activityError; + }); + + await expect(propagateActivityFailure(defect)).rejects.toBe(cause); + }); +}); diff --git a/packages/worker/src/activity-failure.ts b/packages/worker/src/activity-failure.ts new file mode 100644 index 00000000..753e1588 --- /dev/null +++ b/packages/worker/src/activity-failure.ts @@ -0,0 +1,162 @@ +import { ContractError } from "@temporal-contract/contract/errors"; +import type { AsyncResult } from "unthrown"; + +import { + ActivityCancelledError, + ActivityError, + ChildWorkflowCancelledError, + ChildWorkflowError, + ChildWorkflowNotFoundError, + ContractMisuseError, + WorkflowCancelledError, +} from "./errors.js"; + +/** + * Await an activity call and return its value, re-raising the failure so + * **Temporal** decides the workflow's fate — the workflow-side equivalent of + * "let it fail". + * + * Use this instead of unthrown's `.getOrThrow()`. `getOrThrow` throws the + * `ActivityError` / `ActivityCancelledError` *wrapper* itself, which is a + * `TaggedError` and NOT a `TemporalFailure`. Temporal treats a + * non-`TemporalFailure` thrown from workflow code as a workflow-TASK failure + * and retries it indefinitely, so the workflow never fails — it stalls until + * its execution timeout. This helper instead re-raises the *original* + * Temporal failure that `classifyActivityError` observed, which is exactly + * what would have escaped the workflow before activity calls returned + * `AsyncResult`. + * + * Two things are preserved on `ActivityError`, and they are NOT + * interchangeable: + * - `cause` — the *unwrapped* actionable failure (Temporal's + * `ActivityFailure` wrapper seen through). This is documented, caller-facing + * behavior that existing consumers narrow on, and this helper does not + * change it. + * - `originalFailure` — the value exactly as `classifyActivityError` caught + * it, *before* that unwrap (typically the `ActivityFailure` wrapper + * itself). This helper re-raises `originalFailure` (falling back to + * `cause`, then the wrapper) so the failure Temporal observes here is + * byte-for-byte what it would have observed had the activity call thrown + * directly — rethrowing `cause` instead would hand Temporal a bare + * `ApplicationFailure` where it previously saw an `ActivityFailure`, + * changing what a caller further up (e.g. the client's + * `WorkflowFailedError.cause`) sees. + * + * `ActivityCancelledError` has no separate `originalFailure`: cancellation is + * detected *before* the unwrap, so its `cause` already holds the pre-unwrap + * original failure. + * + * A **declared** contract error (`ContractError`, from an activity's `errors` + * map) is also a `TaggedError`, not a `TemporalFailure` — but, unlike + * `ActivityError`/`ActivityCancelledError`, rethrowing it bare does NOT + * stall the workflow: `declareWorkflow`'s own top-level catch (`workflow.ts`) + * recognizes `error instanceof ContractError` by name and converts it via + * `contractErrorToApplicationFailure` before Temporal ever sees it. What that + * fallback conversion produces, though, is *wrong* here: it looks the error + * name up on the *workflow's* declared `errors` map — not the *activity's*, + * which is what this `ContractError` was actually rehydrated against. When + * the name isn't also declared on the workflow (the common case, since the + * error is declared on the activity), the workflow still fails terminally, + * but with a misleading `ContractErrorDataValidationError: Error "X" is not + * declared on workflow "…"` instead of the activity's real, typed failure. + * This branch exists to fix that *misclassification*, not to prevent a + * stall: it re-raises `ContractError.cause`, the original `ApplicationFailure` + * Temporal actually put on the wire, so the workflow fails with the real + * failure instead of a confusing wrong-map error message. + * + * **Fidelity nuance, stated plainly:** for a declared error this means the + * client sees `WorkflowFailedError.cause` as a bare `ApplicationFailure`, + * never wrapped in Temporal's `ActivityFailure` — unlike the `ActivityError` + * path above, which preserves the `ActivityFailure` wrapper exactly. This is + * a deliberate *asymmetry*, not an inevitability: `classifyActivityError` + * still holds the original wrapper (its `error` parameter — the same value + * it hands `ActivityError` as `originalFailure`) at the point it builds a + * `ContractError`, but `_internal_rehydrateContractError` is only handed the + * already-unwrapped failure, so the wrapper is never threaded through the + * rehydration path. A parallel `originalFailure`-style retention on + * `ContractError` would close this gap; that was deliberately left out of + * this task's scope. + * + * A failure with nothing preserved at all rethrows the wrapper, so the error + * identity is never lost. + * + * **Not just activity calls.** The same non-`TemporalFailure`-stall hazard + * applies to `context.executeChildWorkflow` / `context.startChildWorkflow` + * (`ChildWorkflowError`, `ChildWorkflowCancelledError`) and to + * `context.cancellableScope` / `context.nonCancellableScope` + * (`WorkflowCancelledError`, whose `cause` holds the original + * `CancelledFailure`). This helper accepts any of those too — `E` is + * intentionally unconstrained so a bare `throw error` at the bottom doesn't + * quietly stall the workflow for a union this module didn't anticipate. + * + * `ChildWorkflowCancelledError` mirrors `ActivityCancelledError`: every + * construction site (`classifyChildWorkflowError`) supplies `cause` as the + * pre-unwrap cancellation failure Temporal produced, so it is always safe to + * re-raise. `ChildWorkflowError`, however, does NOT uniformly mirror + * `ActivityError`: `classifyChildWorkflowError`'s own construction sites do + * set `cause` to the unwrapped actionable failure, but three OTHER + * construction sites in `child-workflow.ts` — input validation, output + * validation, and signal-input validation — build a `ChildWorkflowError` + * with no `cause` at all, because those failures are detected locally + * (a schema mismatch) before any Temporal call happens, so there is no + * Temporal-observed failure to carry. Re-raising the bare `TaggedError` in + * that case would reproduce the exact stall this helper exists to prevent, + * so when `cause` is absent this helper converts it to a `ContractMisuseError` + * instead — the same treatment `ChildWorkflowNotFoundError` gets below. + * + * `ChildWorkflowNotFoundError` is the other case with no `cause` to + * rethrow: it fires *before* any Temporal call, when the target contract + * doesn't declare the child workflow name at all — a deterministic + * programmer bug, not a Temporal-observed failure. It is converted to a + * `ContractMisuseError` (a non-retryable `ApplicationFailure`) instead, so + * it still fails the workflow terminally rather than stalling it. + */ +export async function propagateActivityFailure(result: AsyncResult): Promise { + const settled = await result; + if (settled.isOk()) { + return settled.value; + } + + // A `Defect` is an unmodeled failure (a bug this library didn't + // anticipate). It falls into the same classification chain below rather + // than being rethrown unclassified: a defect's `cause` can itself be one + // of the shapes handled here (e.g. an `ActivityError` thrown instead of + // returned), and that shape deserves the same re-raise treatment whether + // it arrived via `Err` or `Defect`. + const error: unknown = settled.isErr() ? settled.error : settled.cause; + + if (error instanceof ActivityError) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: Temporal must see the original failure (pre-unwrap) to classify the workflow outcome exactly as it would have if the activity call still threw directly + throw error.originalFailure ?? error.cause ?? error; + } + if (error instanceof ActivityCancelledError) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: `cause` already holds the pre-unwrap original failure for cancellation (see ActivityCancelledError's doc comment) + throw error.cause ?? error; + } + if (error instanceof ContractError) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: re-raise the original ApplicationFailure so the workflow fails with the activity's real declared error, not declareWorkflow's misleading "not declared on workflow" fallback (see doc comment) + throw error.cause ?? error; + } + if (error instanceof ChildWorkflowCancelledError) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: mirrors ActivityCancelledError — `cause` is the pre-unwrap cancellation failure Temporal originally produced + throw error.cause ?? error; + } + if (error instanceof ChildWorkflowError) { + if (error.cause !== undefined) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: mirrors ActivityError — `cause` is the unwrapped actionable failure Temporal originally produced + throw error.cause; + } + // oxlint-disable-next-line unthrown/no-throw -- sanctioned ValidationError/ApplicationFailure model: the input/output/signal-input validation sites in child-workflow.ts construct ChildWorkflowError with no cause at all (a deterministic contract-misuse bug, not a Temporal-observed failure), so there is no pre-existing TemporalFailure to re-raise (CLAUDE.md rule 2 exception) + throw new ContractMisuseError(error.message); + } + if (error instanceof WorkflowCancelledError) { + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: `cause` holds the original CancelledFailure the scope observed (see cancellableScope/nonCancellableScope) + throw error.cause ?? error; + } + if (error instanceof ChildWorkflowNotFoundError) { + // oxlint-disable-next-line unthrown/no-throw -- sanctioned ValidationError/ApplicationFailure model: this is a deterministic contract-misuse bug, not a Temporal-observed failure, so there is no pre-existing TemporalFailure to re-raise (CLAUDE.md rule 2 exception) + throw new ContractMisuseError(error.message); + } + // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: an unmodeled error/defect value is rethrown unchanged + throw error; +} diff --git a/packages/worker/src/cancellation.ts b/packages/worker/src/cancellation.ts index 77617b35..3fb364c3 100644 --- a/packages/worker/src/cancellation.ts +++ b/packages/worker/src/cancellation.ts @@ -32,9 +32,14 @@ import { makeAsyncResult } from "./internal.js"; * @example * ```ts * import { P } from "unthrown"; + * import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; * + * // `fn`'s return value becomes the scope's `T` verbatim — an un-awaited + * // `context.activities.processStep(...)` would make `T` the AsyncResult + * // itself (it has no `isOk`/`isErr`/`.value`), not the activity's output. + * // `propagateActivityFailure` awaits it and hands the scope a plain value. * const result = await context.cancellableScope(async () => { - * return await context.activities.processStep(...); + * return await propagateActivityFailure(context.activities.processStep(...)); * }); * * result.match({ @@ -44,7 +49,8 @@ import { makeAsyncResult } from "./internal.js"; * // error instanceof WorkflowCancelledError — graceful exit * }), * defect: (cause) => { - * // a non-cancellation failure thrown inside the scope (a bug) + * // a non-cancellation failure thrown inside the scope (a bug) — or, + * // via propagateActivityFailure, a non-cancellation activity failure * }, * }); * ``` @@ -85,9 +91,18 @@ export function cancellableScope( * * @example * ```ts - * await context.nonCancellableScope(async () => { - * await context.activities.releaseResources(...); + * // Capture the scope's OWN AsyncResult — a bare `await` would silently + * // discard a defect thrown inside the callback, along with the activity's + * // own un-awaited AsyncResult if `fn` returned it directly. + * const released = await context.nonCancellableScope(async () => { + * const result = await context.activities.releaseResources(...); + * if (result.isErr()) { + * // best-effort cleanup — log and continue regardless + * } * }); + * if (released.isDefect()) { + * throw released.cause; // a genuine bug in cleanup, not a cancel + * } * ``` */ export function nonCancellableScope( diff --git a/packages/worker/src/error-tags.ts b/packages/worker/src/error-tags.ts index bb75cb50..1ae6fbfd 100644 --- a/packages/worker/src/error-tags.ts +++ b/packages/worker/src/error-tags.ts @@ -17,10 +17,10 @@ * are discriminated by `failure.type`, not `_tag`, and have no constant here. */ -/** `_tag` of `ActivityError` — an errors-declaring activity failed for an undeclared reason. */ +/** `_tag` of `ActivityError` — an activity call failed for a reason other than a declared contract error. */ export const ACTIVITY_ERROR_TAG = "@temporal-contract/ActivityError"; -/** `_tag` of `ActivityCancelledError` — a call to an errors-declaring activity was cancelled. */ +/** `_tag` of `ActivityCancelledError` — a call to an activity was cancelled. */ export const ACTIVITY_CANCELLED_ERROR_TAG = "@temporal-contract/ActivityCancelledError"; /** `_tag` of `ActivityDefinitionNotFoundError` — an implementation was supplied for an undeclared activity. */ diff --git a/packages/worker/src/errors.ts b/packages/worker/src/errors.ts index 94f4b4a3..0e65680e 100644 --- a/packages/worker/src/errors.ts +++ b/packages/worker/src/errors.ts @@ -316,24 +316,36 @@ export class ContractMisuseError extends ValidationError { * failure (Temporal's `ActivityFailure` wrapper is seen through), so callers * can branch on the failure category in one step. * - * Only activities that declare an `errors` map surface this — activities - * without declared errors keep Temporal's native throwing behavior. + * Every activity call surfaces this — the workflow-side call convention no + * longer depends on whether the contract declares an `errors` map; only the + * error channel's declared-error members do. + * + * `originalFailure` is a second, separate retention: the value exactly as it + * was caught, *before* `classifyActivityError` unwrapped it into `cause` + * (typically Temporal's `ActivityFailure` wrapper). `cause`'s unwrapping is + * documented, caller-facing behavior and stays as-is — `originalFailure` + * exists purely so {@link propagateActivityFailure} can re-raise the exact + * failure Temporal originally produced, without changing what `cause` means. + * Unset when there is no separate wrapper to retain (e.g. the input/output + * validation branches, where `cause` is already the terminal failure). */ export class ActivityError extends TaggedError(ACTIVITY_ERROR_TAG, { name: "ActivityError", })<{ activityName: string; cause?: unknown; + originalFailure?: unknown; }> { - constructor(activityName: string, message: string, cause?: unknown) { - super({ activityName, cause }); + constructor(activityName: string, message: string, cause?: unknown, originalFailure?: unknown) { + super({ activityName, cause, originalFailure }); this.message = message; } } /** - * Discriminated variant surfaced when a call to an errors-declaring activity - * was cancelled (the workflow itself, or an enclosing cancellation scope). + * Discriminated variant surfaced when a call to an activity was cancelled + * (the workflow itself, or an enclosing cancellation scope) — every activity + * call rides this branch now, not only ones that declare an `errors` map. * Detected via `@temporalio/workflow`'s `isCancellation(...)`. * * A sibling of {@link ActivityError} rather than a subclass, for the same @@ -346,6 +358,11 @@ export class ActivityError extends TaggedError(ACTIVITY_ERROR_TAG, { * workflow complete as `Completed` instead of `Cancelled`. When the workflow * should honor the cancellation request, re-raise it with * {@link rethrowCancellation}. + * + * Unlike {@link ActivityError}, `cause` here is already the value exactly as + * caught (`classifyActivityError` checks cancellation *before* unwrapping + * `ActivityFailure`) — so there is no separate `originalFailure` to retain; + * {@link propagateActivityFailure} re-raises `cause` directly. */ export class ActivityCancelledError extends TaggedError(ACTIVITY_CANCELLED_ERROR_TAG, { name: "ActivityCancelledError", @@ -484,9 +501,32 @@ export class WorkflowCancelledError extends TaggedError(WORKFLOW_CANCELLED_ERROR * * @example * ```ts - * const result = await context.cancellableScope(() => context.activities.processStep(args)); + * // `fn`'s return value becomes the scope's `T` verbatim, so await and + * // narrow the activity's own AsyncResult HERE, inside the callback — + * // returning it un-awaited would make `T` the AsyncResult itself, which + * // has no `isOk`/`isErr`/`.value`. + * const result = await context.cancellableScope(async () => { + * const step = await context.activities.processStep(args); + * if (step.isDefect()) { + * throw step.cause; + * } + * return step.isOk(); + * }); + * if (result.isDefect()) { + * throw result.cause; // a genuine bug thrown inside the scope, not a cancel + * } * if (result.isErr()) { - * await context.nonCancellableScope(() => context.activities.releaseResources(args)); + * // Capture nonCancellableScope's OWN AsyncResult too — a bare `await` + * // would silently discard a defect thrown during cleanup. + * const released = await context.nonCancellableScope(async () => { + * const step = await context.activities.releaseResources(args); + * if (step.isErr()) { + * // best-effort cleanup — log and continue regardless + * } + * }); + * if (released.isDefect()) { + * throw released.cause; // a genuine bug in cleanup, not a cancel + * } * // Honor the cancellation instead of completing normally: * rethrowCancellation(result.error); * } diff --git a/packages/worker/src/workflow.ts b/packages/worker/src/workflow.ts index ca0bbc0e..314421ae 100644 --- a/packages/worker/src/workflow.ts +++ b/packages/worker/src/workflow.ts @@ -105,6 +105,14 @@ export { // re-raises the original CancelledFailure so the execution ends `Cancelled`. export { rethrowCancellation } from "./errors.js"; +// Activity-failure re-raise helper: the workflow-side equivalent of "let it +// fail" for any activity call's `AsyncResult` — declared `errors` map or not. +// Re-raises the original Temporal failure (not the +// `ActivityError`/`ActivityCancelledError` wrapper, which isn't a +// `TemporalFailure`) so Temporal classifies the workflow outcome exactly as +// it would have if the activity call still threw directly. +export { propagateActivityFailure } from "./activity-failure.js"; + // Literal-typed `_tag` constants for this package's tagged errors, so // consumers can `P.tag(ACTIVITY_ERROR_TAG)` without hand-writing the // namespaced strings (mirrors the contract package's error-tags module). @@ -136,8 +144,12 @@ export type { } from "./handlers.js"; // Public activity-inference types: the workflow-side shape of a single -// activity and of the full `context.activities` map. +// activity and of the full `context.activities` map, plus the error union +// (`ActivityErrorsFor`) that `WorkflowInferActivity` uses for its `AsyncResult` +// error channel — exported so consumers can name it directly, e.g. to write a +// helper generic over an activity's error type. export type { + ActivityErrorsFor, WorkflowInferActivity, WorkflowInferWorkflowContextActivities, } from "./activities-proxy.js"; @@ -185,11 +197,16 @@ export type { TypedContinueAsNewOptions } from "./internal.js"; * // context.activities: typed activities (workflow + global) * // context.info: WorkflowInfo * + * // Every activity call returns an AsyncResult — narrow it or use + * // `propagateActivityFailure` to let Temporal decide the outcome. * const inventory = await context.activities.validateInventory({ * orderId: args.orderId, * }); + * if (inventory.isErr()) { + * return { orderId: args.orderId, status: 'out_of_stock' }; + * } * - * if (!inventory.available) { + * if (!inventory.value.available) { * return { orderId: args.orderId, status: 'out_of_stock' }; * } * @@ -197,11 +214,14 @@ export type { TypedContinueAsNewOptions } from "./internal.js"; * customerId: args.customerId, * amount: 100, * }); + * if (payment.isErr()) { + * return { orderId: args.orderId, status: 'failed' }; + * } * * return { * orderId: args.orderId, - * status: payment.success ? 'success' : 'failed', - * transactionId: payment.transactionId, + * status: payment.value.success ? 'success' : 'failed', + * transactionId: payment.value.transactionId, * }; * }, * }); @@ -802,18 +822,41 @@ export type WorkflowContext< * * implementation: async (context, args) => { * const result = await context.cancellableScope(async () => { - * return context.activities.processStep(args); + * // `fn`'s return value becomes the scope's `T` verbatim, so await and + * // narrow the activity's own AsyncResult HERE, inside the callback. + * // `AsyncResult` is deliberately not a full `PromiseLike` (no + * // `.catch`/`.finally`), so returning an un-awaited activity call + * // would make `T` the un-awaited `AsyncResult` itself — which has no + * // `isOk`/`isErr`/`.value` (only the plain `Result` you get by + * // awaiting does). + * const step = await context.activities.processStep(args); + * if (step.isDefect()) { + * throw step.cause; // an unmodeled bug — surfaces as the scope's own defect + * } + * return step.isOk() ? { status: "ok" as const } : { status: "failed" as const }; * }); * - * if (result.isErr() && result.error instanceof WorkflowCancelledError) { - * // workflow was cancelled — perform cleanup that must not be cancelled: - * await context.nonCancellableScope(async () => { - * await context.activities.releaseResources(args); + * if (result.isDefect()) { + * throw result.cause; // a genuine bug thrown inside the scope, not a cancel + * } + * if (result.isErr()) { + * // The scope itself was cancelled — perform cleanup that must not be + * // cancelled. Capture nonCancellableScope's OWN AsyncResult too — a + * // bare `await` here would silently discard a defect thrown inside + * // the cleanup callback. + * const released = await context.nonCancellableScope(async () => { + * const step = await context.activities.releaseResources(args); + * if (step.isErr()) { + * // best-effort cleanup — log and continue regardless + * } * }); + * if (released.isDefect()) { + * throw released.cause; // a genuine bug in cleanup, not a cancel + * } * return { status: "cancelled" }; * } * - * return { status: "ok" }; + * return result.value; * } * ``` */