From bdeb0c67c75dc29e5edad662cb57f8eeec5ae525 Mon Sep 17 00:00:00 2001 From: wangbill Date: Fri, 16 Oct 2026 15:54:39 -0400 Subject: [PATCH 01/11] feat: add bounded payload storage contracts and processing Phase 1 of 6: add provider-independent payload contracts, explicit protobuf mapping, UTF-8 size limits, bounded I/O and permanent worker upload failure conversion. Reserve the 0.4.1 core API version without enabling transport or cleanup yet. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7 --- CHANGELOG.md | 4 + package-lock.json | 4 +- packages/azure-functions-durable/package.json | 2 +- packages/durabletask-js/package.json | 2 +- packages/durabletask-js/src/index.ts | 6 + .../src/payloads/payload-processor.ts | 196 ++++++++++ .../src/payloads/payload-store.ts | 39 ++ .../src/payloads/run-bounded.ts | 32 ++ packages/durabletask-js/test/payloads.spec.ts | 334 ++++++++++++++++++ 9 files changed, 615 insertions(+), 4 deletions(-) create mode 100644 packages/durabletask-js/src/payloads/payload-processor.ts create mode 100644 packages/durabletask-js/src/payloads/payload-store.ts create mode 100644 packages/durabletask-js/src/payloads/run-bounded.ts create mode 100644 packages/durabletask-js/test/payloads.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ac550a..5429baa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ### New +- Add payload storage contracts and bounded protobuf payload processing, with inclusive UTF-8 + thresholds, explicit storage/size failures, and non-retriable worker upload-failure conversion. + This is phase 1 of the external-payload feature; transport, providers, and cleanup follow separately. + Reserve core `0.4.1` for the new APIs; do not publish partial feature phases. - Expose readonly `OrchestrationContext.sourceInstanceId` from the existing protobuf request properties on initial execution and replay, independently of parent identity. Strings are preserved verbatim; null or missing metadata returns `undefined`. diff --git a/package-lock.json b/package-lock.json index 8ef8ff3e..e1d04436 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7600,7 +7600,7 @@ "dependencies": { "@azure/functions": "^4.16.1", "@grpc/grpc-js": "^1.14.4", - "@microsoft/durabletask-js": "0.4.0" + "@microsoft/durabletask-js": "0.4.1" }, "devDependencies": { "@types/jest": "^29.5.1", @@ -7623,7 +7623,7 @@ }, "packages/durabletask-js": { "name": "@microsoft/durabletask-js", - "version": "0.4.0", + "version": "0.4.1", "license": "MIT", "dependencies": { "@grpc/grpc-js": "^1.14.4", diff --git a/packages/azure-functions-durable/package.json b/packages/azure-functions-durable/package.json index d4cf0059..20e54bec 100644 --- a/packages/azure-functions-durable/package.json +++ b/packages/azure-functions-durable/package.json @@ -68,7 +68,7 @@ "dependencies": { "@azure/functions": "^4.16.1", "@grpc/grpc-js": "^1.14.4", - "@microsoft/durabletask-js": "0.4.0" + "@microsoft/durabletask-js": "0.4.1" }, "peerDependencies": { "@azure/identity": "^4.0.0" diff --git a/packages/durabletask-js/package.json b/packages/durabletask-js/package.json index 598a96c9..1b33d65c 100644 --- a/packages/durabletask-js/package.json +++ b/packages/durabletask-js/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/durabletask-js", - "version": "0.4.0", + "version": "0.4.1", "description": "A Durable Task Javascript SDK compatible with Dapr Workflow and its underlying Durable Task engine", "main": "./dist/index.js", "types": "./dist/index.d.ts", diff --git a/packages/durabletask-js/src/index.ts b/packages/durabletask-js/src/index.ts index 58e0275d..5e22ec89 100644 --- a/packages/durabletask-js/src/index.ts +++ b/packages/durabletask-js/src/index.ts @@ -2,6 +2,12 @@ // Licensed under the MIT License. // Client and Worker +export { + PayloadStore, + PayloadStorageOptions, + PayloadStorageException, + PayloadDeleteOutcome, +} from "./payloads/payload-store"; export { TaskHubGrpcClient, TaskHubGrpcClientOptions, MetadataGenerator } from "./client/client"; export { TaskHubGrpcWorker, TaskHubGrpcWorkerOptions } from "./worker/task-hub-grpc-worker"; export { ConcurrencyOptions } from "./worker/concurrency-options"; diff --git a/packages/durabletask-js/src/payloads/payload-processor.ts b/packages/durabletask-js/src/payloads/payload-processor.ts new file mode 100644 index 00000000..235f6d3f --- /dev/null +++ b/packages/durabletask-js/src/payloads/payload-processor.ts @@ -0,0 +1,196 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import * as pb from "../proto/orchestrator_service_pb"; +import { newFailureDetails } from "../utils/pb-helper.util"; +import { PayloadStorageException, PayloadStorageOptions } from "./payload-store"; +import { runBounded } from "./run-bounded"; + +/** Explicit protobuf field mapping, aligned with the .NET AzureBlobPayloads SideCar interceptor. */ +export class PayloadProcessor { + private readonly _threshold: number; + private readonly _maximum: number; + + constructor(private readonly _options: PayloadStorageOptions) { + this._threshold = _options.thresholdBytes ?? 256 * 1024; + this._maximum = _options.maxPayloadBytes ?? 10 * 1024 * 1024; + if (!Number.isSafeInteger(this._threshold) || this._threshold < 0 || this._threshold > 1024 * 1024) { + throw new RangeError("thresholdBytes must be an integer between 0 and 1 MiB."); + } + if (!Number.isSafeInteger(this._maximum) || this._maximum <= 0) { + throw new RangeError("maxPayloadBytes must be a positive safe integer."); + } + if (!_options.store) throw new TypeError("A payload store is required."); + } + + async externalize(message: unknown, signal?: AbortSignal): Promise { + const fields: (StringValue | undefined)[] = []; + if ( + message instanceof pb.CreateInstanceRequest || + message instanceof pb.RaiseEventRequest || + message instanceof pb.SignalEntityRequest + ) { + fields.push(message.getInput()); + } else if (message instanceof pb.TerminateRequest) { + fields.push(message.getOutput()); + } else if ( + message instanceof pb.SuspendRequest || + message instanceof pb.ResumeRequest || + message instanceof pb.RewindInstanceRequest + ) { + fields.push(message.getReason()); + } else if (message instanceof pb.ActivityResponse) { + fields.push(message.getResult()); + } else if (message instanceof pb.OrchestratorResponse) { + fields.push(message.getCustomstatus()); + for (const action of message.getActionsList()) { + fields.push( + action.getScheduletask()?.getInput(), + action.getCreatesuborchestration()?.getInput(), + action.getCompleteorchestration()?.getResult(), + action.getCompleteorchestration()?.getDetails(), + action.getTerminateorchestration()?.getReason(), + action.getSendevent()?.getData(), + action.getSendentitymessage()?.getEntityoperationsignaled()?.getInput(), + action.getSendentitymessage()?.getEntityoperationcalled()?.getInput(), + ); + for (const event of action.getRewindorchestration()?.getNewhistoryList() ?? []) { + this._historyFields(event, fields); + } + } + } else if (message instanceof pb.EntityBatchResult) { + fields.push(message.getEntitystate()); + for (const result of message.getResultsList()) fields.push(result.getSuccess()?.getResult()); + for (const action of message.getActionsList()) { + fields.push(action.getSendsignal()?.getInput(), action.getStartneworchestration()?.getInput()); + } + } else if (message instanceof pb.EntityBatchRequest) { + fields.push(message.getEntitystate()); + for (const operation of message.getOperationsList()) fields.push(operation.getInput()); + } else if (message instanceof pb.EntityRequest) { + fields.push(message.getEntitystate()); + } + const operations: (() => Promise)[] = []; + for (const field of fields) { + const value = field?.getValue(); + if (!field || !value) continue; + const size = Buffer.byteLength(value, "utf8"); + if (size < this._threshold) continue; + operations.push(async () => { + if (size > this._maximum) { + throw new PayloadStorageException( + `Payload size ${size} bytes exceeds the configured maximum of ${this._maximum} bytes.`, + ); + } + field.setValue(await this._options.store.upload(value, signal)); + }); + } + try { + await runBounded(operations, signal); + } catch (error) { + if ( + !isPermanentStorageFailure(error) || + !(message instanceof pb.ActivityResponse || message instanceof pb.OrchestratorResponse) + ) + throw error; + const details = newFailureDetails(error); + details.setIsnonretriable(true); + if (message instanceof pb.ActivityResponse) { + message.clearResult(); + message.setFailuredetails(details); + } else { + message.clearCustomstatus(); + message.setIspartial(false); + message.clearChunkindex(); + message.setActionsList([ + new pb.OrchestratorAction().setCompleteorchestration( + new pb.CompleteOrchestrationAction() + .setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED) + .setFailuredetails(details), + ), + ]); + } + } + } + + async resolve(message: unknown, signal?: AbortSignal): Promise { + const fields: (StringValue | undefined)[] = []; + this._responseFields(message, fields); + const operations: (() => Promise)[] = []; + for (const field of fields) { + const value = field?.getValue(); + if (field && value && this._options.store.isKnownPayloadToken(value)) { + operations.push(async () => { + field.setValue(await this._options.store.download(value, signal)); + }); + } + } + await runBounded(operations, signal); + } + + private _responseFields(message: unknown, fields: (StringValue | undefined)[]): void { + if (message instanceof pb.GetInstanceResponse) { + this._stateFields(message.getOrchestrationstate(), fields); + } else if (message instanceof pb.QueryInstancesResponse) { + for (const state of message.getOrchestrationstateList()) this._stateFields(state, fields); + } else if (message instanceof pb.HistoryChunk) { + for (const event of message.getEventsList()) this._historyFields(event, fields); + } else if (message instanceof pb.GetEntityResponse) { + fields.push(message.getEntity()?.getSerializedstate()); + } else if (message instanceof pb.QueryEntitiesResponse) { + for (const entity of message.getEntitiesList()) fields.push(entity.getSerializedstate()); + } else if (message instanceof pb.WorkItem) { + this._responseFields(message.getActivityrequest(), fields); + this._responseFields(message.getOrchestratorrequest(), fields); + this._responseFields(message.getEntityrequest(), fields); + this._responseFields(message.getEntityrequestv2(), fields); + } else if (message instanceof pb.ActivityRequest) { + fields.push(message.getInput()); + } else if (message instanceof pb.OrchestratorRequest) { + for (const event of [...message.getPasteventsList(), ...message.getNeweventsList()]) { + this._historyFields(event, fields); + } + } else if (message instanceof pb.EntityBatchRequest) { + fields.push(message.getEntitystate()); + for (const operation of message.getOperationsList()) fields.push(operation.getInput()); + } else if (message instanceof pb.EntityRequest) { + fields.push(message.getEntitystate()); + for (const event of message.getOperationrequestsList()) this._historyFields(event, fields); + } + } + + private _stateFields(state: pb.OrchestrationState | undefined, fields: (StringValue | undefined)[]): void { + if (state) fields.push(state.getInput(), state.getOutput(), state.getCustomstatus()); + } + + private _historyFields(event: pb.HistoryEvent, fields: (StringValue | undefined)[]): void { + fields.push( + event.getExecutionstarted()?.getInput(), + event.getExecutioncompleted()?.getResult(), + event.getExecutionterminated()?.getInput(), + event.getTaskscheduled()?.getInput(), + event.getTaskcompleted()?.getResult(), + event.getSuborchestrationinstancecreated()?.getInput(), + event.getSuborchestrationinstancecompleted()?.getResult(), + event.getEventsent()?.getInput(), + event.getEventraised()?.getInput(), + event.getGenericevent()?.getData(), + event.getContinueasnew()?.getInput(), + event.getExecutionsuspended()?.getInput(), + event.getExecutionresumed()?.getInput(), + event.getExecutionrewound()?.getReason(), + event.getEntityoperationsignaled()?.getInput(), + event.getEntityoperationcalled()?.getInput(), + event.getEntityoperationcompleted()?.getOutput(), + ); + this._stateFields(event.getHistorystate()?.getOrchestrationstate(), fields); + } +} + +export function isPermanentStorageFailure(error: unknown): error is Error { + if (error instanceof PayloadStorageException) return true; + if (!(error instanceof Error) || !("statusCode" in error)) return false; + const status = error.statusCode; + return typeof status === "number" && status >= 400 && status < 500 && status !== 408 && status !== 429; +} diff --git a/packages/durabletask-js/src/payloads/payload-store.ts b/packages/durabletask-js/src/payloads/payload-store.ts new file mode 100644 index 00000000..2eafae82 --- /dev/null +++ b/packages/durabletask-js/src/payloads/payload-store.ts @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +/** Confirmed results of a conditional, ownership-checked payload deletion. */ +export enum PayloadDeleteOutcome { + Unspecified = 0, + Deleted = 1, + AlreadyAbsent = 2, + NotStoreOwned = 3, +} + +/** Out-of-band storage for serialized UTF-8 payloads. Implementations must honor cancellation. */ +export abstract class PayloadStore { + abstract upload(payload: string, signal?: AbortSignal): Promise; + abstract download(token: string, signal?: AbortSignal): Promise; + abstract isKnownPayloadToken(value: string): boolean; + + /** Optional cleanup support. Never delete a blob without verifying ownership and its ETag. */ + async delete(_token: string, _signal?: AbortSignal): Promise { + throw new Error("This payload store does not support deletion."); + } +} + +/** A permanent payload storage or size-limit failure. */ +export class PayloadStorageException extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = "PayloadStorageException"; + } +} + +/** Opt-in payload externalization. Configure the same store on all clients and workers. */ +export interface PayloadStorageOptions { + store: PayloadStore; + /** Inclusive serialized UTF-8 size threshold. Defaults to 256 KiB; maximum 1 MiB. */ + thresholdBytes?: number; + /** Maximum serialized size of one externalized payload. Defaults to 10 MiB. */ + maxPayloadBytes?: number; +} diff --git a/packages/durabletask-js/src/payloads/run-bounded.ts b/packages/durabletask-js/src/payloads/run-bounded.ts new file mode 100644 index 00000000..f6646f61 --- /dev/null +++ b/packages/durabletask-js/src/payloads/run-bounded.ts @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +/** Stops dispatch on failure/cancellation, drains started I/O, then throws the first message-order failure. */ +export async function runBounded(operations: (() => Promise)[], signal?: AbortSignal): Promise { + const active = new Set>(); + const failures = new Map(); + let cancellationPreventedDispatch = false; + for (let ordinal = 0; ordinal < operations.length; ordinal++) { + if (active.size >= 8) await Promise.race(active); + if (failures.size) break; + if (signal?.aborted) { + cancellationPreventedDispatch = true; + break; + } + const operation = operations[ordinal]; + const tracked = (async () => { + try { + await operation(); + } catch (error) { + failures.set(ordinal, error); + } + })(); + active.add(tracked); + void tracked.then(() => active.delete(tracked)); + // Observe synchronously rejected operations before claiming the next dispatch. + await Promise.resolve(); + } + await Promise.all(active); + if (failures.size) throw failures.get(Math.min(...failures.keys())); + if (cancellationPreventedDispatch) signal?.throwIfAborted(); +} diff --git a/packages/durabletask-js/test/payloads.spec.ts b/packages/durabletask-js/test/payloads.spec.ts new file mode 100644 index 00000000..96b8580c --- /dev/null +++ b/packages/durabletask-js/test/payloads.spec.ts @@ -0,0 +1,334 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { StringValue, Int32Value } from "google-protobuf/google/protobuf/wrappers_pb"; +import * as pb from "../src/proto/orchestrator_service_pb"; +import { PayloadProcessor } from "../src/payloads/payload-processor"; +import { PayloadStore, PayloadStorageException } from "../src/payloads/payload-store"; +import { runBounded } from "../src/payloads/run-bounded"; + +class MemoryPayloadStore extends PayloadStore { + readonly values = new Map(); + uploads = 0; + async upload(value: string): Promise { + const token = `test:${++this.uploads}`; + this.values.set(token, value); + return token; + } + async download(token: string): Promise { + const value = this.values.get(token); + if (value === undefined) throw new Error("Missing payload"); + return value; + } + isKnownPayloadToken(value: string): boolean { + return value.startsWith("test:"); + } +} + +const text = (value: string) => new StringValue().setValue(value); + +describe("externalized payloads", () => { + let store: MemoryPayloadStore; + let processor: PayloadProcessor; + beforeEach(() => { + store = new MemoryPayloadStore(); + processor = new PayloadProcessor({ store }); + }); + + it.each([ + ["a".repeat(262143), false], + ["a".repeat(262144), true], + ["a".repeat(262145), true], + ["\u00e9".repeat(131072), true], + ["\ud83d\ude00".repeat(65536), true], + ["", false], + ])("uses the inclusive UTF-8 threshold (%#)", async (value, externalized) => { + const request = new pb.CreateInstanceRequest().setInput(text(value)); + await processor.externalize(request); + expect(store.uploads).toBe(externalized ? 1 : 0); + if (externalized) { + const response = new pb.GetInstanceResponse().setOrchestrationstate( + new pb.OrchestrationState().setInput(request.getInput()), + ); + await processor.resolve(response); + expect(response.getOrchestrationstate()!.getInput()!.getValue()).toBe(value); + } else { + expect(request.getInput()!.getValue()).toBe(value); + } + }); + + it("keeps absent payloads absent", async () => { + const request = new pb.CreateInstanceRequest(); + await processor.externalize(request); + expect(request.hasInput()).toBe(false); + expect(store.uploads).toBe(0); + }); + + it("maps large entity batches without the JavaScript variadic argument limit", async () => { + const request = new pb.EntityBatchRequest().setOperationsList( + Array.from({ length: 150000 }, () => new pb.OperationRequest().setInput(text("x"))), + ); + await expect(processor.resolve(request)).resolves.toBeUndefined(); + await expect(processor.externalize(request)).resolves.toBeUndefined(); + expect(store.uploads).toBe(0); + }); + + it("accepts exactly 10 MiB and rejects larger serialized payloads before storage", async () => { + const value = "x".repeat(10 * 1024 * 1024); + await processor.externalize(new pb.CreateInstanceRequest().setInput(text(value))); + await expect( + processor.externalize(new pb.CreateInstanceRequest().setInput(text(value + "x"))), + ).rejects.toBeInstanceOf(PayloadStorageException); + expect(store.uploads).toBe(1); + }); + + it.each([1048577, NaN, Infinity, -1, 0.5])("rejects invalid thresholds (%s)", (thresholdBytes) => { + expect(() => new PayloadProcessor({ store, thresholdBytes })).toThrow(RangeError); + }); + + it.each([ + () => new pb.CreateInstanceRequest().setInput(text("large")), + () => new pb.RaiseEventRequest().setInput(text("large")), + () => new pb.TerminateRequest().setOutput(text("large")), + () => new pb.SuspendRequest().setReason(text("large")), + () => new pb.ResumeRequest().setReason(text("large")), + () => new pb.RewindInstanceRequest().setReason(text("large")), + () => new pb.SignalEntityRequest().setInput(text("large")), + () => new pb.ActivityResponse().setResult(text("large")), + () => new pb.EntityRequest().setEntitystate(text("large")), + () => new pb.EntityBatchRequest().setEntitystate(text("large")), + ])("externalizes every client and worker scalar request (%#)", async (makeRequest) => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + await processor.externalize(makeRequest()); + expect(store.uploads).toBe(1); + }); + + it("externalizes all orchestration actions, custom status and rewritten history", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const response = new pb.OrchestratorResponse() + .setCustomstatus(text("status")) + .setActionsList([ + new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(text("activity"))), + new pb.OrchestratorAction().setCreatesuborchestration( + new pb.CreateSubOrchestrationAction().setInput(text("child")), + ), + new pb.OrchestratorAction().setCompleteorchestration( + new pb.CompleteOrchestrationAction().setResult(text("result")).setDetails(text("details")), + ), + new pb.OrchestratorAction().setTerminateorchestration( + new pb.TerminateOrchestrationAction().setReason(text("reason")), + ), + new pb.OrchestratorAction().setSendevent(new pb.SendEventAction().setData(text("event"))), + new pb.OrchestratorAction().setSendentitymessage( + new pb.SendEntityMessageAction().setEntityoperationcalled( + new pb.EntityOperationCalledEvent().setInput(text("entity")), + ), + ), + new pb.OrchestratorAction().setRewindorchestration( + new pb.RewindOrchestrationAction().setNewhistoryList([ + new pb.HistoryEvent().setExecutionstarted(new pb.ExecutionStartedEvent().setInput(text("rewritten"))), + ]), + ), + ]); + await processor.externalize(response); + expect([...store.values.values()]).toEqual([ + "status", + "activity", + "child", + "result", + "details", + "reason", + "event", + "entity", + "rewritten", + ]); + }); + + it("preserves entity result oneofs and externalizes state, outputs and outgoing operations", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const success = new pb.OperationResult().setSuccess(new pb.OperationResultSuccess().setResult(text("result"))); + const failure = new pb.OperationResult().setFailure(new pb.OperationResultFailure()); + const result = new pb.EntityBatchResult() + .setEntitystate(text("state")) + .setResultsList([success, failure]) + .setActionsList([ + new pb.OperationAction().setSendsignal(new pb.SendSignalAction().setInput(text("signal"))), + new pb.OperationAction().setStartneworchestration(new pb.StartNewOrchestrationAction().setInput(text("start"))), + ]); + await processor.externalize(result); + expect(store.uploads).toBe(4); + expect(success.hasSuccess()).toBe(true); + expect(failure.hasFailure()).toBe(true); + }); + + it("hydrates nested history state, activity and entity V1/V2 work items", async () => { + const token = await store.upload("resolved"); + const history = new pb.HistoryEvent().setHistorystate( + new pb.HistoryStateEvent().setOrchestrationstate( + new pb.OrchestrationState().setInput(text(token)).setOutput(text(token)).setCustomstatus(text(token)), + ), + ); + const messages = [ + new pb.WorkItem().setActivityrequest(new pb.ActivityRequest().setInput(text(token))), + new pb.WorkItem().setOrchestratorrequest(new pb.OrchestratorRequest().setPasteventsList([history])), + new pb.WorkItem().setEntityrequest( + new pb.EntityBatchRequest() + .setEntitystate(text(token)) + .setOperationsList([new pb.OperationRequest().setInput(text(token))]), + ), + new pb.WorkItem().setEntityrequestv2( + new pb.EntityRequest() + .setEntitystate(text(token)) + .setOperationrequestsList([ + new pb.HistoryEvent().setEntityoperationcalled(new pb.EntityOperationCalledEvent().setInput(text(token))), + ]), + ), + new pb.GetEntityResponse().setEntity(new pb.EntityMetadata().setSerializedstate(text(token))), + new pb.QueryEntitiesResponse().setEntitiesList([new pb.EntityMetadata().setSerializedstate(text(token))]), + new pb.QueryInstancesResponse().setOrchestrationstateList([new pb.OrchestrationState().setInput(text(token))]), + new pb.HistoryChunk().setEventsList([ + new pb.HistoryEvent().setExecutionrewound(new pb.ExecutionRewoundEvent().setReason(text(token))), + ]), + ]; + for (const message of messages) { + await processor.resolve(message); + expect(JSON.stringify(message.toObject())).not.toContain(token); + expect(JSON.stringify(message.toObject())).toContain("resolved"); + } + }); + + it.each([ + (value: StringValue) => new pb.HistoryEvent().setExecutionstarted(new pb.ExecutionStartedEvent().setInput(value)), + (value: StringValue) => + new pb.HistoryEvent().setExecutioncompleted(new pb.ExecutionCompletedEvent().setResult(value)), + (value: StringValue) => + new pb.HistoryEvent().setExecutionterminated(new pb.ExecutionTerminatedEvent().setInput(value)), + (value: StringValue) => new pb.HistoryEvent().setTaskscheduled(new pb.TaskScheduledEvent().setInput(value)), + (value: StringValue) => new pb.HistoryEvent().setTaskcompleted(new pb.TaskCompletedEvent().setResult(value)), + (value: StringValue) => + new pb.HistoryEvent().setSuborchestrationinstancecreated( + new pb.SubOrchestrationInstanceCreatedEvent().setInput(value), + ), + (value: StringValue) => + new pb.HistoryEvent().setSuborchestrationinstancecompleted( + new pb.SubOrchestrationInstanceCompletedEvent().setResult(value), + ), + (value: StringValue) => new pb.HistoryEvent().setEventsent(new pb.EventSentEvent().setInput(value)), + (value: StringValue) => new pb.HistoryEvent().setEventraised(new pb.EventRaisedEvent().setInput(value)), + (value: StringValue) => new pb.HistoryEvent().setGenericevent(new pb.GenericEvent().setData(value)), + (value: StringValue) => new pb.HistoryEvent().setContinueasnew(new pb.ContinueAsNewEvent().setInput(value)), + (value: StringValue) => + new pb.HistoryEvent().setExecutionsuspended(new pb.ExecutionSuspendedEvent().setInput(value)), + (value: StringValue) => new pb.HistoryEvent().setExecutionresumed(new pb.ExecutionResumedEvent().setInput(value)), + (value: StringValue) => new pb.HistoryEvent().setExecutionrewound(new pb.ExecutionRewoundEvent().setReason(value)), + (value: StringValue) => + new pb.HistoryEvent().setEntityoperationsignaled(new pb.EntityOperationSignaledEvent().setInput(value)), + (value: StringValue) => + new pb.HistoryEvent().setEntityoperationcalled(new pb.EntityOperationCalledEvent().setInput(value)), + (value: StringValue) => + new pb.HistoryEvent().setEntityoperationcompleted(new pb.EntityOperationCompletedEvent().setOutput(value)), + ])("hydrates and rewrites every payload-bearing history oneof (%#)", async (makeEvent) => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const event = makeEvent(text(await store.upload("payload"))); + const originalCase = event.getEventtypeCase(); + await processor.resolve(new pb.HistoryChunk().setEventsList([event])); + expect(JSON.stringify(event.toObject())).toContain("payload"); + const response = new pb.OrchestratorResponse().setActionsList([ + new pb.OrchestratorAction().setRewindorchestration(new pb.RewindOrchestrationAction().setNewhistoryList([event])), + ]); + await processor.externalize(response); + expect([...store.values.values()]).toEqual(["payload", "payload"]); + expect(event.getEventtypeCase()).toBe(originalCase); + }); + + it("converts permanent activity upload failures without sending the result", async () => { + store.upload = async () => { + throw Object.assign(new Error("denied"), { statusCode: 403 }); + }; + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const response = new pb.ActivityResponse().setResult(text("large")).setCompletiontoken("completion"); + await processor.externalize(response); + expect(response.hasResult()).toBe(false); + expect(response.getFailuredetails()!.getIsnonretriable()).toBe(true); + expect(response.getCompletiontoken()).toBe("completion"); + }); + + it("clears partial orchestration responses before a non-retriable failure", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1, maxPayloadBytes: 2 }); + const response = new pb.OrchestratorResponse() + .setCustomstatus(text("large")) + .setIspartial(true) + .setChunkindex(new Int32Value().setValue(2)) + .setActionsList([new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(text("x")))]); + await processor.externalize(response); + expect(response.hasCustomstatus()).toBe(false); + expect(response.getIspartial()).toBe(false); + expect(response.hasChunkindex()).toBe(false); + expect(response.getActionsList()).toHaveLength(1); + const complete = response.getActionsList()[0].getCompleteorchestration()!; + expect(complete.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(complete.getFailuredetails()!.getIsnonretriable()).toBe(true); + }); + + it.each([408, 429, 500, 503])("propagates transient upload failure %s", async (statusCode) => { + const error = Object.assign(new Error("transient"), { statusCode }); + store.upload = async () => { + throw error; + }; + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + await expect(processor.externalize(new pb.ActivityResponse().setResult(text("large")))).rejects.toBe(error); + }); +}); + +describe("bounded payload I/O", () => { + it("caps concurrency at eight", async () => { + let active = 0; + let maximum = 0; + await runBounded( + Array.from({ length: 40 }, () => async () => { + maximum = Math.max(maximum, ++active); + await new Promise((resolve) => setImmediate(resolve)); + active--; + }), + ); + expect(maximum).toBe(8); + expect(active).toBe(0); + }); + + it("stops dispatch, drains every started operation and throws the lowest-ordinal failure", async () => { + const first = new Error("first in message order"); + const later = new Error("first to finish"); + const started: number[] = []; + const completed: number[] = []; + await expect( + runBounded( + Array.from({ length: 20 }, (_, index) => async () => { + started.push(index); + if (index === 1) throw later; + await new Promise((resolve) => setImmediate(resolve)); + completed.push(index); + if (index === 0) throw first; + }), + ), + ).rejects.toBe(first); + expect(started).toHaveLength(2); + expect(completed).toEqual([0]); + }); + + it("drains on cancellation and does not observe cancellation for no-op messages", async () => { + const abort = new AbortController(); + let completed = 0; + await expect( + runBounded( + Array.from({ length: 20 }, () => async () => { + abort.abort(); + await new Promise((resolve) => setImmediate(resolve)); + completed++; + }), + abort.signal, + ), + ).rejects.toBe(abort.signal.reason); + expect(completed).toBe(1); + await expect(runBounded([], abort.signal)).resolves.toBeUndefined(); + }); +}); From b6c27b1a7ef67670304be9109abdd64a70ab8e8c Mon Sep 17 00:00:00 2001 From: wangbill Date: Fri, 16 Oct 2026 16:07:29 -0400 Subject: [PATCH 02/11] feat: integrate payload transport with SDK clients and workers Phase 2 of 6: externalize outgoing payloads and await query, work-item and streamed history hydration before conversion or dispatch. Add Azure-managed builder opt-in and its core API peer floor, without importing the later Azure Blob provider or cleanup runner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7 --- README.md | 13 + package-lock.json | 2 +- .../durabletask-js-azuremanaged/CHANGELOG.md | 3 + .../durabletask-js-azuremanaged/package.json | 2 +- .../src/client-builder.ts | 22 +- .../src/worker-builder.ts | 9 + .../test/unit/payload-builders.spec.ts | 46 ++++ packages/durabletask-js/src/client/client.ts | 81 ++++-- .../src/payloads/history-stream.ts | 62 +++++ .../src/worker/task-hub-grpc-worker.ts | 29 ++- .../test/payload-transport.spec.ts | 233 ++++++++++++++++++ 11 files changed, 469 insertions(+), 33 deletions(-) create mode 100644 packages/durabletask-js-azuremanaged/test/unit/payload-builders.spec.ts create mode 100644 packages/durabletask-js/src/payloads/history-stream.ts create mode 100644 packages/durabletask-js/test/payload-transport.spec.ts diff --git a/README.md b/README.md index 823f6dea..17ffbf62 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,19 @@ const entityResponseBytes = await worker.processEntityBatchRequest(entityBatchRe `TaskHubGrpcClient` already exposes orchestration start/query/event/terminate/suspend/resume/purge APIs and entity signal/read/query/clean APIs through its existing `hostAddress` and `metadataGenerator` options. Host integrations that need task-hub routing metadata should provide it through `metadataGenerator`, keeping host-specific metadata policy outside the core client. Azure-managed scheduler connection strings remain in `@microsoft/durabletask-js-azuremanaged`. +## External payload transport + +Core clients and workers accept opt-in `payloads: { store }` options, where `store` implements +the exported `PayloadStore` contract. The same store must be configured on every participant. +The default threshold is 256 KiB of serialized UTF-8 bytes (inclusive), with a 10 MiB maximum. +Requests are externalized before sending; queries, history chunks and work items are hydrated +before conversion or execution. Empty payloads remain inline, and disabling the option preserves +the existing transport behavior. + +Azure-managed builders expose `.useExternalizedPayloads({ store })` and require core `>=0.4.1` +for the public payload types. This phase works with a custom/in-memory store and does not import +an Azure Blob provider or register cleanup tasks. Complete the six-phase feature stack before release. + ## Long durable timers `createTimer(Date | seconds)` has no SDK-imposed total-duration cap. Like the Python SDK, diff --git a/package-lock.json b/package-lock.json index e1d04436..29223192 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7671,7 +7671,7 @@ }, "peerDependencies": { "@grpc/grpc-js": "^1.14.4", - "@microsoft/durabletask-js": ">=0.3.0" + "@microsoft/durabletask-js": ">=0.4.1" } }, "packages/durabletask-js-azuremanaged/node_modules/@types/node": { diff --git a/packages/durabletask-js-azuremanaged/CHANGELOG.md b/packages/durabletask-js-azuremanaged/CHANGELOG.md index 60244b3c..a57bd6cb 100644 --- a/packages/durabletask-js-azuremanaged/CHANGELOG.md +++ b/packages/durabletask-js-azuremanaged/CHANGELOG.md @@ -2,6 +2,9 @@ ### New +- Add `.useExternalizedPayloads()` to client and worker builders, preserving settings when + connection options change. The new public option types require core `>=0.4.1`. + Cleanup registration is deferred to the later worker phase; do not release partial feature phases. - Support the `ResourceId` token audience URI in connection strings and an optional final `resourceId` argument in the client/worker factory functions, alongside existing builder/options setters. Trim surrounding whitespace and trailing slashes, remove one case-insensitive `/.default` suffix, diff --git a/packages/durabletask-js-azuremanaged/package.json b/packages/durabletask-js-azuremanaged/package.json index 29f72a6e..151e9a49 100644 --- a/packages/durabletask-js-azuremanaged/package.json +++ b/packages/durabletask-js-azuremanaged/package.json @@ -53,7 +53,7 @@ }, "peerDependencies": { "@grpc/grpc-js": "^1.14.4", - "@microsoft/durabletask-js": ">=0.3.0" + "@microsoft/durabletask-js": ">=0.4.1" }, "devDependencies": { "@types/jest": "^29.5.1", diff --git a/packages/durabletask-js-azuremanaged/src/client-builder.ts b/packages/durabletask-js-azuremanaged/src/client-builder.ts index eab641be..1c848513 100644 --- a/packages/durabletask-js-azuremanaged/src/client-builder.ts +++ b/packages/durabletask-js-azuremanaged/src/client-builder.ts @@ -5,7 +5,7 @@ import { TokenCredential } from "@azure/identity"; import * as grpc from "@grpc/grpc-js"; import { DurableTaskAzureManagedClientOptions } from "./options"; import { ClientRetryOptions } from "./retry-policy"; -import { TaskHubGrpcClient, Logger, ConsoleLogger } from "@microsoft/durabletask-js"; +import { TaskHubGrpcClient, Logger, ConsoleLogger, PayloadStorageOptions } from "@microsoft/durabletask-js"; /** * Builder for creating DurableTaskClient instances that connect to Azure-managed Durable Task service. @@ -15,6 +15,7 @@ export class DurableTaskAzureManagedClientBuilder { private _options: DurableTaskAzureManagedClientOptions; private _grpcChannelOptions: grpc.ChannelOptions = {}; private _logger: Logger = new ConsoleLogger(); + private _payloads?: PayloadStorageOptions; /** * Creates a new instance of DurableTaskAzureManagedClientBuilder. @@ -142,6 +143,12 @@ export class DurableTaskAzureManagedClientBuilder { return this; } + /** Enables transparent externalized payloads using an independently configured payload store. */ + useExternalizedPayloads(options: PayloadStorageOptions): DurableTaskAzureManagedClientBuilder { + this._payloads = options; + return this; + } + /** * Builds and returns a configured TaskHubGrpcClient. * @@ -164,14 +171,15 @@ export class DurableTaskAzureManagedClientBuilder { }; // Use the core TaskHubGrpcClient with custom channel credentials and per-call metadata. - return new TaskHubGrpcClient( + return new TaskHubGrpcClient({ hostAddress, - combinedOptions, - true, - channelCredentials, + options: combinedOptions, + useTLS: true, + credentials: channelCredentials, metadataGenerator, - this._logger, - ); + logger: this._logger, + payloads: this._payloads, + }); } } diff --git a/packages/durabletask-js-azuremanaged/src/worker-builder.ts b/packages/durabletask-js-azuremanaged/src/worker-builder.ts index 8a0b1a76..05e61418 100644 --- a/packages/durabletask-js-azuremanaged/src/worker-builder.ts +++ b/packages/durabletask-js-azuremanaged/src/worker-builder.ts @@ -15,6 +15,7 @@ import { ConsoleLogger, VersioningOptions, WorkItemFilters, + PayloadStorageOptions, } from "@microsoft/durabletask-js"; /** @@ -33,6 +34,7 @@ export class DurableTaskAzureManagedWorkerBuilder { private _channelRecreateFailureThreshold?: number; private _versioning?: VersioningOptions; private _workItemFilters?: WorkItemFilters | "auto"; + private _payloads?: PayloadStorageOptions; /** * Creates a new instance of DurableTaskAzureManagedWorkerBuilder. @@ -305,6 +307,12 @@ export class DurableTaskAzureManagedWorkerBuilder { return this; } + /** Enables payload hydration, externalization and the reserved auto-purge tasks. */ + useExternalizedPayloads(options: PayloadStorageOptions): DurableTaskAzureManagedWorkerBuilder { + this._payloads = options; + return this; + } + /** * Builds and returns a configured TaskHubGrpcWorker. * @@ -339,6 +347,7 @@ export class DurableTaskAzureManagedWorkerBuilder { channelRecreateFailureThreshold: this._channelRecreateFailureThreshold, versioning: this._versioning, workItemFilters: this._workItemFilters, + payloads: this._payloads, // DTS natively supports long timers, matching the Python Azure-managed worker. maximumTimerIntervalMs: null, }); diff --git a/packages/durabletask-js-azuremanaged/test/unit/payload-builders.spec.ts b/packages/durabletask-js-azuremanaged/test/unit/payload-builders.spec.ts new file mode 100644 index 00000000..d7975126 --- /dev/null +++ b/packages/durabletask-js-azuremanaged/test/unit/payload-builders.spec.ts @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { PayloadStore, NoOpLogger } from "@microsoft/durabletask-js"; +import { DurableTaskAzureManagedClientBuilder, DurableTaskAzureManagedWorkerBuilder } from "../../src"; +import * as pb from "../../../durabletask-js/src/proto/orchestrator_service_pb"; + +class Store extends PayloadStore { + async upload(): Promise { + return "external:payload"; + } + async download(): Promise { + return '"resolved"'; + } + isKnownPayloadToken(value: string): boolean { + return value.startsWith("external:"); + } +} + +describe("Azure-managed payload configuration", () => { + it("preserves client payload settings when connection options are replaced", async () => { + const client = new DurableTaskAzureManagedClientBuilder() + .useExternalizedPayloads({ store: new Store(), thresholdBytes: 1 }) + .connectionString("Endpoint=http://localhost:8080;Authentication=None;TaskHub=payload-test") + .logger(new NoOpLogger()) + .build(); + expect(client["_payloads"]).toBeDefined(); + await client.stop(); + }); + it("preserves worker payload settings and business registrations", () => { + const worker = new DurableTaskAzureManagedWorkerBuilder() + .useExternalizedPayloads({ store: new Store() }) + .addNamedOrchestrator("business", () => undefined) + .connectionString("Endpoint=http://localhost:8080;Authentication=None;TaskHub=payload-test") + .useWorkItemFilters() + .build(); + const request = worker["_buildGetWorkItemsRequest"](); + expect(request.getCapabilitiesList()).toContain(pb.WorkerCapability.WORKER_CAPABILITY_LARGE_PAYLOADS); + expect( + request + .getWorkitemfilters()! + .getOrchestrationsList() + .map((f) => f.getName()), + ).toEqual(["business"]); + }); +}); diff --git a/packages/durabletask-js/src/client/client.ts b/packages/durabletask-js/src/client/client.ts index 54d3bb7c..f71ba9fc 100644 --- a/packages/durabletask-js/src/client/client.ts +++ b/packages/durabletask-js/src/client/client.ts @@ -37,6 +37,9 @@ import { EntityInstanceId } from "../entities/entity-instance-id"; import { EntityMetadata, createEntityMetadata, createEntityMetadataWithoutState } from "../entities/entity-metadata"; import { EntityQuery, normalizeInstanceIdPrefix } from "../entities/entity-query"; import { SignalEntityOptions } from "../entities/signal-entity-options"; +import { PayloadStorageOptions } from "../payloads/payload-store"; +import { PayloadProcessor } from "../payloads/payload-processor"; +import { readPayloadHistory, PayloadHydrationError } from "../payloads/history-stream"; import { CleanEntityStorageRequest, CleanEntityStorageResult, @@ -59,6 +62,8 @@ export { MetadataGenerator } from "../utils/grpc-helper.util"; * Options for creating a TaskHubGrpcClient. */ export interface TaskHubGrpcClientOptions { + /** Optional externalized payload storage. Disabled by default. */ + payloads?: PayloadStorageOptions; /** The host address to connect to. Defaults to "localhost:4001". */ hostAddress?: string; /** gRPC channel options. */ @@ -84,6 +89,8 @@ export class TaskHubGrpcClient { private _metadataGenerator?: MetadataGenerator; private _logger: Logger; private _defaultVersion?: string; + private readonly _payloads?: PayloadProcessor; + private readonly _payloadCancellation = new AbortController(); /** * Creates a new TaskHubGrpcClient instance. @@ -137,6 +144,7 @@ export class TaskHubGrpcClient { resolvedMetadataGenerator = hostAddressOrOptions.metadataGenerator; resolvedLogger = hostAddressOrOptions.logger; resolvedDefaultVersion = hostAddressOrOptions.defaultVersion; + if (hostAddressOrOptions.payloads) this._payloads = new PayloadProcessor(hostAddressOrOptions.payloads); } else { // Deprecated positional parameters constructor resolvedHostAddress = hostAddressOrOptions; @@ -154,6 +162,7 @@ export class TaskHubGrpcClient { } async stop(): Promise { + this._payloadCancellation.abort(); await this._stub.close(); // Brief pause to allow gRPC cleanup - this is a known issue with grpc-node @@ -161,6 +170,22 @@ export class TaskHubGrpcClient { await new Promise((resolve) => setTimeout(resolve, 1000)); } + private async _call( + method: Parameters>[0], + request: TReq, + metadataGenerator?: MetadataGenerator, + signal?: AbortSignal, + ): Promise { + if (!this._payloads) return callWithMetadata(method, request, metadataGenerator, signal); + const payloadSignal = signal + ? AbortSignal.any([signal, this._payloadCancellation.signal]) + : this._payloadCancellation.signal; + await this._payloads.externalize(request, payloadSignal); + const response = await callWithMetadata(method, request, metadataGenerator, payloadSignal); + await this._payloads.resolve(response, payloadSignal); + return response; + } + /** * Schedules a new orchestrator using the DurableTask client. * @@ -256,7 +281,7 @@ export class TaskHubGrpcClient { ClientLogs.schedulingOrchestration(this._logger, req.getInstanceid(), name, inputSizeInBytes); try { - const res = await callWithMetadata( + const res = await this._call( this._stub.startInstance.bind(this._stub), req, this._metadataGenerator, @@ -301,7 +326,7 @@ export class TaskHubGrpcClient { req.setInstanceid(instanceId); req.setGetinputsandoutputs(fetchPayloads); - const res = await callWithMetadata( + const res = await this._call( this._stub.getInstance.bind(this._stub), req, this._metadataGenerator, @@ -343,7 +368,7 @@ export class TaskHubGrpcClient { signal, `Timed out waiting for orchestration '${instanceId}' to start after ${timeout}s`, (waitSignal) => - callWithMetadata(this._stub.waitForInstanceStart.bind(this._stub), req, this._metadataGenerator, waitSignal), + this._call(this._stub.waitForInstanceStart.bind(this._stub), req, this._metadataGenerator, waitSignal), ); return newOrchestrationState(req.getInstanceid(), res); @@ -415,7 +440,7 @@ export class TaskHubGrpcClient { // Metadata failures must not enter the remote-deadline retry path. const metadata = this._metadataGenerator ? await this._metadataGenerator() : new grpc.Metadata(); try { - return await callWithMetadata( + return await this._call( this._stub.waitForInstanceCompletion.bind(this._stub), req, async () => metadata, @@ -498,7 +523,7 @@ export class TaskHubGrpcClient { ClientLogs.raisingEvent(this._logger, instanceId, eventName); try { - await callWithMetadata( + await this._call( this._stub.raiseEvent.bind(this._stub), req, this._metadataGenerator, @@ -565,7 +590,7 @@ export class TaskHubGrpcClient { ClientLogs.terminatingInstance(this._logger, instanceId); - await callWithMetadata( + await this._call( this._stub.terminateInstance.bind(this._stub), req, this._metadataGenerator, @@ -591,7 +616,7 @@ export class TaskHubGrpcClient { ClientLogs.suspendingInstance(this._logger, instanceId); - await callWithMetadata( + await this._call( this._stub.suspendInstance.bind(this._stub), req, this._metadataGenerator, @@ -617,7 +642,7 @@ export class TaskHubGrpcClient { ClientLogs.resumingInstance(this._logger, instanceId); - await callWithMetadata( + await this._call( this._stub.resumeInstance.bind(this._stub), req, this._metadataGenerator, @@ -656,7 +681,7 @@ export class TaskHubGrpcClient { ClientLogs.rewindingInstance(this._logger, instanceId, reason); try { - await callWithMetadata( + await this._call( this._stub.rewindInstance.bind(this._stub), req, this._metadataGenerator, @@ -721,7 +746,7 @@ export class TaskHubGrpcClient { ClientLogs.restartingInstance(this._logger, instanceId, restartWithNewInstanceId); try { - const res = await callWithMetadata( + const res = await this._call( this._stub.restartInstance.bind(this._stub), req, this._metadataGenerator, @@ -775,7 +800,7 @@ export class TaskHubGrpcClient { ClientLogs.purgingInstanceMetadata(this._logger, instanceId); - res = await callWithMetadata( + res = await this._call( this._stub.purgeInstances.bind(this._stub), req, this._metadataGenerator, @@ -811,7 +836,7 @@ export class TaskHubGrpcClient { runtimeStatusList.map(String).join(", "), ); - const callPromise = callWithMetadata( + const callPromise = this._call( this._stub.purgeInstances.bind(this._stub), req, this._metadataGenerator, @@ -915,7 +940,7 @@ export class TaskHubGrpcClient { req.setQuery(query); - const response = await callWithMetadata( + const response = await this._call( this._stub.queryInstances.bind(this._stub), req, this._metadataGenerator, @@ -1001,7 +1026,7 @@ export class TaskHubGrpcClient { req.setCompletedtimeto(timestamp); } - const response = await callWithMetadata( + const response = await this._call( this._stub.listInstanceIds.bind(this._stub), req, this._metadataGenerator, @@ -1049,6 +1074,26 @@ export class TaskHubGrpcClient { const metadata = this._metadataGenerator ? await this._metadataGenerator() : new grpc.Metadata(); const stream = this._stub.streamInstanceHistory(req, metadata); + if (this._payloads) { + try { + const events = await readPayloadHistory(stream, this._payloads, this._payloadCancellation.signal); + return events.map(convertProtoHistoryEvent).filter((event): event is HistoryEvent => event !== undefined); + } catch (error) { + if (error instanceof PayloadHydrationError) throw error.cause; + if (error instanceof Error && "code" in error) { + if (error.code === grpc.status.NOT_FOUND) return []; + if (error.code === grpc.status.CANCELLED) + throw new Error("The getOrchestrationHistory operation was canceled."); + if (error.code === grpc.status.INTERNAL) { + throw new Error( + `An error occurred while retrieving the history for orchestration with instanceId '${instanceId}'.`, + ); + } + } + throw error; + } + } + return new Promise((resolve, reject) => { const historyEvents: HistoryEvent[] = []; @@ -1147,7 +1192,7 @@ export class TaskHubGrpcClient { ClientLogs.signalingEntity(this._logger, id.toString(), operationName); try { - await callWithMetadata( + await this._call( this._stub.signalEntity.bind(this._stub), req, this._metadataGenerator, @@ -1183,7 +1228,7 @@ export class TaskHubGrpcClient { ClientLogs.gettingEntity(this._logger, id.toString()); - const res = await callWithMetadata( + const res = await this._call( this._stub.getEntity.bind(this._stub), req, this._metadataGenerator, @@ -1271,7 +1316,7 @@ export class TaskHubGrpcClient { req.setQuery(protoQuery); - const res = await callWithMetadata( + const res = await this._call( this._stub.queryEntities.bind(this._stub), req, this._metadataGenerator, @@ -1311,7 +1356,7 @@ export class TaskHubGrpcClient { protoReq.setContinuationtoken(token); } - const res = await callWithMetadata( + const res = await this._call( this._stub.cleanEntityStorage.bind(this._stub), protoReq, this._metadataGenerator, diff --git a/packages/durabletask-js/src/payloads/history-stream.ts b/packages/durabletask-js/src/payloads/history-stream.ts new file mode 100644 index 00000000..81d09eaa --- /dev/null +++ b/packages/durabletask-js/src/payloads/history-stream.ts @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as grpc from "@grpc/grpc-js"; +import * as pb from "../proto/orchestrator_service_pb"; +import { PayloadProcessor } from "./payload-processor"; + +export class PayloadHydrationError extends Error { + constructor(cause: unknown) { + super("History payload hydration failed.", { cause }); + this.name = "PayloadHydrationError"; + } +} + +/** Serializes chunk hydration, applies stream backpressure, and drains I/O before settling. */ +export function readPayloadHistory( + stream: grpc.ClientReadableStream, + processor: PayloadProcessor, + signal?: AbortSignal, +): Promise { + return new Promise((resolve, reject) => { + const events: pb.HistoryEvent[] = []; + let pending = Promise.resolve(); + let finishing = false; + const finish = (error?: unknown) => { + if (finishing) return; + finishing = true; + stream.removeListener("data", onData); + stream.removeListener("end", onEnd); + stream.removeListener("error", onError); + stream.removeListener("close", onClose); + stream.on("error", () => {}); + signal?.removeEventListener("abort", onAbort); + if (error !== undefined) stream.cancel(); + stream.destroy(); + void pending.then(() => (error === undefined ? resolve(events) : reject(error)), reject); + }; + const onData = (chunk: pb.HistoryChunk) => { + stream.pause(); + pending = pending.then(async () => { + try { + await processor.resolve(chunk, signal); + } catch (error) { + throw new PayloadHydrationError(error); + } + for (const event of chunk.getEventsList()) events.push(event); + if (!finishing) stream.resume(); + }); + void pending.catch(finish); + }; + const onEnd = () => finish(); + const onError = (error: unknown) => finish(error); + const onClose = () => finish(new Error("History stream closed before all history was received.")); + const onAbort = () => finish(signal?.reason ?? new Error("History hydration was cancelled.")); + stream.on("data", onData); + stream.once("end", onEnd); + stream.once("error", onError); + stream.once("close", onClose); + signal?.addEventListener("abort", onAbort, { once: true }); + if (signal?.aborted) onAbort(); + }); +} diff --git a/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts b/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts index 962b6fec..46284b60 100644 --- a/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts +++ b/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts @@ -26,6 +26,9 @@ import { ExponentialBackoff, sleep, withTimeout } from "../utils/backoff.util"; import { VersioningOptions, VersionMatchStrategy, VersionFailureStrategy } from "./versioning-options"; import { WorkItemFilters, generateWorkItemFiltersFromRegistry, toGrpcWorkItemFilters } from "./work-item-filters"; import { compareVersions } from "../utils/versioning.util"; +import { PayloadStorageOptions } from "../payloads/payload-store"; +import { PayloadProcessor } from "../payloads/payload-processor"; +import { readPayloadHistory, PayloadHydrationError } from "../payloads/history-stream"; import * as WorkerLogs from "./logs"; import { ConcurrencyOptions, ResolvedConcurrencyOptions, resolveConcurrencyOptions } from "./concurrency-options"; import { @@ -61,6 +64,8 @@ type WorkItemStreamResult = { * Options for creating a TaskHubGrpcWorker. */ export interface TaskHubGrpcWorkerOptions { + /** Optional externalized payload storage. Disabled by default. */ + payloads?: PayloadStorageOptions; /** * Maximum backend timer segment in milliseconds. Defaults to three days, like Python. * null, zero, or negative values disable segmentation. Must be finite; positive @@ -139,6 +144,7 @@ export class TaskHubGrpcWorker { private _workerLoopPromise: Promise | null; private _deferredStubCloseTimers: Map>; private readonly _maximumTimerIntervalMs: number | null; + private readonly _payloads?: PayloadProcessor; /** * Creates a new TaskHubGrpcWorker instance. @@ -207,6 +213,7 @@ export class TaskHubGrpcWorker { resolvedWorkItemFilters = hostAddressOrOptions.workItemFilters; resolvedConcurrency = hostAddressOrOptions.concurrency; resolvedMaximumTimerIntervalMs = hostAddressOrOptions.maximumTimerIntervalMs; + if (hostAddressOrOptions.payloads) this._payloads = new PayloadProcessor(hostAddressOrOptions.payloads); } else { // Deprecated positional parameters constructor resolvedHostAddress = hostAddressOrOptions; @@ -857,6 +864,7 @@ export class TaskHubGrpcWorker { private _buildGetWorkItemsRequest(): pb.GetWorkItemsRequest { const request = new pb.GetWorkItemsRequest(); request.setCapabilitiesList([pb.WorkerCapability.WORKER_CAPABILITY_HISTORY_STREAMING]); + if (this._payloads) request.addCapabilities(pb.WorkerCapability.WORKER_CAPABILITY_LARGE_PAYLOADS); request.setMaxconcurrentactivityworkitems( Math.min(this._concurrency.maximumConcurrentActivityWorkItems, MAX_PROTOCOL_CONCURRENCY), ); @@ -974,6 +982,7 @@ export class TaskHubGrpcWorker { request: TReq, signal?: AbortSignal, ): Promise { + if (this._payloads) await this._payloads.externalize(request, signal); const backoff = new ExponentialBackoff({ initialDelayMs: 200, maxDelayMs: 15000, @@ -1059,6 +1068,13 @@ export class TaskHubGrpcWorker { if (settled) return; signal?.throwIfAborted(); stream = stub.streamInstanceHistory(request, metadata); + if (this._payloads) { + readPayloadHistory(stream, this._payloads, signal).then((hydrated) => { + for (const event of hydrated) events.push(event); + finish(); + }, onError); + return; + } stream.on("data", onData); stream.once("end", onEnd); stream.once("error", onError); @@ -1079,6 +1095,8 @@ export class TaskHubGrpcWorker { ): Promise { const instanceId = req.getInstanceid(); + if (this._payloads) await this._payloads.resolve(req, retrySignal); + if (!instanceId) { throw new Error(`Could not execute the orchestrator as the instanceId was not provided (${instanceId})`); } @@ -1096,6 +1114,7 @@ export class TaskHubGrpcWorker { req.setPasteventsList(pastEvents); } catch (e: unknown) { if (retrySignal?.aborted) return; + if (e instanceof PayloadHydrationError) throw e.cause; const error = e instanceof Error ? e : new Error(String(e)); WorkerLogs.executionError(this._logger, instanceId, error); const res = new pb.OrchestratorResponse(); @@ -1327,6 +1346,7 @@ export class TaskHubGrpcWorker { stub: stubs.TaskHubSidecarServiceClient, retrySignal?: AbortSignal, ): Promise { + if (this._payloads) await this._payloads.resolve(req, retrySignal); const instanceId = req.getOrchestrationinstance()?.getInstanceid(); if (!instanceId) { @@ -1335,12 +1355,7 @@ export class TaskHubGrpcWorker { const versionCheck = this._checkVersionCompatibility(req.getVersion()?.getValue()); if (!versionCheck.compatible && !versionCheck.shouldFail) { - WorkerLogs.versionMismatchAbandon( - this._logger, - instanceId, - versionCheck.errorType!, - versionCheck.errorMessage!, - ); + WorkerLogs.versionMismatchAbandon(this._logger, instanceId, versionCheck.errorType!, versionCheck.errorMessage!); try { const abandon = new pb.AbandonActivityTaskRequest().setCompletiontoken(completionToken); await this._deliverResponse(stub.abandonTaskActivityWorkItem.bind(stub), abandon, retrySignal); @@ -1444,6 +1459,7 @@ export class TaskHubGrpcWorker { operationInfos?: pb.OperationInfo[], retrySignal?: AbortSignal, ): Promise { + if (this._payloads) await this._payloads.resolve(req, retrySignal); const instanceIdString = req.getInstanceid(); if (!instanceIdString) { @@ -1546,6 +1562,7 @@ export class TaskHubGrpcWorker { stub: stubs.TaskHubSidecarServiceClient, retrySignal?: AbortSignal, ): Promise { + if (this._payloads) await this._payloads.resolve(req, retrySignal); // Convert EntityRequest (V2) to EntityBatchRequest (V1) format const batchRequest = new pb.EntityBatchRequest(); batchRequest.setInstanceid(req.getInstanceid()); diff --git a/packages/durabletask-js/test/payload-transport.spec.ts b/packages/durabletask-js/test/payload-transport.spec.ts new file mode 100644 index 00000000..a1bc197b --- /dev/null +++ b/packages/durabletask-js/test/payload-transport.spec.ts @@ -0,0 +1,233 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as grpc from "@grpc/grpc-js"; +import { Empty } from "google-protobuf/google/protobuf/empty_pb"; +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import { TaskHubGrpcClient, TaskHubGrpcWorker, PayloadStore, NoOpLogger } from "../src"; +import * as pb from "../src/proto/orchestrator_service_pb"; +import * as stubs from "../src/proto/orchestrator_service_grpc_pb"; +import * as pbh from "../src/utils/pb-helper.util"; + +const text = (value: string) => new StringValue().setValue(value); +class MemoryStore extends PayloadStore { + readonly values = new Map(); + async upload(value: string): Promise { + const token = `payload:${this.values.size}`; + this.values.set(token, value); + return token; + } + async download(token: string): Promise { + await new Promise((resolve) => setTimeout(resolve, 5)); + if (!this.values.has(token)) throw new Error("Missing blob"); + return this.values.get(token)!; + } + isKnownPayloadToken(value: string): boolean { + return value.startsWith("payload:"); + } +} + +async function waitFor(predicate: () => boolean): Promise { + const deadline = Date.now() + 3000; + while (!predicate()) { + if (Date.now() > deadline) throw new Error("Timed out"); + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + +describe("transparent payload transport over gRPC", () => { + let server: grpc.Server; + let client: TaskHubGrpcClient; + let worker: TaskHubGrpcWorker; + let store: MemoryStore; + let hostAddress: string; + let input: string; + let subscription: grpc.ServerWritableStream | undefined; + let activityResponse: pb.ActivityResponse | undefined; + let activityCalls: number; + let requestedInput: string | undefined; + let orchestrationResponses: pb.OrchestratorResponse[]; + let historyEvents: pb.HistoryEvent[]; + + beforeEach(async () => { + store = new MemoryStore(); + input = await store.upload(JSON.stringify({ message: "hydrated" })); + subscription = undefined; + activityResponse = undefined; + activityCalls = 0; + requestedInput = undefined; + orchestrationResponses = []; + historyEvents = [pbh.newExecutionStartedEvent("flow", "instance", input)]; + server = new grpc.Server(); + const state = () => + new pb.OrchestrationState() + .setInstanceid("instance") + .setName("workflow") + .setInput(text(input)) + .setOutput(text(input)) + .setCustomstatus(text(input)); + const service = { + hello: (_call, callback) => callback(null, new Empty()), + startInstance: (call, callback) => { + requestedInput = call.request.getInput()?.getValue(); + callback(null, new pb.CreateInstanceResponse().setInstanceid(call.request.getInstanceid())); + }, + getInstance: (_call, callback) => + callback(null, new pb.GetInstanceResponse().setExists(true).setOrchestrationstate(state())), + waitForInstanceStart: (_call, callback) => + callback(null, new pb.GetInstanceResponse().setExists(true).setOrchestrationstate(state())), + queryInstances: (_call, callback) => + callback(null, new pb.QueryInstancesResponse().setOrchestrationstateList([state()])), + streamInstanceHistory: (call) => { + call.write(new pb.HistoryChunk().setEventsList(historyEvents)); + call.end(); + }, + getWorkItems: (call) => { + subscription = call; + call.on("cancelled", () => call.end()); + }, + completeActivityTask: (call, callback) => { + activityResponse = call.request; + callback(null, new pb.CompleteTaskResponse()); + }, + completeOrchestratorTask: (call, callback) => { + orchestrationResponses.push(call.request); + callback(null, new pb.CompleteTaskResponse()); + }, + } satisfies Pick< + stubs.ITaskHubSidecarServiceServer, + | "hello" + | "startInstance" + | "getInstance" + | "waitForInstanceStart" + | "queryInstances" + | "streamInstanceHistory" + | "getWorkItems" + | "completeActivityTask" + | "completeOrchestratorTask" + >; + server.addService(stubs.TaskHubSidecarServiceService, service); + const port = await new Promise((resolve, reject) => + server.bindAsync("127.0.0.1:0", grpc.ServerCredentials.createInsecure(), (error, port) => + error ? reject(error) : resolve(port), + ), + ); + hostAddress = `127.0.0.1:${port}`; + const payloads = { store, thresholdBytes: 1 }; + client = new TaskHubGrpcClient({ hostAddress, payloads, logger: new NoOpLogger() }); + worker = new TaskHubGrpcWorker({ hostAddress, payloads, logger: new NoOpLogger() }); + worker.addNamedActivity("echo", async (_context, value) => { + activityCalls++; + return value; + }); + }); + + afterEach(async () => { + if (worker["_isRunning"]) await worker.stop(); + await client.stop(); + server.forceShutdown(); + }); + + it("externalizes before sending and hydrates state, wait, query and streamed history before conversion", async () => { + await client.scheduleNewOrchestration("workflow", { message: "request" }); + expect(requestedInput).toMatch(/^payload:/); + expect(await store.download(requestedInput!)).toBe('{"message":"request"}'); + expect((await client.getOrchestrationState("instance"))!.serializedInput).toBe('{"message":"hydrated"}'); + expect((await client.waitForOrchestrationStart("instance"))!.serializedInput).toBe('{"message":"hydrated"}'); + const states = []; + for await (const item of client.getAllInstances({ fetchInputsAndOutputs: true })) states.push(item); + expect(states[0].serializedInput).toBe('{"message":"hydrated"}'); + const history = await client.getOrchestrationHistory("instance"); + expect(history[0]).toMatchObject({ input: '{"message":"hydrated"}' }); + }); + + it("advertises support, awaits input hydration and externalizes worker output", async () => { + await worker.start(); + await waitFor(() => !!subscription); + expect(subscription!.request.getCapabilitiesList()).toContain(pb.WorkerCapability.WORKER_CAPABILITY_LARGE_PAYLOADS); + subscription!.write( + new pb.WorkItem() + .setCompletiontoken("completion") + .setActivityrequest( + new pb.ActivityRequest() + .setName("echo") + .setTaskid(1) + .setInput(text(input)) + .setOrchestrationinstance(new pb.OrchestrationInstance().setInstanceid("instance")), + ), + ); + await waitFor(() => !!activityResponse); + expect(activityCalls).toBe(1); + expect(activityResponse!.getCompletiontoken()).toBe("completion"); + expect(await store.download(activityResponse!.getResult()!.getValue())).toBe('{"message":"hydrated"}'); + }); + + it("propagates missing streamed payloads rather than returning tokens or succeeding early at end", async () => { + store.values.clear(); + await expect(client.getOrchestrationHistory("instance")).rejects.toThrow("Missing blob"); + }); + + it("hydrates streamed orchestration history before user execution and externalizes its result", async () => { + let observed: unknown; + worker.addNamedOrchestrator("flow", async (_ctx, value) => { + observed = value; + return value; + }); + await worker.start(); + await waitFor(() => !!subscription); + subscription!.write( + new pb.WorkItem() + .setCompletiontoken("streamed") + .setOrchestratorrequest( + new pb.OrchestratorRequest() + .setInstanceid("instance") + .setRequireshistorystreaming(true) + .setNeweventsList([pbh.newOrchestratorStartedEvent()]), + ), + ); + await waitFor(() => orchestrationResponses.length > 0); + expect(observed).toEqual({ message: "hydrated" }); + const result = orchestrationResponses[0].getActionsList()[0].getCompleteorchestration()!.getResult()!.getValue(); + expect(await store.download(result)).toBe('{"message":"hydrated"}'); + }); + + it("leaves a streamed payload download failure retryable instead of failing the orchestration", async () => { + const errors = jest.fn(); + worker = new TaskHubGrpcWorker({ + hostAddress, + payloads: { store }, + logger: { + error: errors, + warn: () => {}, + info: () => {}, + debug: () => {}, + }, + }); + const flow = jest.fn(async () => "should not run"); + worker.addNamedOrchestrator("flow", flow); + store.values.clear(); + await worker.start(); + await waitFor(() => !!subscription); + subscription!.write( + new pb.WorkItem() + .setCompletiontoken("retryable") + .setOrchestratorrequest( + new pb.OrchestratorRequest() + .setInstanceid("instance") + .setRequireshistorystreaming(true) + .setNeweventsList([pbh.newOrchestratorStartedEvent()]), + ), + ); + await waitFor(() => errors.mock.calls.length > 0); + await waitFor(() => worker["_pendingWorkItems"].size === 0); + expect(flow).not.toHaveBeenCalled(); + expect(orchestrationResponses).toEqual([]); + }); + + it("collects large history chunks without the JavaScript variadic argument limit", async () => { + historyEvents = Array.from({ length: 150000 }, () => + new pb.HistoryEvent().setGenericevent(new pb.GenericEvent().setData(text("x"))), + ); + expect(await client.getOrchestrationHistory("instance")).toHaveLength(historyEvents.length); + }, 20000); +}); From d5c9df7e28b7d0bdc9ddaa01defa5f1a727aa880 Mon Sep 17 00:00:00 2001 From: wangbill Date: Mon, 19 Oct 2026 12:27:59 -0400 Subject: [PATCH 03/11] fix: externalize continue-as-new carryover payloads Traverse native carryover history through the existing payload field mapping and cover runtime generation, UTF-8 limits, wire size, and hydration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb2196a6-5b17-4d7f-b59b-51017b21de0a --- CHANGELOG.md | 2 + .../src/payloads/payload-processor.ts | 5 +- packages/durabletask-js/test/payloads.spec.ts | 123 ++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5429baa7..e896761b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ ### Fixes +- Apply payload externalization and UTF-8 size limits to native continue-as-new carryover history. + Event metadata, ordering, and hydrated payload values are preserved. - Prevent shared or duplicate pending tasks from stranding `whenAll` orchestrations after all activities finish. Notify every dependent composite, preserve duplicate result positions, and detach completed `whenAny` groups from their remaining children. diff --git a/packages/durabletask-js/src/payloads/payload-processor.ts b/packages/durabletask-js/src/payloads/payload-processor.ts index 235f6d3f..0e8ab491 100644 --- a/packages/durabletask-js/src/payloads/payload-processor.ts +++ b/packages/durabletask-js/src/payloads/payload-processor.ts @@ -7,7 +7,7 @@ import { newFailureDetails } from "../utils/pb-helper.util"; import { PayloadStorageException, PayloadStorageOptions } from "./payload-store"; import { runBounded } from "./run-bounded"; -/** Explicit protobuf field mapping, aligned with the .NET AzureBlobPayloads SideCar interceptor. */ +/** Explicit protobuf field mapping based on the .NET AzureBlobPayloads SideCar interceptor. */ export class PayloadProcessor { private readonly _threshold: number; private readonly _maximum: number; @@ -55,6 +55,9 @@ export class PayloadProcessor { action.getSendentitymessage()?.getEntityoperationsignaled()?.getInput(), action.getSendentitymessage()?.getEntityoperationcalled()?.getInput(), ); + for (const event of action.getCompleteorchestration()?.getCarryovereventsList() ?? []) { + this._historyFields(event, fields); + } for (const event of action.getRewindorchestration()?.getNewhistoryList() ?? []) { this._historyFields(event, fields); } diff --git a/packages/durabletask-js/test/payloads.spec.ts b/packages/durabletask-js/test/payloads.spec.ts index 96b8580c..5119ec82 100644 --- a/packages/durabletask-js/test/payloads.spec.ts +++ b/packages/durabletask-js/test/payloads.spec.ts @@ -6,6 +6,14 @@ import * as pb from "../src/proto/orchestrator_service_pb"; import { PayloadProcessor } from "../src/payloads/payload-processor"; import { PayloadStore, PayloadStorageException } from "../src/payloads/payload-store"; import { runBounded } from "../src/payloads/run-bounded"; +import { NoOpLogger } from "../src/types/logger.type"; +import { + newEventRaisedEvent, + newExecutionStartedEvent, + newOrchestratorStartedEvent, +} from "../src/utils/pb-helper.util"; +import { OrchestrationExecutor } from "../src/worker/orchestration-executor"; +import { Registry } from "../src/worker/registry"; class MemoryPayloadStore extends PayloadStore { readonly values = new Map(); @@ -26,6 +34,29 @@ class MemoryPayloadStore extends PayloadStore { } const text = (value: string) => new StringValue().setValue(value); +const MIB = 1024 * 1024; + +async function continueAsNewResponse(events: pb.HistoryEvent[]): Promise { + const registry = new Registry(); + registry.addNamedOrchestrator("Carryover", async function* (context) { + yield context.waitForExternalEvent("continue"); + context.continueAsNew("next", true, "next-version"); + }); + const result = await new OrchestrationExecutor(registry, new NoOpLogger()).execute( + "carryover-instance", + [], + [ + newOrchestratorStartedEvent(), + newExecutionStartedEvent("Carryover", "carryover-instance"), + ...events, + newEventRaisedEvent("continue", "null"), + ], + ); + return new pb.OrchestratorResponse() + .setInstanceid("carryover-instance") + .setCompletiontoken("completion") + .setActionsList(result.actions); +} describe("externalized payloads", () => { let store: MemoryPayloadStore; @@ -278,6 +309,98 @@ describe("externalized payloads", () => { processor = new PayloadProcessor({ store, thresholdBytes: 1 }); await expect(processor.externalize(new pb.ActivityResponse().setResult(text("large")))).rejects.toBe(error); }); + + describe("continue-as-new carryover", () => { + it("externalizes five 1 MiB runtime events below the 4 MiB wire limit and preserves hydrated history", async () => { + const values = Array.from({ length: 5 }, (_, index) => `${index}${"x".repeat(MIB - 3)}`); + const response = await continueAsNewResponse( + values.map((value, index) => newEventRaisedEvent(index < 2 ? "FIRST" : "SECOND", JSON.stringify(value))), + ); + const original = response.toObject(); + const action = response.getActionsList()[0]; + const completion = action.getCompleteorchestration()!; + const events = completion.getCarryovereventsList(); + expect(action.getId()).toBe(1); + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_CONTINUED_AS_NEW); + expect(events.map((event) => event.getEventraised()!.getName())).toEqual([ + "first", + "first", + "second", + "second", + "second", + ]); + expect(events.map((event) => Buffer.byteLength(event.getEventraised()!.getInput()!.getValue(), "utf8"))).toEqual( + Array(5).fill(MIB), + ); + expect(response.serializeBinary().byteLength).toBeGreaterThan(4 * MIB); + + await processor.externalize(response); + + const binary = response.serializeBinary(); + expect(binary.byteLength).toBeLessThan(4 * MIB); + expect(store.uploads).toBe(5); + expect([...store.values.values()]).toEqual(values.map((value) => JSON.stringify(value))); + const wireResponse = pb.OrchestratorResponse.deserializeBinary(binary); + const carryover = wireResponse.getActionsList()[0].getCompleteorchestration()!.getCarryovereventsList(); + expect( + carryover.every((event) => store.isKnownPayloadToken(event.getEventraised()!.getInput()!.getValue())), + ).toBe(true); + + await processor.resolve(new pb.HistoryChunk().setEventsList(carryover)); + + expect(carryover.map((event) => JSON.parse(event.getEventraised()!.getInput()!.getValue()))).toEqual(values); + expect(wireResponse.toObject()).toEqual(original); + }); + + it.each([ + ["below threshold", "\u00e9".repeat(131070) + "a", 262143, false], + ["at threshold", "\u00e9".repeat(131071), 262144, true], + ["above threshold", "\u00e9".repeat(131071) + "a", 262145, true], + ["below maximum", "\u00e9".repeat(524286) + "a", MIB - 1, true], + ["at maximum", "\u00e9".repeat(524287), MIB, true], + ])("applies serialized UTF-8 carryover boundaries %s", async (_label, value, size, externalized) => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const encoded = JSON.stringify(value); + expect(Buffer.byteLength(encoded, "utf8")).toBe(size); + const response = await continueAsNewResponse([newEventRaisedEvent("buffered", encoded)]); + + await processor.externalize(response); + + const completion = response.getActionsList()[0].getCompleteorchestration()!; + const input = completion.getCarryovereventsList()[0].getEventraised()!.getInput()!.getValue(); + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_CONTINUED_AS_NEW); + expect(store.uploads).toBe(externalized ? 1 : 0); + expect(store.isKnownPayloadToken(input)).toBe(externalized); + await processor.resolve(new pb.HistoryChunk().setEventsList(completion.getCarryovereventsList())); + expect(completion.getCarryovereventsList()[0].getEventraised()!.getInput()!.getValue()).toBe(encoded); + }); + + it.each([ + ["configured maximum", "\u00e9".repeat(524287) + "a", MIB, MIB + 1], + ["default maximum", "x".repeat(10 * MIB - 1), undefined, 10 * MIB + 1], + ])("converts carryover above the %s to a non-retriable failure", async (_label, value, maximum, size) => { + processor = new PayloadProcessor({ store, maxPayloadBytes: maximum }); + const encoded = JSON.stringify(value); + expect(Buffer.byteLength(encoded, "utf8")).toBe(size); + const response = await continueAsNewResponse([newEventRaisedEvent("buffered", encoded)]); + + await processor.externalize(response); + + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(response.getActionsList()).toHaveLength(1); + const completion = response.getActionsList()[0].getCompleteorchestration()!; + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(completion.getCarryovereventsList()).toEqual([]); + expect(completion.getFailuredetails()!.getErrortype()).toBe("PayloadStorageException"); + expect(completion.getFailuredetails()!.getIsnonretriable()).toBe(true); + expect(completion.getFailuredetails()!.getErrormessage()).toBe( + `Payload size ${size} bytes exceeds the configured maximum of ${maximum ?? 10 * MIB} bytes.`, + ); + expect(response.getInstanceid()).toBe("carryover-instance"); + expect(response.getCompletiontoken()).toBe("completion"); + }); + }); }); describe("bounded payload I/O", () => { From 55dfcf37da0ed3a2791f778d75621cb7f91144a7 Mon Sep 17 00:00:00 2001 From: wangbill Date: Mon, 19 Oct 2026 12:37:27 -0400 Subject: [PATCH 04/11] fix: preflight payload sizes before uploads Snapshot eligible UTF-8 payloads and validate every size before bounded storage I/O. Preserve precancellation, worker failure conversion, and message-order I/O failure handling without adding rollback deletion. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb2196a6-5b17-4d7f-b59b-51017b21de0a --- CHANGELOG.md | 4 + .../src/payloads/payload-processor.ts | 20 +- packages/durabletask-js/test/payloads.spec.ts | 329 ++++++++++++++++++ 3 files changed, 346 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e896761b..1040a155 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,10 @@ - Apply payload externalization and UTF-8 size limits to native continue-as-new carryover history. Event metadata, ordering, and hydrated payload values are preserved. +- Validate every eligible payload's UTF-8 size before starting uploads, preventing storage writes + and token mutations for deterministically oversized messages. Precancellation, worker failure + conversion, and bounded I/O error handling are preserved; transient I/O or submission failures + can still leave stored payloads. - Prevent shared or duplicate pending tasks from stranding `whenAll` orchestrations after all activities finish. Notify every dependent composite, preserve duplicate result positions, and detach completed `whenAny` groups from their remaining children. diff --git a/packages/durabletask-js/src/payloads/payload-processor.ts b/packages/durabletask-js/src/payloads/payload-processor.ts index 0e8ab491..143ad5be 100644 --- a/packages/durabletask-js/src/payloads/payload-processor.ts +++ b/packages/durabletask-js/src/payloads/payload-processor.ts @@ -74,23 +74,29 @@ export class PayloadProcessor { } else if (message instanceof pb.EntityRequest) { fields.push(message.getEntitystate()); } - const operations: (() => Promise)[] = []; + const candidates: { field: StringValue; value: string; size: number }[] = []; for (const field of fields) { const value = field?.getValue(); if (!field || !value) continue; const size = Buffer.byteLength(value, "utf8"); if (size < this._threshold) continue; - operations.push(async () => { + candidates.push({ field, value, size }); + } + try { + if (candidates.length) signal?.throwIfAborted(); + for (const { size } of candidates) { if (size > this._maximum) { throw new PayloadStorageException( `Payload size ${size} bytes exceeds the configured maximum of ${this._maximum} bytes.`, ); } - field.setValue(await this._options.store.upload(value, signal)); - }); - } - try { - await runBounded(operations, signal); + } + await runBounded( + candidates.map(({ field, value }) => async () => { + field.setValue(await this._options.store.upload(value, signal)); + }), + signal, + ); } catch (error) { if ( !isPermanentStorageFailure(error) || diff --git a/packages/durabletask-js/test/payloads.spec.ts b/packages/durabletask-js/test/payloads.spec.ts index 5119ec82..5c466e96 100644 --- a/packages/durabletask-js/test/payloads.spec.ts +++ b/packages/durabletask-js/test/payloads.spec.ts @@ -35,6 +35,15 @@ class MemoryPayloadStore extends PayloadStore { const text = (value: string) => new StringValue().setValue(value); const MIB = 1024 * 1024; +const nextTurn = () => new Promise((resolve) => setImmediate(resolve)); + +function gate() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} async function continueAsNewResponse(events: pb.HistoryEvent[]): Promise { const registry = new Registry(); @@ -401,6 +410,326 @@ describe("externalized payloads", () => { expect(response.getCompletiontoken()).toBe("completion"); }); }); + + describe("whole-message size preflight", () => { + it("fails an oversized action before uploading or mutating a valid custom status", async () => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const status = text("s".repeat(262144)); + const input = text("x".repeat(MIB + 1)); + const response = new pb.OrchestratorResponse() + .setInstanceid("instance") + .setCompletiontoken("completion") + .setCustomstatus(status) + .setIspartial(true) + .setChunkindex(new Int32Value().setValue(7)) + .setActionsList([new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(input))]); + + await processor.externalize(response); + + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(status.getValue()).toBe("s".repeat(262144)); + expect(input.getValue()).toBe("x".repeat(MIB + 1)); + expect(response.hasCustomstatus()).toBe(false); + expect(response.getIspartial()).toBe(false); + expect(response.hasChunkindex()).toBe(false); + expect(response.getActionsList()).toHaveLength(1); + const completion = response.getActionsList()[0].getCompleteorchestration()!; + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(completion.getFailuredetails()!.getErrortype()).toBe("PayloadStorageException"); + expect(completion.getFailuredetails()!.getIsnonretriable()).toBe(true); + expect(completion.getFailuredetails()!.getErrormessage()).toBe( + `Payload size ${MIB + 1} bytes exceeds the configured maximum of ${MIB} bytes.`, + ); + expect(response.getInstanceid()).toBe("instance"); + expect(response.getCompletiontoken()).toBe("completion"); + }); + + it("rejects an oversized entity operation before uploading or mutating valid state", async () => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text("s".repeat(262144))) + .setOperationsList([new pb.OperationRequest().setInput(text("x".repeat(MIB + 1)))]); + const original = request.toObject(); + + await expect(processor.externalize(request)).rejects.toBeInstanceOf(PayloadStorageException); + + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(request.toObject()).toEqual(original); + }); + + it("rejects oversized client input without storage or field mutations", async () => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const request = new pb.CreateInstanceRequest().setInput(text("x".repeat(MIB + 1))); + const original = request.toObject(); + + await expect(processor.externalize(request)).rejects.toBeInstanceOf(PayloadStorageException); + + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(request.toObject()).toEqual(original); + }); + + it("converts oversized activity output while preserving completion metadata", async () => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const result = text("x".repeat(MIB + 1)); + const response = new pb.ActivityResponse() + .setInstanceid("instance") + .setTaskid(17) + .setCompletiontoken("completion") + .setResult(result); + + await processor.externalize(response); + + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(result.getValue()).toBe("x".repeat(MIB + 1)); + expect(response.hasResult()).toBe(false); + expect(response.getFailuredetails()!.getErrortype()).toBe("PayloadStorageException"); + expect(response.getFailuredetails()!.getIsnonretriable()).toBe(true); + expect(response.getFailuredetails()!.getErrormessage()).toBe( + `Payload size ${MIB + 1} bytes exceeds the configured maximum of ${MIB} bytes.`, + ); + expect(response.getInstanceid()).toBe("instance"); + expect(response.getTaskid()).toBe(17); + expect(response.getCompletiontoken()).toBe("completion"); + }); + + it("detects invalid size before a hypothetical earlier transient upload failure", async () => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const upload = jest + .spyOn(store, "upload") + .mockRejectedValue(Object.assign(new Error("offline"), { statusCode: 503 })); + const response = new pb.OrchestratorResponse() + .setCustomstatus(text("s".repeat(262144))) + .setActionsList([ + new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(text("x".repeat(MIB + 1)))), + ]); + + await expect(processor.externalize(response)).resolves.toBeUndefined(); + + expect(upload).not.toHaveBeenCalled(); + expect(store.values.size).toBe(0); + const completion = response.getActionsList()[0].getCompleteorchestration()!; + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(completion.getFailuredetails()!.getErrortype()).toBe("PayloadStorageException"); + expect(completion.getFailuredetails()!.getIsnonretriable()).toBe(true); + }); + + it.each([ + () => new pb.CreateInstanceRequest().setInput(text("x".repeat(MIB + 1))), + () => new pb.ActivityResponse().setResult(text("x".repeat(MIB + 1))), + () => new pb.EntityBatchRequest().setEntitystate(text("x".repeat(MIB + 1))), + () => + new pb.OrchestratorResponse() + .setCustomstatus(text("s".repeat(262144))) + .setIspartial(true) + .setChunkindex(new Int32Value().setValue(7)) + .setActionsList([ + new pb.OrchestratorAction().setScheduletask( + new pb.ScheduleTaskAction().setInput(text("x".repeat(MIB + 1))), + ), + ]), + ])("honors precancellation before deterministic size failure (%#)", async (makeMessage) => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const abort = new AbortController(); + abort.abort(); + const message = makeMessage(); + const original = message.toObject(); + + await expect(processor.externalize(message, abort.signal)).rejects.toBe(abort.signal.reason); + + expect(abort.signal.reason.name).toBe("AbortError"); + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(message.toObject()).toEqual(original); + }); + + it.each([ + () => new pb.CreateInstanceRequest(), + () => new pb.CreateInstanceRequest().setInput(text("")), + () => new pb.CreateInstanceRequest().setInput(text("abc")), + ])( + "keeps canceled no-op messages unchanged even when the maximum is below the threshold (%#)", + async (makeMessage) => { + processor = new PayloadProcessor({ store, thresholdBytes: 4, maxPayloadBytes: 2 }); + const abort = new AbortController(); + abort.abort(); + const message = makeMessage(); + const original = message.toObject(); + + await expect(processor.externalize(message, abort.signal)).resolves.toBeUndefined(); + + expect(store.uploads).toBe(0); + expect(message.toObject()).toEqual(original); + }, + ); + + it("externalizes every eligible valid field and leaves inline, empty, and absent values untouched", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 4, maxPayloadBytes: 4 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text("1234")) + .setOperationsList([ + new pb.OperationRequest().setInput(text("\u00e9\u00e9")), + new pb.OperationRequest().setInput(text("abc")), + new pb.OperationRequest().setInput(text("")), + new pb.OperationRequest(), + ]); + const original = request.toObject(); + + await processor.externalize(request); + + expect([...store.values.values()]).toEqual(["1234", "\u00e9\u00e9"]); + expect(request.getEntitystate()!.getValue()).toBe("test:1"); + expect(request.getOperationsList()[0].getInput()!.getValue()).toBe("test:2"); + expect(request.getOperationsList()[1].getInput()!.getValue()).toBe("abc"); + expect(request.getOperationsList()[2].getInput()!.getValue()).toBe(""); + expect(request.getOperationsList()[3].hasInput()).toBe(false); + await processor.resolve(request); + expect(request.toObject()).toEqual(original); + }); + }); + + describe("in-flight payload I/O", () => { + it.each([false, true])( + "classifies the lowest-message-order failure after drainage (permanent: %s)", + async (permanent) => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const earlier = gate(); + const later = gate(); + const bothStarted = gate(); + const laterFinished = gate(); + const firstError = Object.assign(new Error("earlier failure"), { statusCode: permanent ? 403 : 503 }); + const secondError = permanent + ? Object.assign(new Error("later transient failure"), { statusCode: 503 }) + : new PayloadStorageException("later permanent failure"); + const started: string[] = []; + const finished: string[] = []; + let active = 0; + let settled = false; + store.upload = async (value) => { + started.push(value); + active++; + if (started.length === 2) bothStarted.release(); + try { + await (value === "earlier" ? earlier.promise : later.promise); + throw value === "earlier" ? firstError : secondError; + } finally { + finished.push(value); + active--; + if (value === "later") laterFinished.release(); + } + }; + const response = new pb.OrchestratorResponse() + .setCustomstatus(text("earlier")) + .setIspartial(true) + .setChunkindex(new Int32Value().setValue(2)) + .setActionsList([ + new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(text("later"))), + ]); + const original = response.toObject(); + const pending = processor.externalize(response).then( + () => { + settled = true; + }, + (error: unknown) => { + settled = true; + return error; + }, + ); + await bothStarted.promise; + later.release(); + await laterFinished.promise; + await nextTurn(); + const beforeDrain = { settled, active, finished: [...finished] }; + earlier.release(); + const error = await pending; + + expect(beforeDrain).toEqual({ settled: false, active: 1, finished: ["later"] }); + expect(started).toEqual(["earlier", "later"]); + expect(finished).toEqual(["later", "earlier"]); + expect(active).toBe(0); + if (permanent) { + expect(error).toBeUndefined(); + expect(response.hasCustomstatus()).toBe(false); + expect(response.getIspartial()).toBe(false); + expect(response.hasChunkindex()).toBe(false); + expect(response.getActionsList()).toHaveLength(1); + const completion = response.getActionsList()[0].getCompleteorchestration()!; + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(completion.getFailuredetails()!.getErrormessage()).toBe("earlier failure"); + expect(completion.getFailuredetails()!.getIsnonretriable()).toBe(true); + } else { + expect(error).toBe(firstError); + expect(response.toObject()).toEqual(original); + } + const atSettlement = response.toObject(); + await nextTurn(); + expect(response.toObject()).toEqual(atSettlement); + }, + ); + + it.each(["externalize", "resolve"] as const)( + "drains started %s mutations before cancellation settles", + async (mode) => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const abort = new AbortController(); + const started = gate(); + const hold = gate(); + const first = mode === "externalize" ? "first" : "test:first"; + const second = mode === "externalize" ? "second" : "test:second"; + store.values.set("test:first", "resolved-first"); + store.values.set("test:second", "resolved-second"); + const upload = store.upload.bind(store); + const download = store.download.bind(store); + const calls: string[] = []; + let active = 0; + let settled = false; + const io = async (value: string) => { + calls.push(value); + active++; + abort.abort(); + started.release(); + try { + await hold.promise; + return mode === "externalize" ? await upload(value) : await download(value); + } finally { + active--; + } + }; + if (mode === "externalize") store.upload = io; + else store.download = io; + const request = new pb.EntityBatchRequest() + .setEntitystate(text(first)) + .setOperationsList([new pb.OperationRequest().setInput(text(second))]); + const pending = processor[mode](request, abort.signal).then( + () => { + settled = true; + }, + (error: unknown) => { + settled = true; + return error; + }, + ); + await started.promise; + await nextTurn(); + const beforeDrain = { settled, active, state: request.getEntitystate()!.getValue() }; + hold.release(); + const error = await pending; + + expect(beforeDrain).toEqual({ settled: false, active: 1, state: first }); + expect(calls).toEqual([first]); + expect(error).toBe(abort.signal.reason); + expect(active).toBe(0); + expect(request.getEntitystate()!.getValue()).toBe(mode === "externalize" ? "test:1" : "resolved-first"); + expect(request.getOperationsList()[0].getInput()!.getValue()).toBe(second); + const atSettlement = request.toObject(); + await nextTurn(); + expect(request.toObject()).toEqual(atSettlement); + }, + ); + }); }); describe("bounded payload I/O", () => { From 3fab3aace00ffdffdd17110062fcc39a2debe0fe Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 20 Oct 2026 18:39:39 -0400 Subject: [PATCH 05/11] fix: abandon queued work on payload processing failure Release acquired work through the captured task-specific abandon RPC after unconverted payload failures, preserving host errors and cancellation-aware bounded delivery. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 3 + README.md | 7 + .../src/worker/task-hub-grpc-worker.ts | 77 ++++-- .../test/functions-grpc-support.spec.ts | 47 ++++ .../test/payload-transport.spec.ts | 225 +++++++++++++--- .../test/worker-response-delivery.spec.ts | 255 +++++++++++++++++- 6 files changed, 557 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1040a155..1b23d918 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,9 @@ ### Fixes +- Abandon acquired worker work items when enabled payload hydration or unconverted output storage + processing fails, awaiting shutdown-aware bounded release retries without creating fake business + failures. Tokenless host processing preserves unconverted storage errors. - Apply payload externalization and UTF-8 size limits to native continue-as-new carryover history. Event metadata, ordering, and hydrated payload values are preserved. - Validate every eligible payload's UTF-8 size before starting uploads, preventing storage writes diff --git a/README.md b/README.md index 17ffbf62..9273cd52 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,13 @@ Requests are externalized before sending; queries, history chunks and work items before conversion or execution. Empty payloads remain inline, and disabling the option preserves the existing transport behavior. +Workers abandon acquired queue work items when payload downloads or unconverted output storage/size +checks fail. Release is awaited with bounded gRPC retries and the original work item's cancellation +signal; release failures are logged, and shutdown prevents late sends. Permanent activity and +orchestration output failures still produce non-retriable failed completions. Tokenless host APIs +(`processOrchestratorRequest` and `processEntityBatchRequest`) instead reject with the original +unconverted storage error so the host can apply its own retry policy. + Azure-managed builders expose `.useExternalizedPayloads({ store })` and require core `>=0.4.1` for the public payload types. This phase works with a custom/in-memory store and does not import an Azure Blob provider or register cleanup tasks. Complete the six-phase feature stack before release. diff --git a/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts b/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts index 46284b60..b5b8d35b 100644 --- a/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts +++ b/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts @@ -964,11 +964,23 @@ export class TaskHubGrpcWorker { return undefined; } - private _trackPendingWorkItem(workPromise: Promise, onError: (error: Error) => void): void { + private _trackPendingWorkItem( + workPromise: Promise, + onError: (error: Error) => void, + abandon: () => Promise, + signal?: AbortSignal, + ): void { const handledPromise = workPromise - .catch((e: unknown) => { + .catch(async (e: unknown) => { const error = e instanceof Error ? e : new Error(String(e)); onError(error); + if (this._payloads && !signal?.aborted) { + try { + await abandon(); + } catch (error: unknown) { + WorkerLogs.workerError(this._logger, error); + } + } }) .finally(() => { this._pendingWorkItems.delete(handledPromise); @@ -982,7 +994,6 @@ export class TaskHubGrpcWorker { request: TReq, signal?: AbortSignal, ): Promise { - if (this._payloads) await this._payloads.externalize(request, signal); const backoff = new ExponentialBackoff({ initialDelayMs: 200, maxDelayMs: 15000, @@ -1019,9 +1030,17 @@ export class TaskHubGrpcWorker { ): void { const retrySignal = this._abortController?.signal; const workPromise = this._executeOrchestratorInternal(req, completionToken, stub, retrySignal); - this._trackPendingWorkItem(workPromise, (error) => { - WorkerLogs.executionError(this._logger, req.getInstanceid() || "(unknown)", error); - }); + this._trackPendingWorkItem( + workPromise, + (error) => WorkerLogs.executionError(this._logger, req.getInstanceid() || "(unknown)", error), + () => + this._deliverResponse( + stub.abandonTaskOrchestratorWorkItem.bind(stub), + new pb.AbandonOrchestrationTaskRequest().setCompletiontoken(completionToken), + retrySignal, + ), + retrySignal, + ); } private async _streamOrchestrationHistory( @@ -1314,6 +1333,8 @@ export class TaskHubGrpcWorker { endSpan(tracingResult?.span); } + // Storage failures must reach the queue boundary, not become failed business completions. + if (this._payloads) await this._payloads.externalize(res, retrySignal); try { await this._deliverResponse(stub.completeOrchestratorTask.bind(stub), res, retrySignal); } catch (e: unknown) { @@ -1332,9 +1353,17 @@ export class TaskHubGrpcWorker { ): void { const retrySignal = this._abortController?.signal; const workPromise = this._executeActivityInternal(req, completionToken, stub, retrySignal); - this._trackPendingWorkItem(workPromise, (error) => { - WorkerLogs.workerError(this._logger, error); - }); + this._trackPendingWorkItem( + workPromise, + (error) => WorkerLogs.workerError(this._logger, error), + () => + this._deliverResponse( + stub.abandonTaskActivityWorkItem.bind(stub), + new pb.AbandonActivityTaskRequest().setCompletiontoken(completionToken), + retrySignal, + ), + retrySignal, + ); } /** @@ -1416,6 +1445,7 @@ export class TaskHubGrpcWorker { endSpan(activitySpan); } + if (this._payloads) await this._payloads.externalize(res, retrySignal); try { await this._deliverResponse(stub.completeActivityTask.bind(stub), res, retrySignal); } catch (e: unknown) { @@ -1435,9 +1465,17 @@ export class TaskHubGrpcWorker { ): void { const retrySignal = this._abortController?.signal; const workPromise = this._executeEntityInternal(req, completionToken, stub, operationInfos, retrySignal); - this._trackPendingWorkItem(workPromise, (error) => { - WorkerLogs.workerError(this._logger, error); - }); + this._trackPendingWorkItem( + workPromise, + (error) => WorkerLogs.workerError(this._logger, error), + () => + this._deliverResponse( + stub.abandonTaskEntityWorkItem.bind(stub), + new pb.AbandonEntityTaskRequest().setCompletiontoken(completionToken), + retrySignal, + ), + retrySignal, + ); } /** @@ -1539,9 +1577,17 @@ export class TaskHubGrpcWorker { ): void { const retrySignal = this._abortController?.signal; const workPromise = this._executeEntityV2Internal(req, completionToken, stub, retrySignal); - this._trackPendingWorkItem(workPromise, (error) => { - WorkerLogs.workerError(this._logger, error); - }); + this._trackPendingWorkItem( + workPromise, + (error) => WorkerLogs.workerError(this._logger, error), + () => + this._deliverResponse( + stub.abandonTaskEntityWorkItem.bind(stub), + new pb.AbandonEntityTaskRequest().setCompletiontoken(completionToken), + retrySignal, + ), + retrySignal, + ); } /** @@ -1697,6 +1743,7 @@ export class TaskHubGrpcWorker { stub: stubs.TaskHubSidecarServiceClient, retrySignal?: AbortSignal, ): Promise { + if (this._payloads) await this._payloads.externalize(batchResult, retrySignal); try { await this._deliverResponse(stub.completeEntityTask.bind(stub), batchResult, retrySignal); } catch (e: any) { diff --git a/packages/durabletask-js/test/functions-grpc-support.spec.ts b/packages/durabletask-js/test/functions-grpc-support.spec.ts index c6a55717..293c98a5 100644 --- a/packages/durabletask-js/test/functions-grpc-support.spec.ts +++ b/packages/durabletask-js/test/functions-grpc-support.spec.ts @@ -3,6 +3,8 @@ import { OrchestrationContext, + PayloadStore, + PayloadStorageException, TaskEntity, TaskHubGrpcWorker, TOrchestrator, @@ -12,6 +14,7 @@ import { import * as pb from "../src/proto/orchestrator_service_pb"; import { newExecutionStartedEvent, newOrchestratorStartedEvent } from "../src/utils/pb-helper.util"; import { NoOpLogger } from "../src/types/logger.type"; +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; const TEST_INSTANCE_ID = "functions-grpc-instance"; @@ -88,6 +91,50 @@ describe("Functions gRPC support surface", () => { expect(response.getResultsList()[0].getSuccess()?.getResult()?.getValue()).toBe("1"); expect(response.getEntitystate()?.getValue()).toBe("1"); }); + + it.each([ + { kind: "orchestrator", phase: "download" }, + { kind: "entity", phase: "download" }, + { kind: "orchestrator", phase: "upload" }, + { kind: "entity", phase: "upload" }, + { kind: "entity", phase: "maximum" }, + ])("preserves the original $phase failure for a tokenless $kind host invocation", async ({ kind, phase }) => { + const failure = Object.assign(new Error("storage unavailable"), { statusCode: 503 }); + class Store extends PayloadStore { + async upload(): Promise { + throw failure; + } + async download(): Promise { + throw failure; + } + isKnownPayloadToken(value: string): boolean { + return value.startsWith("payload:"); + } + } + const worker = new TaskHubGrpcWorker({ + payloads: { store: new Store(), thresholdBytes: 1, maxPayloadBytes: phase === "maximum" ? 1 : undefined }, + logger: new NoOpLogger(), + }); + worker.addNamedOrchestrator("flow", async () => "output"); + worker.addNamedEntity("counter", () => new CounterEntity()); + const input = new StringValue().setValue(phase === "download" ? "payload:input" : "10"); + const request = + kind === "orchestrator" + ? new pb.OrchestratorRequest() + .setInstanceid(TEST_INSTANCE_ID) + .setNeweventsList([ + newOrchestratorStartedEvent(), + newExecutionStartedEvent("flow", TEST_INSTANCE_ID, input.getValue()), + ]) + : createEntityBatchRequest("counter", "key").setEntitystate(input); + const processing = + kind === "orchestrator" + ? worker.processOrchestratorRequest(request.serializeBinary()) + : worker.processEntityBatchRequest(request.serializeBinary()); + if (phase === "maximum") await expect(processing).rejects.toBeInstanceOf(PayloadStorageException); + else await expect(processing).rejects.toBe(failure); + expect(worker["_pendingWorkItems"].size).toBe(0); + }); }); function createEntityBatchRequest(entityName: string, entityKey: string): pb.EntityBatchRequest { diff --git a/packages/durabletask-js/test/payload-transport.spec.ts b/packages/durabletask-js/test/payload-transport.spec.ts index a1bc197b..5e6f2be4 100644 --- a/packages/durabletask-js/test/payload-transport.spec.ts +++ b/packages/durabletask-js/test/payload-transport.spec.ts @@ -4,12 +4,57 @@ import * as grpc from "@grpc/grpc-js"; import { Empty } from "google-protobuf/google/protobuf/empty_pb"; import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; -import { TaskHubGrpcClient, TaskHubGrpcWorker, PayloadStore, NoOpLogger } from "../src"; +import { TaskHubGrpcClient, TaskHubGrpcWorker, PayloadStore, NoOpLogger, TaskEntity } from "../src"; import * as pb from "../src/proto/orchestrator_service_pb"; import * as stubs from "../src/proto/orchestrator_service_grpc_pb"; import * as pbh from "../src/utils/pb-helper.util"; const text = (value: string) => new StringValue().setValue(value); +type WorkKind = "activity" | "orchestrator" | "streamed-orchestrator" | "entity-v1" | "entity-v2"; + +function workItem(kind: WorkKind, input: string, token: string): pb.WorkItem { + const item = new pb.WorkItem().setCompletiontoken(token); + if (kind === "activity") { + return item.setActivityrequest( + new pb.ActivityRequest() + .setName("echo") + .setTaskid(1) + .setInput(text(input)) + .setOrchestrationinstance(new pb.OrchestrationInstance().setInstanceid("instance")), + ); + } + if (kind === "orchestrator" || kind === "streamed-orchestrator") { + return item.setOrchestratorrequest( + new pb.OrchestratorRequest() + .setInstanceid("instance") + .setRequireshistorystreaming(kind === "streamed-orchestrator") + .setNeweventsList( + kind === "streamed-orchestrator" + ? [pbh.newOrchestratorStartedEvent()] + : [pbh.newOrchestratorStartedEvent(), pbh.newExecutionStartedEvent("flow", "instance", input)], + ), + ); + } + if (kind === "entity-v1") { + return item.setEntityrequest( + new pb.EntityBatchRequest() + .setInstanceid("@echo@key") + .setOperationsList([ + new pb.OperationRequest().setOperation("echo").setRequestid("request").setInput(text(input)), + ]), + ); + } + return item.setEntityrequestv2( + new pb.EntityRequest() + .setInstanceid("@echo@key") + .setOperationrequestsList([ + new pb.HistoryEvent().setEntityoperationcalled( + new pb.EntityOperationCalledEvent().setOperation("echo").setRequestid("request").setInput(text(input)), + ), + ]), + ); +} + class MemoryStore extends PayloadStore { readonly values = new Map(); async upload(value: string): Promise { @@ -47,6 +92,11 @@ describe("transparent payload transport over gRPC", () => { let activityCalls: number; let requestedInput: string | undefined; let orchestrationResponses: pb.OrchestratorResponse[]; + let activityResponses: pb.ActivityResponse[]; + let entityResponses: pb.EntityBatchResult[]; + let abandonments: { kind: string; token: string }[]; + let onCompleteActivity: stubs.ITaskHubSidecarServiceServer["completeActivityTask"]; + let errors: jest.Mock; let historyEvents: pb.HistoryEvent[]; beforeEach(async () => { @@ -57,6 +107,11 @@ describe("transparent payload transport over gRPC", () => { activityCalls = 0; requestedInput = undefined; orchestrationResponses = []; + activityResponses = []; + entityResponses = []; + abandonments = []; + errors = jest.fn(); + onCompleteActivity = (_call, callback) => callback(null, new pb.CompleteTaskResponse()); historyEvents = [pbh.newExecutionStartedEvent("flow", "instance", input)]; server = new grpc.Server(); const state = () => @@ -88,12 +143,29 @@ describe("transparent payload transport over gRPC", () => { }, completeActivityTask: (call, callback) => { activityResponse = call.request; - callback(null, new pb.CompleteTaskResponse()); + activityResponses.push(call.request); + onCompleteActivity(call, callback); }, completeOrchestratorTask: (call, callback) => { orchestrationResponses.push(call.request); callback(null, new pb.CompleteTaskResponse()); }, + completeEntityTask: (call, callback) => { + entityResponses.push(call.request); + callback(null, new pb.CompleteTaskResponse()); + }, + abandonTaskActivityWorkItem: (call, callback) => { + abandonments.push({ kind: "activity", token: call.request.getCompletiontoken() }); + callback(null, new pb.AbandonActivityTaskResponse()); + }, + abandonTaskOrchestratorWorkItem: (call, callback) => { + abandonments.push({ kind: "orchestrator", token: call.request.getCompletiontoken() }); + callback(null, new pb.AbandonOrchestrationTaskResponse()); + }, + abandonTaskEntityWorkItem: (call, callback) => { + abandonments.push({ kind: "entity", token: call.request.getCompletiontoken() }); + callback(null, new pb.AbandonEntityTaskResponse()); + }, } satisfies Pick< stubs.ITaskHubSidecarServiceServer, | "hello" @@ -105,6 +177,10 @@ describe("transparent payload transport over gRPC", () => { | "getWorkItems" | "completeActivityTask" | "completeOrchestratorTask" + | "completeEntityTask" + | "abandonTaskActivityWorkItem" + | "abandonTaskOrchestratorWorkItem" + | "abandonTaskEntityWorkItem" >; server.addService(stubs.TaskHubSidecarServiceService, service); const port = await new Promise((resolve, reject) => @@ -126,8 +202,32 @@ describe("transparent payload transport over gRPC", () => { if (worker["_isRunning"]) await worker.stop(); await client.stop(); server.forceShutdown(); + jest.restoreAllMocks(); }); + function registerPayloadWorker(output?: unknown, maxPayloadBytes?: number): jest.Mock { + worker = new TaskHubGrpcWorker({ + hostAddress, + payloads: { store, thresholdBytes: 1, maxPayloadBytes }, + logger: { error: errors, warn: () => {}, info: () => {}, debug: () => {} }, + }); + const invoked = jest.fn((value: unknown) => output ?? value); + worker.addNamedActivity("echo", async (_context, value) => invoked(value)); + worker.addNamedOrchestrator("flow", async (_context, value) => invoked(value)); + class Echo extends TaskEntity { + echo(value: unknown) { + return invoked(value); + } + protected initializeState(): null { + return null; + } + } + worker.addNamedEntity("echo", () => new Echo()); + return invoked; + } + + const completeCount = () => activityResponses.length + orchestrationResponses.length + entityResponses.length; + it("externalizes before sending and hydrates state, wait, query and streamed history before conversion", async () => { await client.scheduleNewOrchestration("workflow", { message: "request" }); expect(requestedInput).toMatch(/^payload:/); @@ -158,6 +258,7 @@ describe("transparent payload transport over gRPC", () => { ); await waitFor(() => !!activityResponse); expect(activityCalls).toBe(1); + expect(abandonments).toEqual([]); expect(activityResponse!.getCompletiontoken()).toBe("completion"); expect(await store.download(activityResponse!.getResult()!.getValue())).toBe('{"message":"hydrated"}'); }); @@ -191,37 +292,101 @@ describe("transparent payload transport over gRPC", () => { expect(await store.download(result)).toBe('{"message":"hydrated"}'); }); - it("leaves a streamed payload download failure retryable instead of failing the orchestration", async () => { - const errors = jest.fn(); - worker = new TaskHubGrpcWorker({ - hostAddress, - payloads: { store }, - logger: { - error: errors, - warn: () => {}, - info: () => {}, - debug: () => {}, + it.each(["activity", "orchestrator", "streamed-orchestrator", "entity-v1", "entity-v2"])( + "abandons %s on download failure and permits redelivery after storage recovers", + async (kind) => { + const invoked = registerPayloadWorker(); + const failure = Object.assign(new Error("download unavailable"), { statusCode: 503 }); + const download = jest.spyOn(store, "download").mockRejectedValueOnce(failure); + await worker.start(); + await waitFor(() => !!subscription); + const stream = worker["_responseStream"]; + subscription!.write(workItem(kind, input, "retryable")); + await waitFor(() => errors.mock.calls.length > 0 && worker["_pendingWorkItems"].size === 0); + expect(invoked).not.toHaveBeenCalled(); + expect(download).toHaveBeenCalledTimes(1); + expect(completeCount()).toBe(0); + expect(abandonments).toEqual([ + { + kind: kind.startsWith("entity") ? "entity" : kind === "activity" ? "activity" : "orchestrator", + token: "retryable", + }, + ]); + expect(subscription!.cancelled).toBe(false); + expect(worker["_responseStream"]).toBe(stream); + + subscription!.write(workItem(kind, input, "redelivered")); + await waitFor(() => completeCount() === 1 && worker["_pendingWorkItems"].size === 0); + expect(download).toHaveBeenCalledTimes(2); + expect(invoked).toHaveBeenCalledTimes(1); + expect(invoked).toHaveBeenCalledWith({ message: "hydrated" }); + expect(abandonments).toHaveLength(1); + }, + ); + + it.each<{ kind: WorkKind; maximum?: number }>([ + { kind: "activity" }, + { kind: "orchestrator" }, + { kind: "entity-v1" }, + { kind: "entity-v2" }, + { kind: "entity-v1", maximum: 64 }, + { kind: "entity-v2", maximum: 64 }, + ])("abandons $kind when output externalization fails (maximum=$maximum)", async ({ kind, maximum }) => { + const invoked = registerPayloadWorker("x".repeat(128), maximum); + const upload = jest.spyOn(store, "upload"); + if (!maximum) upload.mockRejectedValueOnce(Object.assign(new Error("upload unavailable"), { statusCode: 503 })); + await worker.start(); + await waitFor(() => !!subscription); + const stream = worker["_responseStream"]; + subscription!.write(workItem(kind, '"inline"', "output-failure")); + await waitFor(() => errors.mock.calls.length > 0 && worker["_pendingWorkItems"].size === 0); + expect(invoked).toHaveBeenCalledTimes(1); + expect(upload).toHaveBeenCalledTimes(maximum ? 0 : 1); + expect(completeCount()).toBe(0); + expect(abandonments).toEqual([ + { + kind: kind.startsWith("entity") ? "entity" : kind === "activity" ? "activity" : "orchestrator", + token: "output-failure", }, - }); - const flow = jest.fn(async () => "should not run"); - worker.addNamedOrchestrator("flow", flow); - store.values.clear(); + ]); + expect(subscription!.cancelled).toBe(false); + expect(worker["_responseStream"]).toBe(stream); + }); + + it.each(["activity", "orchestrator"])( + "preserves permanent %s output failure conversion without abandoning", + async (kind) => { + registerPayloadWorker("x".repeat(128), 64); + await worker.start(); + await waitFor(() => !!subscription); + subscription!.write(workItem(kind, '"inline"', "permanent")); + await waitFor(() => completeCount() === 1 && worker["_pendingWorkItems"].size === 0); + const failure = + kind === "activity" + ? activityResponses[0].getFailuredetails() + : orchestrationResponses[0].getActionsList()[0].getCompleteorchestration()?.getFailuredetails(); + expect(failure?.getErrortype()).toBe("PayloadStorageException"); + expect(failure?.getIsnonretriable()).toBe(true); + expect(abandonments).toEqual([]); + }, + ); + + it("retries a transient completion RPC with the same externalized response and only one upload", async () => { + const invoked = registerPayloadWorker("output"); + const upload = jest.spyOn(store, "upload"); + onCompleteActivity = (_call, callback) => { + if (activityResponses.length === 1) callback({ code: grpc.status.UNAVAILABLE, details: "retry completion" }); + else callback(null, new pb.CompleteTaskResponse()); + }; await worker.start(); await waitFor(() => !!subscription); - subscription!.write( - new pb.WorkItem() - .setCompletiontoken("retryable") - .setOrchestratorrequest( - new pb.OrchestratorRequest() - .setInstanceid("instance") - .setRequireshistorystreaming(true) - .setNeweventsList([pbh.newOrchestratorStartedEvent()]), - ), - ); - await waitFor(() => errors.mock.calls.length > 0); - await waitFor(() => worker["_pendingWorkItems"].size === 0); - expect(flow).not.toHaveBeenCalled(); - expect(orchestrationResponses).toEqual([]); + subscription!.write(workItem("activity", '"inline"', "response-retry")); + await waitFor(() => activityResponses.length === 2 && worker["_pendingWorkItems"].size === 0); + expect(invoked).toHaveBeenCalledTimes(1); + expect(upload).toHaveBeenCalledTimes(1); + expect(activityResponses[0].getResult()?.getValue()).toMatch(/^payload:/); + expect(activityResponses[1].serializeBinary()).toEqual(activityResponses[0].serializeBinary()); + expect(abandonments).toEqual([]); }); it("collects large history chunks without the JavaScript variadic argument limit", async () => { diff --git a/packages/durabletask-js/test/worker-response-delivery.spec.ts b/packages/durabletask-js/test/worker-response-delivery.spec.ts index 7de57a9d..bda5759b 100644 --- a/packages/durabletask-js/test/worker-response-delivery.spec.ts +++ b/packages/durabletask-js/test/worker-response-delivery.spec.ts @@ -8,6 +8,7 @@ import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; import { Value } from "google-protobuf/google/protobuf/struct_pb"; import { newExecutionStartedEvent, newOrchestratorStartedEvent } from "../src/utils/pb-helper.util"; import { TaskEntity } from "../src/entities/task-entity"; +import { PayloadStore } from "../src/payloads/payload-store"; import * as pb from "../src/proto/orchestrator_service_pb"; import * as stubs from "../src/proto/orchestrator_service_grpc_pb"; import { TaskHubGrpcWorker } from "../src/worker/task-hub-grpc-worker"; @@ -33,6 +34,14 @@ function activityRequest() { .setOrchestrationinstance(new pb.OrchestrationInstance().setInstanceid("instance")); } +class WorkerPayloadStore extends PayloadStore { + upload = jest.fn(async (_value: string, _signal?: AbortSignal) => "payload:output"); + download = jest.fn(async (_token: string, _signal?: AbortSignal) => '"input"'); + isKnownPayloadToken(value: string): boolean { + return value.startsWith("payload:"); + } +} + describe("Worker response retries", () => { let stub: stubs.TaskHubSidecarServiceClient; let worker: TaskHubGrpcWorker; @@ -64,19 +73,24 @@ describe("Worker response retries", () => { observed(ctx.sourceInstanceId, ctx.instanceId, ctx.version); return "done"; }); - const send = jest.spyOn(stub, "completeOrchestratorTask").mockImplementation( - (_request, _metadata, optionsOrCallback: Partial | Callback, callback?: Callback) => { - const respond = typeof optionsOrCallback === "function" ? optionsOrCallback : callback!; - respond(null, new pb.CompleteTaskResponse()); - return unaryCall(); - }, - ); - const request = new pb.OrchestratorRequest().setInstanceid("clone-id").setNeweventsList([ - newOrchestratorStartedEvent(), - newExecutionStartedEvent("Flow", "clone-id", undefined, undefined, "execution-id", "1.0"), - ]); + const send = jest + .spyOn(stub, "completeOrchestratorTask") + .mockImplementation( + (_request, _metadata, optionsOrCallback: Partial | Callback, callback?: Callback) => { + const respond = typeof optionsOrCallback === "function" ? optionsOrCallback : callback!; + respond(null, new pb.CompleteTaskResponse()); + return unaryCall(); + }, + ); + const request = new pb.OrchestratorRequest() + .setInstanceid("clone-id") + .setNeweventsList([ + newOrchestratorStartedEvent(), + newExecutionStartedEvent("Flow", "clone-id", undefined, undefined, "execution-id", "1.0"), + ]); request.setExecutionid(new StringValue().setValue("execution-id")); - if (sourceInstanceId !== undefined) request.getPropertiesMap().set("sourceInstanceId", Value.fromJavaScript(sourceInstanceId)); + if (sourceInstanceId !== undefined) + request.getPropertiesMap().set("sourceInstanceId", Value.fromJavaScript(sourceInstanceId)); const item = new pb.WorkItem().setCompletiontoken("token").setOrchestratorrequest(request); worker["_dispatchWorkItem"](pb.WorkItem.deserializeBinary(item.serializeBinary()), stub); await Promise.all(worker["_pendingWorkItems"]); @@ -391,4 +405,221 @@ describe("Worker response retries", () => { await stoppingNewRun; } }); + + describe("payload failure release", () => { + let store: WorkerPayloadStore; + let abandon: jest.SpiedFunction; + let complete: jest.SpiedFunction; + let onAbandon: (request: pb.AbandonActivityTaskRequest, callback: Callback) => void; + let cancel: jest.Mock; + const failure = Object.assign(new Error("payload storage unavailable"), { statusCode: 503 }); + + beforeEach(() => { + store = new WorkerPayloadStore(); + worker = new TaskHubGrpcWorker({ payloads: { store, thresholdBytes: 1 }, logger, shutdownTimeoutMs: 100 }); + worker["_abortController"] = controller; + worker["_stub"] = stub; + worker.addNamedActivity("answer", () => 42); + cancel = jest.fn(); + onAbandon = (_request, callback) => callback(null, new pb.AbandonActivityTaskResponse()); + abandon = jest + .spyOn(stub, "abandonTaskActivityWorkItem") + .mockImplementation( + ( + request, + _metadata, + optionsOrCallback: Partial | Callback, + callback?: Callback, + ) => { + onAbandon(request, typeof optionsOrCallback === "function" ? optionsOrCallback : callback!); + return unaryCall(cancel); + }, + ); + complete = jest + .spyOn(stub, "completeActivityTask") + .mockImplementation( + (_request, _metadata, optionsOrCallback: Partial | Callback, callback?: Callback) => { + (typeof optionsOrCallback === "function" ? optionsOrCallback : callback!)( + null, + new pb.CompleteTaskResponse(), + ); + return unaryCall(); + }, + ); + }); + + function dispatch(): void { + worker["_dispatchWorkItem"]( + new pb.WorkItem() + .setCompletiontoken("payload-token") + .setActivityrequest(activityRequest().setInput(new StringValue().setValue("payload:input"))), + stub, + ); + } + + it("awaits abandon and retries transient release errors with the same token and fresh metadata", async () => { + store.download.mockRejectedValueOnce(failure); + const metadata = jest.fn(async () => new grpc.Metadata()); + worker["_metadataGenerator"] = metadata; + let finish!: () => void; + onAbandon = (_request, callback) => { + if (abandon.mock.calls.length === 1) + callback(grpcError(grpc.status.UNAVAILABLE), new pb.AbandonActivityTaskResponse()); + else finish = () => callback(null, new pb.AbandonActivityTaskResponse()); + }; + dispatch(); + await jest.advanceTimersByTimeAsync(200); + expect(abandon).toHaveBeenCalledTimes(2); + expect(worker["_pendingWorkItems"].size).toBe(1); + expect(abandon.mock.calls[0][0].getCompletiontoken()).toBe("payload-token"); + expect(abandon.mock.calls[1][0]).toBe(abandon.mock.calls[0][0]); + expect(abandon.mock.calls[1][1]).not.toBe(abandon.mock.calls[0][1]); + finish(); + await Promise.all(worker["_pendingWorkItems"]); + expect(metadata).toHaveBeenCalledTimes(2); + expect(store.download).toHaveBeenCalledTimes(1); + expect(complete).not.toHaveBeenCalled(); + expect(worker["_pendingWorkItems"].size).toBe(0); + expect(logger.error).toHaveBeenCalledTimes(1); + }); + + it.each([grpc.status.PERMISSION_DENIED, grpc.status.UNAVAILABLE])( + "logs terminal abandon failure %s without an unhandled rejection", + async (status) => { + store.download.mockRejectedValueOnce(failure); + onAbandon = (_request, callback) => callback(grpcError(status), new pb.AbandonActivityTaskResponse()); + dispatch(); + await jest.runAllTimersAsync(); + await Promise.all(worker["_pendingWorkItems"]); + expect(abandon).toHaveBeenCalledTimes(status === grpc.status.UNAVAILABLE ? 10 : 1); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining("payload storage unavailable")); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining("delivery failed")); + expect(complete).not.toHaveBeenCalled(); + expect(worker["_pendingWorkItems"].size).toBe(0); + expect(jest.getTimerCount()).toBe(0); + }, + ); + + it("releases through the captured stub even if the worker replaces its current stub", async () => { + let reject!: (error: Error) => void; + store.download.mockReturnValueOnce(new Promise((_resolve, fail) => (reject = fail))); + const replacement = new stubs.TaskHubSidecarServiceClient("localhost:2", grpc.credentials.createInsecure()); + const replacementAbandon = jest.spyOn(replacement, "abandonTaskActivityWorkItem"); + try { + dispatch(); + await jest.advanceTimersByTimeAsync(0); + worker["_stub"] = replacement; + reject(failure); + await jest.runAllTimersAsync(); + await Promise.all(worker["_pendingWorkItems"]); + expect(abandon).toHaveBeenCalledTimes(1); + expect(replacementAbandon).not.toHaveBeenCalled(); + expect(complete).not.toHaveBeenCalled(); + } finally { + replacement.close(); + } + }); + + it.each(["download", "upload"] as const)( + "does not abandon a late %s failure with a replacement run's signal", + async (phase) => { + let reject!: (error: Error) => void; + store[phase].mockReturnValueOnce(new Promise((_resolve, fail) => (reject = fail))); + dispatch(); + await jest.advanceTimersByTimeAsync(0); + expect(store[phase]).toHaveBeenCalledTimes(1); + controller.abort(); + worker["_abortController"] = new AbortController(); + reject(failure); + await jest.runAllTimersAsync(); + await Promise.all(worker["_pendingWorkItems"]); + expect(abandon).not.toHaveBeenCalled(); + expect(complete).not.toHaveBeenCalled(); + expect(worker["_pendingWorkItems"].size).toBe(0); + expect(worker["_abortController"]!.signal.aborted).toBe(false); + }, + ); + + it.each(["metadata", "retry", "RPC"] as const)( + "shutdown cancels abandon while awaiting %s without sending in a new run", + async (phase) => { + store.download.mockRejectedValueOnce(failure); + let finishMetadata!: (value: grpc.Metadata) => void; + const metadata = jest.fn(async () => new grpc.Metadata()); + if (phase === "metadata") + metadata.mockImplementation(() => new Promise((resolve) => (finishMetadata = resolve))); + worker["_metadataGenerator"] = metadata; + onAbandon = (_request, callback) => { + if (phase === "retry") callback(grpcError(grpc.status.UNAVAILABLE), new pb.AbandonActivityTaskResponse()); + }; + worker["_isRunning"] = true; + dispatch(); + await jest.advanceTimersByTimeAsync(0); + expect(metadata).toHaveBeenCalledTimes(1); + const stopping = worker.stop(); + await jest.advanceTimersByTimeAsync(1100); + await stopping; + worker["_abortController"] = new AbortController(); + if (phase === "metadata") finishMetadata(new grpc.Metadata()); + await jest.runAllTimersAsync(); + await Promise.all(worker["_pendingWorkItems"]); + expect(abandon).toHaveBeenCalledTimes(phase === "metadata" ? 0 : 1); + expect(cancel).toHaveBeenCalledTimes(phase === "RPC" ? 1 : 0); + expect(complete).not.toHaveBeenCalled(); + expect(worker["_pendingWorkItems"].size).toBe(0); + expect(jest.getTimerCount()).toBe(0); + }, + ); + + it.each([undefined, grpc.status.PERMISSION_DENIED])( + "does not double abandon an explicit version rejection (release status=%s)", + async (status) => { + worker = new TaskHubGrpcWorker({ + payloads: { store }, + logger, + versioning: { + version: "2", + matchStrategy: VersionMatchStrategy.Strict, + failureStrategy: VersionFailureStrategy.Reject, + }, + }); + worker["_abortController"] = controller; + const reject = jest + .spyOn(stub, "abandonTaskOrchestratorWorkItem") + .mockImplementation( + ( + _request, + _metadata, + optionsOrCallback: Partial | Callback, + callback?: Callback, + ) => { + (typeof optionsOrCallback === "function" ? optionsOrCallback : callback!)( + status === undefined ? null : grpcError(status), + new pb.AbandonOrchestrationTaskResponse(), + ); + return unaryCall(); + }, + ); + worker["_dispatchWorkItem"]( + new pb.WorkItem() + .setCompletiontoken("version-token") + .setOrchestratorrequest( + new pb.OrchestratorRequest() + .setInstanceid("instance") + .setNeweventsList([ + newOrchestratorStartedEvent(), + newExecutionStartedEvent("flow", "instance", undefined, undefined, undefined, "1"), + ]), + ), + stub, + ); + await jest.runAllTimersAsync(); + await Promise.all(worker["_pendingWorkItems"]); + expect(reject).toHaveBeenCalledTimes(1); + expect(reject.mock.calls[0][0].getCompletiontoken()).toBe("version-token"); + expect(abandon).not.toHaveBeenCalled(); + expect(complete).not.toHaveBeenCalled(); + }, + ); + }); }); From dcb09f4df0784ad9598f45d756373b014c94d95c Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 20 Oct 2026 18:44:53 -0400 Subject: [PATCH 06/11] fix: isolate completion wait RPC deadline retries Hydrate successful wait responses outside scheduler deadline recovery so custom payload-store errors retain their original identity and cancellation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 2 + README.md | 3 + packages/durabletask-js/src/client/client.ts | 16 ++- .../test/client-wait-grpc.spec.ts | 113 +++++++++++++++++- 4 files changed, 128 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b23d918..60dad1e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ ### Fixes +- Limit completion-wait deadline retries to scheduler RPC failures. Payload download errors, + including custom-store gRPC deadlines, propagate unchanged after the successful wait response. - Abandon acquired worker work items when enabled payload hydration or unconverted output storage processing fails, awaiting shutdown-aware bounded release retries without creating fake business failures. Tokenless host processing preserves unconverted storage errors. diff --git a/README.md b/README.md index 9273cd52..42003dc4 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,9 @@ orchestration output failures still produce non-retriable failed completions. To (`processOrchestratorRequest` and `processEntityBatchRequest`) instead reject with the original unconverted storage error so the host can apply its own retry policy. +Completion waits retry scheduler RPC deadline failures, not payload download failures. Storage +errors propagate unchanged, including gRPC deadline errors from custom stores. + Azure-managed builders expose `.useExternalizedPayloads({ store })` and require core `>=0.4.1` for the public payload types. This phase works with a custom/in-memory store and does not import an Azure Blob provider or register cleanup tasks. Complete the six-phase feature stack before release. diff --git a/packages/durabletask-js/src/client/client.ts b/packages/durabletask-js/src/client/client.ts index f71ba9fc..33c01068 100644 --- a/packages/durabletask-js/src/client/client.ts +++ b/packages/durabletask-js/src/client/client.ts @@ -384,6 +384,7 @@ export class TaskHubGrpcClient { * * If an orchestrator instance is already in a terminal state when this method is called, it returns immediately. * Server deadline errors are retried with backoff within the original total timeout. + * Payload storage errors propagate without retrying the successful wait RPC. * * @param {string} instanceId - The unique identifier of the orchestrator instance to wait for. * @param {boolean} fetchPayloads - Indicates whether to fetch the orchestrator instance's @@ -435,25 +436,30 @@ export class TaskHubGrpcClient { private async _waitForCompletion(req: pb.GetInstanceRequest, signal: AbortSignal): Promise { const backoff = new ExponentialBackoff({ initialDelayMs: 100, maxDelayMs: 1000, jitterFactor: 0 }); + const waitSignal = this._payloads ? AbortSignal.any([signal, this._payloadCancellation.signal]) : signal; while (true) { - signal.throwIfAborted(); + waitSignal.throwIfAborted(); // Metadata failures must not enter the remote-deadline retry path. const metadata = this._metadataGenerator ? await this._metadataGenerator() : new grpc.Metadata(); + let response: pb.GetInstanceResponse; try { - return await this._call( + // GetInstanceRequest has no externalizable fields; isolate RPC errors from response hydration. + response = await callWithMetadata( this._stub.waitForInstanceCompletion.bind(this._stub), req, async () => metadata, - signal, + waitSignal, ); } catch (error) { - signal.throwIfAborted(); + waitSignal.throwIfAborted(); if (error instanceof Error && "code" in error && error.code === grpc.status.DEADLINE_EXCEEDED) { - await backoff.wait(signal); + await backoff.wait(waitSignal); continue; } throw error; } + if (this._payloads) await this._payloads.resolve(response, waitSignal); + return response; } } diff --git a/packages/durabletask-js/test/client-wait-grpc.spec.ts b/packages/durabletask-js/test/client-wait-grpc.spec.ts index 8d034199..0cf4d532 100644 --- a/packages/durabletask-js/test/client-wait-grpc.spec.ts +++ b/packages/durabletask-js/test/client-wait-grpc.spec.ts @@ -4,6 +4,7 @@ import * as grpc from "@grpc/grpc-js"; import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; import { TaskHubGrpcClient } from "../src/client/client"; +import { PayloadStore } from "../src/payloads/payload-store"; import { TimeoutError } from "../src/exception/timeout-error"; import { NoOpLogger } from "../src/types/logger.type"; import * as pb from "../src/proto/orchestrator_service_pb"; @@ -11,6 +12,14 @@ import { TaskHubSidecarServiceService } from "../src/proto/orchestrator_service_ type WaitCall = grpc.ServerUnaryCall; +class WaitPayloadStore extends PayloadStore { + upload = jest.fn(async (_value: string, _signal?: AbortSignal) => "payload:output"); + download = jest.fn(async (_token: string, _signal?: AbortSignal) => '"hydrated"'); + isKnownPayloadToken(value: string): boolean { + return value.startsWith("payload:"); + } +} + function completedResponse(): pb.GetInstanceResponse { const state = new pb.OrchestrationState(); state.setName("grpc-wait"); @@ -25,6 +34,7 @@ function completedResponse(): pb.GetInstanceResponse { describe("Client waits over real gRPC", () => { const server = new grpc.Server(); let client: TaskHubGrpcClient; + let hostAddress: string; let handler: grpc.handleUnaryCall; let calls: WaitCall[]; let metadataCalls: number; @@ -46,8 +56,9 @@ describe("Client waits over real gRPC", () => { else resolve(boundPort); }); }); + hostAddress = `127.0.0.1:${port}`; client = new TaskHubGrpcClient({ - hostAddress: `127.0.0.1:${port}`, + hostAddress, options: { interceptors: [ (options: grpc.InterceptorOptions, nextCall: grpc.NextCall) => @@ -182,4 +193,104 @@ describe("Client waits over real gRPC", () => { }); expect(calls).toHaveLength(2); }); + + describe("externalized completion waits", () => { + let payloadClient: TaskHubGrpcClient; + let store: WaitPayloadStore; + let stopped: boolean; + + beforeEach(() => { + store = new WaitPayloadStore(); + stopped = false; + payloadClient = new TaskHubGrpcClient({ + hostAddress, + payloads: { store }, + metadataGenerator: client["_metadataGenerator"], + logger: new NoOpLogger(), + }); + handler = (_call, callback) => { + const response = completedResponse(); + response.getOrchestrationstate()!.setOutput(new StringValue().setValue("payload:output")); + callback(null, response); + }; + }); + + afterEach(async () => { + if (!stopped) await payloadClient.stop(); + }); + + it.each(["grpc deadline", "HTTP 503"] as const)( + "propagates the original storage %s after one successful wait RPC and download", + async (kind) => { + const failure = + kind === "grpc deadline" + ? Object.assign(new Error("storage deadline"), { + code: grpc.status.DEADLINE_EXCEEDED, + details: "storage deadline", + metadata: new grpc.Metadata(), + }) + : Object.assign(new Error("storage unavailable"), { statusCode: 503 }); + store.download.mockRejectedValue(failure); + await expect(payloadClient.waitForOrchestrationCompletion("grpc-instance", true, 0.45)).rejects.toBe(failure); + expect(calls).toHaveLength(1); + expect(store.download).toHaveBeenCalledTimes(1); + expect(metadataCalls).toBe(1); + }, + ); + + it("retries genuine wait RPC deadlines before hydrating the successful response once", async () => { + const success = handler; + handler = (call, callback) => { + if (calls.length < 3) callback({ code: grpc.status.DEADLINE_EXCEEDED, details: "remote deadline" }); + else success(call, callback); + }; + await expect(payloadClient.waitForOrchestrationCompletion("grpc-instance", true, 5)).resolves.toMatchObject({ + serializedOutput: '"hydrated"', + }); + expect(calls).toHaveLength(3); + expect(calls.map((call) => call.metadata.get("attempt"))).toEqual([["1"], ["2"], ["3"]]); + expect(store.download).toHaveBeenCalledTimes(1); + }); + + it("does not retry a metadata deadline or start the wait RPC", async () => { + const failure = Object.assign(new Error("metadata deadline"), { code: grpc.status.DEADLINE_EXCEEDED }); + payloadClient["_metadataGenerator"] = async () => { + throw failure; + }; + await expect(payloadClient.waitForOrchestrationCompletion("grpc-instance", true, 1)).rejects.toBe(failure); + expect(calls).toHaveLength(0); + expect(store.download).not.toHaveBeenCalled(); + }); + + it.each(["abort", "timeout", "stop"] as const)( + "preserves %s cancellation while the successful wait response is hydrating", + async (cause) => { + let started!: () => void; + const downloading = new Promise((resolve) => (started = resolve)); + store.download.mockImplementation( + (_token, signal) => + new Promise((_resolve, reject) => { + signal!.addEventListener("abort", () => reject(signal!.reason), { once: true }); + started(); + }), + ); + const controller = new AbortController(); + const reason = new Error("cancel hydration"); + const pending = payloadClient.waitForOrchestrationCompletion("grpc-instance", true, 0.5, controller.signal); + const rejected = pending.catch((error: unknown) => error); + await downloading; + if (cause === "abort") controller.abort(reason); + if (cause === "stop") { + stopped = true; + await payloadClient.stop(); + } + const error = await rejected; + if (cause === "timeout") expect(error).toBeInstanceOf(TimeoutError); + else expect(error).toBe(cause === "abort" ? reason : payloadClient["_payloadCancellation"].signal.reason); + expect(calls).toHaveLength(1); + expect(store.download).toHaveBeenCalledTimes(1); + expect(store.download.mock.calls[0][1]?.aborted).toBe(true); + }, + ); + }); }); From aeb598f68d0d4a47c89a762b516a5131af60d4c8 Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 20 Oct 2026 19:33:55 -0400 Subject: [PATCH 07/11] fix: skip recognized payload tokens during externalization Preserve one-pass hydration across repeated and partial-success retries, excluding known references before size preflight while retaining raw payload validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 2 + .../src/payloads/payload-processor.ts | 2 +- packages/durabletask-js/test/payloads.spec.ts | 122 ++++++++++++++++++ 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1040a155..bf4c6c54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,8 @@ and token mutations for deterministically oversized messages. Precancellation, worker failure conversion, and bounded I/O error handling are preserved; transient I/O or submission failures can still leave stored payloads. +- Skip store-recognized payload tokens before externalization size checks and uploads, so retries + of already or partially externalized messages hydrate back to the original data in one pass. - Prevent shared or duplicate pending tasks from stranding `whenAll` orchestrations after all activities finish. Notify every dependent composite, preserve duplicate result positions, and detach completed `whenAny` groups from their remaining children. diff --git a/packages/durabletask-js/src/payloads/payload-processor.ts b/packages/durabletask-js/src/payloads/payload-processor.ts index 143ad5be..01ca5b3a 100644 --- a/packages/durabletask-js/src/payloads/payload-processor.ts +++ b/packages/durabletask-js/src/payloads/payload-processor.ts @@ -77,7 +77,7 @@ export class PayloadProcessor { const candidates: { field: StringValue; value: string; size: number }[] = []; for (const field of fields) { const value = field?.getValue(); - if (!field || !value) continue; + if (!field || !value || this._options.store.isKnownPayloadToken(value)) continue; const size = Buffer.byteLength(value, "utf8"); if (size < this._threshold) continue; candidates.push({ field, value, size }); diff --git a/packages/durabletask-js/test/payloads.spec.ts b/packages/durabletask-js/test/payloads.spec.ts index 5c466e96..460100e4 100644 --- a/packages/durabletask-js/test/payloads.spec.ts +++ b/packages/durabletask-js/test/payloads.spec.ts @@ -104,6 +104,128 @@ describe("externalized payloads", () => { expect(store.uploads).toBe(0); }); + describe("recognized payload tokens", () => { + it("externalizes a message repeatedly without uploading tokens and hydrates it once", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text("original state")) + .setOperationsList([new pb.OperationRequest().setInput(text("original input"))]); + const original = request.toObject(); + const upload = jest.spyOn(store, "upload"); + + await processor.externalize(request); + const externalized = request.toObject(); + await processor.externalize(request); + await processor.externalize(request); + await processor.resolve(request); + + expect(request.toObject()).toEqual(original); + expect(upload.mock.calls.map(([value]) => value)).toEqual(["original state", "original input"]); + expect(store.uploads).toBe(2); + expect(externalized.entitystate!.value).toBe("test:1"); + expect(externalized.operationsList[0].input!.value).toBe("test:2"); + }); + + it("retries a partially externalized message without wrapping the successful token", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text("original state")) + .setOperationsList([new pb.OperationRequest().setInput(text("original input"))]); + const original = request.toObject(); + const failure = Object.assign(new Error("retry input"), { statusCode: 503 }); + const uploadValue = store.upload.bind(store); + let failInput = true; + const upload = jest.spyOn(store, "upload").mockImplementation(async (value) => { + if (value === "original input" && failInput) { + failInput = false; + throw failure; + } + return uploadValue(value); + }); + + await expect(processor.externalize(request)).rejects.toBe(failure); + expect(request.getEntitystate()!.getValue()).toBe("test:1"); + expect(request.getOperationsList()[0].getInput()!.getValue()).toBe("original input"); + await processor.externalize(request); + await processor.resolve(request); + + expect(request.toObject()).toEqual(original); + expect(upload.mock.calls.map(([value]) => value)).toEqual(["original state", "original input", "original input"]); + expect([...store.values.values()]).toEqual(["original state", "original input"]); + }); + + it("uses the configured store's recognition rather than a token-prefix assumption", async () => { + const token = "opaque-owned-reference"; + store.values.set(token, "original state"); + store.isKnownPayloadToken = (value) => store.values.has(value); + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text(token)) + .setOperationsList([new pb.OperationRequest().setInput(text("test:literal input"))]); + const upload = jest.spyOn(store, "upload"); + + await processor.externalize(request); + await processor.resolve(request); + + expect(request.getEntitystate()!.getValue()).toBe("original state"); + expect(request.getOperationsList()[0].getInput()!.getValue()).toBe("test:literal input"); + expect(upload.mock.calls.map(([value]) => value)).toEqual(["test:literal input"]); + }); + + it("excludes known tokens from maximum validation while externalizing eligible raw fields", async () => { + const token = "test:reference-longer-than-the-maximum"; + store.values.set(token, "original state"); + processor = new PayloadProcessor({ store, thresholdBytes: 1, maxPayloadBytes: 1 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text(token)) + .setOperationsList([new pb.OperationRequest().setInput(text("x"))]); + + await expect(processor.externalize(request)).resolves.toBeUndefined(); + expect(request.getEntitystate()!.getValue()).toBe(token); + expect([...store.values.values()]).toEqual(["original state", "x"]); + await processor.resolve(request); + expect(request.getEntitystate()!.getValue()).toBe("original state"); + expect(request.getOperationsList()[0].getInput()!.getValue()).toBe("x"); + }); + + it("still preflights every raw field before uploading when a message already contains tokens", async () => { + const token = "test:reference-longer-than-the-maximum"; + store.values.set(token, "original state"); + processor = new PayloadProcessor({ store, thresholdBytes: 1, maxPayloadBytes: 1 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text(token)) + .setOperationsList([ + new pb.OperationRequest().setInput(text("x")), + new pb.OperationRequest().setInput(text("xx")), + ]); + const original = request.toObject(); + + await expect(processor.externalize(request)).rejects.toThrow( + "Payload size 2 bytes exceeds the configured maximum of 1 bytes.", + ); + expect(store.uploads).toBe(0); + expect([...store.values.values()]).toEqual(["original state"]); + expect(request.toObject()).toEqual(original); + }); + + it("treats an all-token message as a canceled no-op", async () => { + store.values.set("test:state", "original state"); + store.values.set("test:input", "original input"); + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const request = new pb.EntityBatchRequest() + .setEntitystate(text("test:state")) + .setOperationsList([new pb.OperationRequest().setInput(text("test:input"))]); + const original = request.toObject(); + const abort = new AbortController(); + abort.abort(); + + await expect(processor.externalize(request, abort.signal)).resolves.toBeUndefined(); + + expect(store.uploads).toBe(0); + expect(request.toObject()).toEqual(original); + }); + }); + it("maps large entity batches without the JavaScript variadic argument limit", async () => { const request = new pb.EntityBatchRequest().setOperationsList( Array.from({ length: 150000 }, () => new pb.OperationRequest().setInput(text("x"))), From bee13134d6ea2e317938b3e4eda450cf6b463bc1 Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 20 Oct 2026 19:38:19 -0400 Subject: [PATCH 08/11] fix: preserve cancellation during payload externalization Rethrow caught errors when the signal is aborted before permanent-failure conversion, preserving original cancellation and independent message-order I/O errors for both worker responses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 3 + .../src/payloads/payload-processor.ts | 1 + packages/durabletask-js/test/payloads.spec.ts | 169 +++++++++++++++++- 3 files changed, 172 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf4c6c54..4476945f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,9 @@ can still leave stored payloads. - Skip store-recognized payload tokens before externalization size checks and uploads, so retries of already or partially externalized messages hydrate back to the original data in one pass. +- Preserve canceled payload processing as an error instead of a permanent worker failure, even + when the abort reason resembles a storage failure. Rethrow the drained runner's original error + without replacing earlier I/O failures with a later cancellation reason. - Prevent shared or duplicate pending tasks from stranding `whenAll` orchestrations after all activities finish. Notify every dependent composite, preserve duplicate result positions, and detach completed `whenAny` groups from their remaining children. diff --git a/packages/durabletask-js/src/payloads/payload-processor.ts b/packages/durabletask-js/src/payloads/payload-processor.ts index 01ca5b3a..5b0e1250 100644 --- a/packages/durabletask-js/src/payloads/payload-processor.ts +++ b/packages/durabletask-js/src/payloads/payload-processor.ts @@ -99,6 +99,7 @@ export class PayloadProcessor { ); } catch (error) { if ( + signal?.aborted || !isPermanentStorageFailure(error) || !(message instanceof pb.ActivityResponse || message instanceof pb.OrchestratorResponse) ) diff --git a/packages/durabletask-js/test/payloads.spec.ts b/packages/durabletask-js/test/payloads.spec.ts index 460100e4..4a9f5b3a 100644 --- a/packages/durabletask-js/test/payloads.spec.ts +++ b/packages/durabletask-js/test/payloads.spec.ts @@ -18,7 +18,7 @@ import { Registry } from "../src/worker/registry"; class MemoryPayloadStore extends PayloadStore { readonly values = new Map(); uploads = 0; - async upload(value: string): Promise { + async upload(value: string, _signal?: AbortSignal): Promise { const token = `test:${++this.uploads}`; this.values.set(token, value); return token; @@ -713,6 +713,173 @@ describe("externalized payloads", () => { }); }); + describe("worker response cancellation", () => { + describe.each([ + { + name: "activity", + makeResponse: () => + new pb.ActivityResponse() + .setInstanceid("instance") + .setTaskid(17) + .setCompletiontoken("completion") + .setResult(text("result")), + }, + { + name: "orchestrator", + makeResponse: () => + new pb.OrchestratorResponse() + .setInstanceid("instance") + .setCompletiontoken("completion") + .setCustomstatus(text("status")) + .setIspartial(true) + .setChunkindex(new Int32Value().setValue(2)) + .setActionsList([ + new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(text("input"))), + ]), + }, + ])("$name", ({ makeResponse }) => { + describe.each([ + { name: "storage exception", makeReason: () => new PayloadStorageException("canceled") }, + { name: "HTTP 403", makeReason: () => Object.assign(new Error("canceled"), { statusCode: 403 }) }, + ])("$name", ({ makeReason }) => { + it.each(["pre-aborted", "in-flight"] as const)( + "rethrows the original %s reason without converting the response", + async (stage) => { + processor = new PayloadProcessor({ + store, + thresholdBytes: 1, + maxPayloadBytes: stage === "pre-aborted" ? 1 : 1024, + }); + const response = makeResponse(); + const original = response.toObject(); + const abort = new AbortController(); + const reason = makeReason(); + const started = gate(); + let active = 0; + const upload = jest.spyOn(store, "upload").mockImplementation(async (_value, signal) => { + active++; + started.release(); + try { + return await new Promise((_resolve, reject) => { + signal!.addEventListener("abort", () => reject(signal!.reason), { once: true }); + }); + } finally { + active--; + } + }); + if (stage === "pre-aborted") abort.abort(reason); + + const pending = expect(processor.externalize(response, abort.signal)).rejects.toBe(reason); + if (stage === "in-flight") { + await started.promise; + abort.abort(reason); + } + await pending; + + expect(active).toBe(0); + if (stage === "pre-aborted") expect(upload).not.toHaveBeenCalled(); + else expect(upload).toHaveBeenCalled(); + expect(store.values.size).toBe(0); + expect(response.toObject()).toEqual(original); + await nextTurn(); + expect(response.toObject()).toEqual(original); + }, + ); + + it("still converts a genuine permanent upload failure when the signal is not aborted", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const response = makeResponse(); + const error = makeReason(); + jest.spyOn(store, "upload").mockRejectedValue(error); + const abort = new AbortController(); + + await expect(processor.externalize(response, abort.signal)).resolves.toBeUndefined(); + + const details = + response instanceof pb.ActivityResponse + ? response.getFailuredetails() + : response.getActionsList()[0].getCompleteorchestration()!.getFailuredetails(); + expect(details!.getIsnonretriable()).toBe(true); + expect(details!.getErrortype()).toBe(error.name); + expect(details!.getErrormessage()).toBe(error.message); + expect(abort.signal.aborted).toBe(false); + }); + + it("leaves canceled no-op responses unchanged", async () => { + processor = new PayloadProcessor({ store, thresholdBytes: 1024 }); + const response = makeResponse(); + const original = response.toObject(); + const abort = new AbortController(); + abort.abort(makeReason()); + + await expect(processor.externalize(response, abort.signal)).resolves.toBeUndefined(); + + expect(response.toObject()).toEqual(original); + expect(store.uploads).toBe(0); + }); + }); + }); + + it.each([403, 503])( + "rethrows the earlier I/O error %s rather than a later abort reason after drainage", + async (statusCode) => { + processor = new PayloadProcessor({ store, thresholdBytes: 1 }); + const response = new pb.OrchestratorResponse() + .setCustomstatus(text("earlier")) + .setActionsList([ + new pb.OrchestratorAction().setScheduletask(new pb.ScheduleTaskAction().setInput(text("later"))), + ]); + const original = response.toObject(); + const abort = new AbortController(); + const firstError = Object.assign(new Error("independent earlier I/O failure"), { statusCode }); + const laterReason = new PayloadStorageException("later cancellation"); + const hold = gate(); + const laterStarted = gate(); + let active = 0; + let settled = false; + const started: string[] = []; + store.upload = async (value) => { + started.push(value); + active++; + try { + if (value === "earlier") { + await hold.promise; + throw firstError; + } + abort.abort(laterReason); + laterStarted.release(); + throw abort.signal.reason; + } finally { + active--; + } + }; + const pending = processor.externalize(response, abort.signal).then( + () => { + settled = true; + }, + (error: unknown) => { + settled = true; + return error; + }, + ); + await laterStarted.promise; + await nextTurn(); + const beforeDrain = { settled, active }; + hold.release(); + const error = await pending; + + expect(beforeDrain).toEqual({ settled: false, active: 1 }); + expect(active).toBe(0); + expect(started).toEqual(["earlier", "later"]); + expect(error).toBe(firstError); + expect(error).not.toBe(laterReason); + expect(response.toObject()).toEqual(original); + await nextTurn(); + expect(response.toObject()).toEqual(original); + }, + ); + }); + describe("in-flight payload I/O", () => { it.each([false, true])( "classifies the lowest-message-order failure after drainage (permanent: %s)", From 137bb547e8781fd84c2738c3e13ae0f65cd66174 Mon Sep 17 00:00:00 2001 From: wangbill Date: Thu, 22 Oct 2026 13:04:47 -0400 Subject: [PATCH 09/11] fix: process rewind history input payloads Include ExecutionRewoundEvent input in the shared history mapping so hydration, rewriting, carryover, and whole-message UTF-8 size preflight cover it without changing rewind metadata. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 2 + .../src/payloads/payload-processor.ts | 1 + packages/durabletask-js/test/payloads.spec.ts | 114 ++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4476945f..356f4162 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ - Preserve canceled payload processing as an error instead of a permanent worker failure, even when the abort reason resembles a storage failure. Rethrow the drained runner's original error without replacing earlier I/O failures with a later cancellation reason. +- Include rewind-history input in payload hydration, externalization, and UTF-8 size preflight, + preserving the rewind reason and metadata. - Prevent shared or duplicate pending tasks from stranding `whenAll` orchestrations after all activities finish. Notify every dependent composite, preserve duplicate result positions, and detach completed `whenAny` groups from their remaining children. diff --git a/packages/durabletask-js/src/payloads/payload-processor.ts b/packages/durabletask-js/src/payloads/payload-processor.ts index 5b0e1250..4aecec5b 100644 --- a/packages/durabletask-js/src/payloads/payload-processor.ts +++ b/packages/durabletask-js/src/payloads/payload-processor.ts @@ -190,6 +190,7 @@ export class PayloadProcessor { event.getExecutionsuspended()?.getInput(), event.getExecutionresumed()?.getInput(), event.getExecutionrewound()?.getReason(), + event.getExecutionrewound()?.getInput(), event.getEntityoperationsignaled()?.getInput(), event.getEntityoperationcalled()?.getInput(), event.getEntityoperationcompleted()?.getOutput(), diff --git a/packages/durabletask-js/test/payloads.spec.ts b/packages/durabletask-js/test/payloads.spec.ts index 4a9f5b3a..871364b2 100644 --- a/packages/durabletask-js/test/payloads.spec.ts +++ b/packages/durabletask-js/test/payloads.spec.ts @@ -383,6 +383,7 @@ describe("externalized payloads", () => { new pb.HistoryEvent().setExecutionsuspended(new pb.ExecutionSuspendedEvent().setInput(value)), (value: StringValue) => new pb.HistoryEvent().setExecutionresumed(new pb.ExecutionResumedEvent().setInput(value)), (value: StringValue) => new pb.HistoryEvent().setExecutionrewound(new pb.ExecutionRewoundEvent().setReason(value)), + (value: StringValue) => new pb.HistoryEvent().setExecutionrewound(new pb.ExecutionRewoundEvent().setInput(value)), (value: StringValue) => new pb.HistoryEvent().setEntityoperationsignaled(new pb.EntityOperationSignaledEvent().setInput(value)), (value: StringValue) => @@ -403,6 +404,119 @@ describe("externalized payloads", () => { expect(event.getEventtypeCase()).toBe(originalCase); }); + describe("rewind input payloads", () => { + const makeEvent = (input: string) => + new pb.HistoryEvent() + .setEventid(42) + .setExecutionrewound( + new pb.ExecutionRewoundEvent() + .setReason(text("rewind reason")) + .setParentexecutionid(text("previous-execution")) + .setInstanceid(text("rewound-instance")) + .setName(text("RewoundFlow")) + .setVersion(text("v2")) + .setInput(text(input)), + ); + + it.each([ + { name: "history chunk", message: (event: pb.HistoryEvent) => new pb.HistoryChunk().setEventsList([event]) }, + { + name: "past history", + message: (event: pb.HistoryEvent) => new pb.OrchestratorRequest().setPasteventsList([event]), + }, + { + name: "new history", + message: (event: pb.HistoryEvent) => new pb.OrchestratorRequest().setNeweventsList([event]), + }, + ])("hydrates rewind input once in $name without changing metadata or reason", async ({ message }) => { + const value = '{"original":"input"}'; + const token = await store.upload(value); + const event = makeEvent(token); + const download = jest.spyOn(store, "download"); + + await processor.resolve(message(event)); + + expect(event.toObject()).toEqual(makeEvent(value).toObject()); + expect(download.mock.calls).toEqual([[token, undefined]]); + }); + + describe.each(["rewritten", "carryover"] as const)("%s history", (surface) => { + const responseFor = (event: pb.HistoryEvent) => + new pb.OrchestratorResponse() + .setInstanceid("rewound-instance") + .setCompletiontoken("completion") + .setActionsList([ + surface === "rewritten" + ? new pb.OrchestratorAction() + .setId(7) + .setRewindorchestration(new pb.RewindOrchestrationAction().setNewhistoryList([event])) + : new pb.OrchestratorAction() + .setId(7) + .setCompleteorchestration( + new pb.CompleteOrchestrationAction() + .setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_CONTINUED_AS_NEW) + .setCarryovereventsList([event]), + ), + ]); + + it.each([ + ["below threshold", "\u00e9".repeat(131070) + "a", 262143, false], + ["at threshold", "\u00e9".repeat(131071), 262144, true], + ["above threshold", "\u00e9".repeat(131071) + "a", 262145, true], + ])( + "externalizes rewind input %s and round-trips metadata after one hydration", + async (_name, value, bytes, eligible) => { + const input = JSON.stringify(value); + expect(Buffer.byteLength(input, "utf8")).toBe(bytes); + const response = responseFor(makeEvent(input)); + const original = response.toObject(); + + await processor.externalize(response); + await processor.externalize(response); + + expect(store.uploads).toBe(eligible ? 1 : 0); + expect([...store.values.values()]).toEqual(eligible ? [input] : []); + const wire = pb.OrchestratorResponse.deserializeBinary(response.serializeBinary()); + const action = wire.getActionsList()[0]; + const events = + action.getRewindorchestration()?.getNewhistoryList() ?? + action.getCompleteorchestration()!.getCarryovereventsList(); + expect(store.isKnownPayloadToken(events[0].getExecutionrewound()!.getInput()!.getValue())).toBe(eligible); + const download = jest.spyOn(store, "download"); + + await processor.resolve(new pb.HistoryChunk().setEventsList(events)); + + expect(download).toHaveBeenCalledTimes(eligible ? 1 : 0); + expect(wire.toObject()).toEqual(original); + }, + ); + + it("preflights oversized rewind input before uploading a valid status or mutating payload fields", async () => { + processor = new PayloadProcessor({ store, maxPayloadBytes: MIB }); + const event = makeEvent("\u00e9".repeat(MIB / 2) + "a"); + const originalEvent = event.toObject(); + const status = text("s".repeat(262144)); + const response = responseFor(event).setCustomstatus(status); + + await processor.externalize(response); + + expect(store.uploads).toBe(0); + expect(store.values.size).toBe(0); + expect(status.getValue()).toBe("s".repeat(262144)); + expect(event.toObject()).toEqual(originalEvent); + expect(response.hasCustomstatus()).toBe(false); + expect(response.getActionsList()).toHaveLength(1); + const completion = response.getActionsList()[0].getCompleteorchestration()!; + expect(completion.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(completion.getFailuredetails()!.getErrortype()).toBe("PayloadStorageException"); + expect(completion.getFailuredetails()!.getIsnonretriable()).toBe(true); + expect(completion.getFailuredetails()!.getErrormessage()).toBe( + `Payload size ${MIB + 1} bytes exceeds the configured maximum of ${MIB} bytes.`, + ); + }); + }); + }); + it("converts permanent activity upload failures without sending the result", async () => { store.upload = async () => { throw Object.assign(new Error("denied"), { statusCode: 403 }); From bf9a7ef53dc5a997dde9bc5cd86a70f3c70ca8d4 Mon Sep 17 00:00:00 2001 From: wangbill Date: Thu, 22 Oct 2026 13:12:22 -0400 Subject: [PATCH 10/11] fix(client): isolate scheduling RPC error translation Keep payload externalization and metadata generation outside the startInstance error translation catch. Preserve original non-scheduler errors, real scheduler causes, scheduling options and stop cancellation, with real loopback regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 2 + packages/durabletask-js/src/client/client.ts | 37 ++-- .../test/payload-transport.spec.ts | 168 +++++++++++++++++- 3 files changed, 188 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3716a126..4a334eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ ### Fixes +- Restrict scheduling error translation to `startInstance` RPC failures. Payload upload and + metadata errors retain their original identity instead of becoming duplicate-ID or argument errors. - Limit completion-wait deadline retries to scheduler RPC failures. Payload download errors, including custom-store gRPC deadlines, propagate unchanged after the successful wait response. - Abandon acquired worker work items when enabled payload hydration or unconverted output storage diff --git a/packages/durabletask-js/src/client/client.ts b/packages/durabletask-js/src/client/client.ts index 33c01068..ec9109f2 100644 --- a/packages/durabletask-js/src/client/client.ts +++ b/packages/durabletask-js/src/client/client.ts @@ -281,27 +281,32 @@ export class TaskHubGrpcClient { ClientLogs.schedulingOrchestration(this._logger, req.getInstanceid(), name, inputSizeInBytes); try { - const res = await this._call( - this._stub.startInstance.bind(this._stub), - req, - this._metadataGenerator, - ); + const signal = this._payloads ? this._payloadCancellation.signal : undefined; + if (this._payloads) await this._payloads.externalize(req, signal); + const metadata = this._metadataGenerator ? await this._metadataGenerator() : new grpc.Metadata(); + let res: pb.CreateInstanceResponse; + try { + res = await callWithMetadata(this._stub.startInstance.bind(this._stub), req, async () => metadata, signal); + } catch (e: unknown) { + if (e instanceof Error && "code" in e) { + const grpcError = e as grpc.ServiceError; + const message = grpcError.details || grpcError.message; + if (grpcError.code === grpc.status.ALREADY_EXISTS) { + throw new OrchestrationAlreadyExistsError(message, { cause: e }); + } + if (grpcError.code === grpc.status.INVALID_ARGUMENT) { + throw new TypeError(message, { cause: e }); + } + // The JS SDK has no named cancellation error contract, so preserve CANCELLED as a ServiceError. + } + throw e; + } + if (this._payloads) await this._payloads.resolve(res, signal); setSpanOk(span); return res.getInstanceid(); } catch (e: unknown) { setSpanError(span, e); - if (e instanceof Error && "code" in e) { - const grpcError = e as grpc.ServiceError; - const message = grpcError.details || grpcError.message; - if (grpcError.code === grpc.status.ALREADY_EXISTS) { - throw new OrchestrationAlreadyExistsError(message, { cause: e }); - } - if (grpcError.code === grpc.status.INVALID_ARGUMENT) { - throw new TypeError(message, { cause: e }); - } - // The JS SDK has no named cancellation error contract, so preserve CANCELLED as a ServiceError. - } throw e; } finally { endSpan(span); diff --git a/packages/durabletask-js/test/payload-transport.spec.ts b/packages/durabletask-js/test/payload-transport.spec.ts index 5e6f2be4..6dea7e05 100644 --- a/packages/durabletask-js/test/payload-transport.spec.ts +++ b/packages/durabletask-js/test/payload-transport.spec.ts @@ -4,7 +4,15 @@ import * as grpc from "@grpc/grpc-js"; import { Empty } from "google-protobuf/google/protobuf/empty_pb"; import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; -import { TaskHubGrpcClient, TaskHubGrpcWorker, PayloadStore, NoOpLogger, TaskEntity } from "../src"; +import { + TaskHubGrpcClient, + TaskHubGrpcWorker, + PayloadStore, + NoOpLogger, + TaskEntity, + OrchestrationAlreadyExistsError, + OrchestrationStatus, +} from "../src"; import * as pb from "../src/proto/orchestrator_service_pb"; import * as stubs from "../src/proto/orchestrator_service_grpc_pb"; import * as pbh from "../src/utils/pb-helper.util"; @@ -98,6 +106,9 @@ describe("transparent payload transport over gRPC", () => { let onCompleteActivity: stubs.ITaskHubSidecarServiceServer["completeActivityTask"]; let errors: jest.Mock; let historyEvents: pb.HistoryEvent[]; + let startCalls: grpc.ServerUnaryCall[]; + let onStartInstance: stubs.ITaskHubSidecarServiceServer["startInstance"]; + let clientStopped: boolean; beforeEach(async () => { store = new MemoryStore(); @@ -111,6 +122,10 @@ describe("transparent payload transport over gRPC", () => { entityResponses = []; abandonments = []; errors = jest.fn(); + clientStopped = false; + startCalls = []; + onStartInstance = (call, callback) => + callback(null, new pb.CreateInstanceResponse().setInstanceid(call.request.getInstanceid())); onCompleteActivity = (_call, callback) => callback(null, new pb.CompleteTaskResponse()); historyEvents = [pbh.newExecutionStartedEvent("flow", "instance", input)]; server = new grpc.Server(); @@ -124,8 +139,9 @@ describe("transparent payload transport over gRPC", () => { const service = { hello: (_call, callback) => callback(null, new Empty()), startInstance: (call, callback) => { + startCalls.push(call); requestedInput = call.request.getInput()?.getValue(); - callback(null, new pb.CreateInstanceResponse().setInstanceid(call.request.getInstanceid())); + onStartInstance(call, callback); }, getInstance: (_call, callback) => callback(null, new pb.GetInstanceResponse().setExists(true).setOrchestrationstate(state())), @@ -200,7 +216,7 @@ describe("transparent payload transport over gRPC", () => { afterEach(async () => { if (worker["_isRunning"]) await worker.stop(); - await client.stop(); + if (!clientStopped) await client.stop(); server.forceShutdown(); jest.restoreAllMocks(); }); @@ -241,6 +257,152 @@ describe("transparent payload transport over gRPC", () => { expect(history[0]).toMatchObject({ input: '{"message":"hydrated"}' }); }); + describe("scheduling error domains", () => { + it.each([grpc.status.ALREADY_EXISTS, grpc.status.INVALID_ARGUMENT])( + "preserves upload status %s without scheduling", + async (code) => { + const failure = Object.assign(new Error("storage rejected upload"), { + code, + details: "storage rejected upload", + metadata: new grpc.Metadata(), + }); + const upload = jest.spyOn(store, "upload").mockRejectedValue(failure); + const metadata = jest.fn(async () => new grpc.Metadata()); + client["_metadataGenerator"] = metadata; + + await expect(client.scheduleNewOrchestration("workflow", { message: "request" })).rejects.toBe(failure); + expect(upload).toHaveBeenCalledTimes(1); + expect(metadata).not.toHaveBeenCalled(); + expect(startCalls).toHaveLength(0); + }, + ); + + describe.each([true, false])("with payloads enabled=%s", (payloadsEnabled) => { + beforeEach(async () => { + await client.stop(); + client = new TaskHubGrpcClient({ + hostAddress, + payloads: payloadsEnabled ? { store, thresholdBytes: 1 } : undefined, + defaultVersion: "default-version", + logger: new NoOpLogger(), + }); + }); + + it.each([grpc.status.ALREADY_EXISTS, grpc.status.INVALID_ARGUMENT])( + "preserves metadata status %s without scheduling", + async (code) => { + const failure = Object.assign(new Error("metadata rejected request"), { code }); + const upload = jest.spyOn(store, "upload"); + const metadata = jest.fn(async () => { + throw failure; + }); + client["_metadataGenerator"] = metadata; + + await expect(client.scheduleNewOrchestration("workflow", { message: "request" })).rejects.toBe(failure); + expect(metadata).toHaveBeenCalledTimes(1); + expect(upload).toHaveBeenCalledTimes(payloadsEnabled ? 1 : 0); + expect(startCalls).toHaveLength(0); + }, + ); + + it.each([ + [grpc.status.ALREADY_EXISTS, OrchestrationAlreadyExistsError], + [grpc.status.INVALID_ARGUMENT, TypeError], + ])("translates actual startInstance status %s with its cause and details", async (code, expectedError) => { + onStartInstance = (_call, callback) => callback({ code, details: "scheduler rejected start" }); + + const scheduled = client.scheduleNewOrchestration("workflow", { message: "request" }); + await expect(scheduled).rejects.toBeInstanceOf(expectedError); + await expect(scheduled).rejects.toMatchObject({ + message: "scheduler rejected start", + cause: expect.objectContaining({ code, details: "scheduler rejected start" }), + }); + expect(startCalls).toHaveLength(1); + }); + + it.each([undefined, "explicit-version"])( + "preserves successful scheduling options (version=%s)", + async (version) => { + const metadata = new grpc.Metadata(); + metadata.set("taskhub", "scheduling-error-domains"); + const generateMetadata = jest.fn(async () => metadata); + client["_metadataGenerator"] = generateMetadata; + onStartInstance = (_call, callback) => + callback(null, new pb.CreateInstanceResponse().setInstanceid("scheduled-instance")); + const upload = jest.spyOn(store, "upload"); + const startAt = new Date("2026-10-22T15:00:00.000Z"); + const input = { message: "request \u4f60\u597d" }; + + await expect( + client.scheduleNewOrchestration("workflow", input, { + instanceId: "requested-instance", + startAt, + tags: { owner: "scheduling-error-domains" }, + version, + dedupeStatuses: [OrchestrationStatus.COMPLETED], + }), + ).resolves.toBe("scheduled-instance"); + + expect(startCalls).toHaveLength(1); + const { request, metadata: receivedMetadata } = startCalls[0]; + expect(request.getName()).toBe("workflow"); + expect(request.getInstanceid()).toBe("requested-instance"); + expect(request.getScheduledstarttimestamp()?.toDate()).toEqual(startAt); + expect(request.getVersion()?.getValue()).toBe(version ?? "default-version"); + expect(request.getTagsMap().get("owner")).toBe("scheduling-error-domains"); + expect(request.getOrchestrationidreusepolicy()?.getReplaceablestatusList()).toHaveLength(6); + expect(request.getOrchestrationidreusepolicy()?.getReplaceablestatusList()).not.toContain( + pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED, + ); + expect(receivedMetadata.get("taskhub")).toEqual(["scheduling-error-domains"]); + expect(generateMetadata).toHaveBeenCalledTimes(1); + expect(upload).toHaveBeenCalledTimes(payloadsEnabled ? 1 : 0); + const wireInput = request.getInput()!.getValue(); + expect(payloadsEnabled ? await store.download(wireInput) : wireInput).toBe(JSON.stringify(input)); + }, + ); + }); + + it("preserves stop cancellation during upload without starting the scheduling RPC", async () => { + let uploadSignal: AbortSignal | undefined; + const upload = jest.spyOn(store, "upload").mockImplementation( + (_value: string, signal?: AbortSignal) => + new Promise((_resolve, reject) => { + uploadSignal = signal; + signal!.addEventListener("abort", () => reject(signal!.reason), { once: true }); + }), + ); + const pending = client.scheduleNewOrchestration("workflow", { message: "request" }); + const rejected = pending.catch((error: unknown) => error); + await waitFor(() => !!uploadSignal); + clientStopped = true; + await client.stop(); + + expect(await rejected).toBe(uploadSignal!.reason); + expect(uploadSignal!.aborted).toBe(true); + expect(upload).toHaveBeenCalledTimes(1); + expect(startCalls).toHaveLength(0); + }); + + it("preserves stop cancellation of the pending scheduling RPC", async () => { + let canceled = false; + onStartInstance = (call) => { + call.on("cancelled", () => { + canceled = true; + }); + }; + const pending = client.scheduleNewOrchestration("workflow", { message: "request" }); + const rejected = pending.catch((error: unknown) => error); + await waitFor(() => startCalls.length === 1); + clientStopped = true; + await client.stop(); + await waitFor(() => canceled); + + expect(await rejected).toBe(client["_payloadCancellation"].signal.reason); + expect(startCalls).toHaveLength(1); + }); + }); + it("advertises support, awaits input hydration and externalizes worker output", async () => { await worker.start(); await waitFor(() => !!subscription); From f3ea69fc79eb594961ab4d57651f5e1360fb9127 Mon Sep 17 00:00:00 2001 From: wangbill Date: Thu, 22 Oct 2026 14:28:47 -0400 Subject: [PATCH 11/11] fix(client): centralize unary RPC error boundaries Keep a single payload/metadata/RPC/hydration pipeline in _call. Invoke operation mapping or retry callbacks only for actual RPC failures; preserve original non-RPC errors and cancellation, per-attempt fresh metadata, one externalization/hydration, and no-generator dispatch timing. Route start, completion waits, rewind and restart through that shared boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91 --- CHANGELOG.md | 3 + README.md | 6 +- packages/durabletask-js/src/client/client.ts | 207 +++++++-------- .../test/client-error-cause.spec.ts | 241 +++++++++++++++++- .../durabletask-js/test/client-wait.spec.ts | 147 +++++++++++ 5 files changed, 493 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d68cb70d..52f244be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,9 @@ ### Fixes +- Share the unary client payload/metadata/RPC/hydration pipeline across scheduling, waits, rewind, + and restart. Only actual RPC failures enter operation-specific error translation or retry; + storage and metadata failures keep their original identity, and cancellation prevents retries. - Restrict scheduling error translation to `startInstance` RPC failures. Payload upload and metadata errors retain their original identity instead of becoming duplicate-ID or argument errors. - Limit completion-wait deadline retries to scheduler RPC failures. Payload download errors, diff --git a/README.md b/README.md index 42003dc4..2759742a 100644 --- a/README.md +++ b/README.md @@ -39,8 +39,10 @@ orchestration output failures still produce non-retriable failed completions. To (`processOrchestratorRequest` and `processEntityBatchRequest`) instead reject with the original unconverted storage error so the host can apply its own retry policy. -Completion waits retry scheduler RPC deadline failures, not payload download failures. Storage -errors propagate unchanged, including gRPC deadline errors from custom stores. +All unary client operations share the same payload and metadata pipeline. Only actual scheduler +RPC failures enter operation-specific error translation or retry; storage and metadata errors +propagate unchanged. Completion waits retry scheduler deadlines with fresh metadata, without +re-uploading the request or retrying response hydration. Cancellation stops RPCs and retry backoff. Azure-managed builders expose `.useExternalizedPayloads({ store })` and require core `>=0.4.1` for the public payload types. This phase works with a custom/in-memory store and does not import diff --git a/packages/durabletask-js/src/client/client.ts b/packages/durabletask-js/src/client/client.ts index ec9109f2..0b56ad04 100644 --- a/packages/durabletask-js/src/client/client.ts +++ b/packages/durabletask-js/src/client/client.ts @@ -170,20 +170,41 @@ export class TaskHubGrpcClient { await new Promise((resolve) => setTimeout(resolve, 1000)); } + // The RPC-only handler may throw a mapped error, return true to retry the RPC, + // or return false to rethrow the original error. private async _call( method: Parameters>[0], request: TReq, metadataGenerator?: MetadataGenerator, signal?: AbortSignal, + handleRpcError?: (error: unknown, effectiveSignal?: AbortSignal) => boolean | Promise, ): Promise { - if (!this._payloads) return callWithMetadata(method, request, metadataGenerator, signal); - const payloadSignal = signal - ? AbortSignal.any([signal, this._payloadCancellation.signal]) - : this._payloadCancellation.signal; - await this._payloads.externalize(request, payloadSignal); - const response = await callWithMetadata(method, request, metadataGenerator, payloadSignal); - await this._payloads.resolve(response, payloadSignal); - return response; + const effectiveSignal = this._payloads + ? signal + ? AbortSignal.any([signal, this._payloadCancellation.signal]) + : this._payloadCancellation.signal + : signal; + effectiveSignal?.throwIfAborted(); + if (this._payloads) await this._payloads.externalize(request, effectiveSignal); + while (true) { + effectiveSignal?.throwIfAborted(); + let attemptMetadata: MetadataGenerator | undefined; + if (metadataGenerator) { + const metadata = await metadataGenerator(); + attemptMetadata = async () => metadata; + } + let response: TRes; + try { + response = await callWithMetadata(method, request, attemptMetadata, effectiveSignal); + } catch (error) { + effectiveSignal?.throwIfAborted(); + if ((await handleRpcError?.(error, effectiveSignal)) === true) continue; + throw error; + } + effectiveSignal?.throwIfAborted(); + if (this._payloads) await this._payloads.resolve(response, effectiveSignal); + return response; + } } /** @@ -281,27 +302,26 @@ export class TaskHubGrpcClient { ClientLogs.schedulingOrchestration(this._logger, req.getInstanceid(), name, inputSizeInBytes); try { - const signal = this._payloads ? this._payloadCancellation.signal : undefined; - if (this._payloads) await this._payloads.externalize(req, signal); - const metadata = this._metadataGenerator ? await this._metadataGenerator() : new grpc.Metadata(); - let res: pb.CreateInstanceResponse; - try { - res = await callWithMetadata(this._stub.startInstance.bind(this._stub), req, async () => metadata, signal); - } catch (e: unknown) { - if (e instanceof Error && "code" in e) { - const grpcError = e as grpc.ServiceError; - const message = grpcError.details || grpcError.message; - if (grpcError.code === grpc.status.ALREADY_EXISTS) { - throw new OrchestrationAlreadyExistsError(message, { cause: e }); - } - if (grpcError.code === grpc.status.INVALID_ARGUMENT) { - throw new TypeError(message, { cause: e }); + const res = await this._call( + this._stub.startInstance.bind(this._stub), + req, + this._metadataGenerator, + undefined, + (e: unknown) => { + if (e instanceof Error && "code" in e) { + const grpcError = e as grpc.ServiceError; + const message = grpcError.details || grpcError.message; + if (grpcError.code === grpc.status.ALREADY_EXISTS) { + throw new OrchestrationAlreadyExistsError(message, { cause: e }); + } + if (grpcError.code === grpc.status.INVALID_ARGUMENT) { + throw new TypeError(message, { cause: e }); + } + // The JS SDK has no named cancellation error contract, so preserve CANCELLED as a ServiceError. } - // The JS SDK has no named cancellation error contract, so preserve CANCELLED as a ServiceError. - } - throw e; - } - if (this._payloads) await this._payloads.resolve(res, signal); + return false; + }, + ); setSpanOk(span); return res.getInstanceid(); @@ -441,31 +461,19 @@ export class TaskHubGrpcClient { private async _waitForCompletion(req: pb.GetInstanceRequest, signal: AbortSignal): Promise { const backoff = new ExponentialBackoff({ initialDelayMs: 100, maxDelayMs: 1000, jitterFactor: 0 }); - const waitSignal = this._payloads ? AbortSignal.any([signal, this._payloadCancellation.signal]) : signal; - while (true) { - waitSignal.throwIfAborted(); - // Metadata failures must not enter the remote-deadline retry path. - const metadata = this._metadataGenerator ? await this._metadataGenerator() : new grpc.Metadata(); - let response: pb.GetInstanceResponse; - try { - // GetInstanceRequest has no externalizable fields; isolate RPC errors from response hydration. - response = await callWithMetadata( - this._stub.waitForInstanceCompletion.bind(this._stub), - req, - async () => metadata, - waitSignal, - ); - } catch (error) { - waitSignal.throwIfAborted(); + return this._call( + this._stub.waitForInstanceCompletion.bind(this._stub), + req, + this._metadataGenerator, + signal, + async (error, effectiveSignal) => { if (error instanceof Error && "code" in error && error.code === grpc.status.DEADLINE_EXCEEDED) { - await backoff.wait(waitSignal); - continue; + await backoff.wait(effectiveSignal); + return true; } - throw error; - } - if (this._payloads) await this._payloads.resolve(response, waitSignal); - return response; - } + return false; + }, + ); } private async _withWaitCancellation( @@ -691,35 +699,34 @@ export class TaskHubGrpcClient { ClientLogs.rewindingInstance(this._logger, instanceId, reason); - try { - await this._call( - this._stub.rewindInstance.bind(this._stub), - req, - this._metadataGenerator, - ); - } catch (e) { - // Handle gRPC errors and convert them to appropriate errors - if (e instanceof Error && "code" in e) { - const grpcError = e as grpc.ServiceError; - if (grpcError.code === grpc.status.NOT_FOUND) { - throw new Error(`An orchestration with the instanceId '${instanceId}' was not found.`, { cause: e }); - } - if (grpcError.code === grpc.status.FAILED_PRECONDITION) { - throw new Error( - grpcError.details || - `Cannot rewind orchestration '${instanceId}': it is in a state that does not allow rewinding.`, - { cause: e }, - ); - } - if (grpcError.code === grpc.status.UNIMPLEMENTED) { - throw new Error(grpcError.details || `The rewind operation is not supported by the backend.`, { cause: e }); - } - if (grpcError.code === grpc.status.CANCELLED) { - throw new Error(`The rewind operation for '${instanceId}' was canceled.`, { cause: e }); + await this._call( + this._stub.rewindInstance.bind(this._stub), + req, + this._metadataGenerator, + undefined, + (e: unknown) => { + if (e instanceof Error && "code" in e) { + const grpcError = e as grpc.ServiceError; + if (grpcError.code === grpc.status.NOT_FOUND) { + throw new Error(`An orchestration with the instanceId '${instanceId}' was not found.`, { cause: e }); + } + if (grpcError.code === grpc.status.FAILED_PRECONDITION) { + throw new Error( + grpcError.details || + `Cannot rewind orchestration '${instanceId}': it is in a state that does not allow rewinding.`, + { cause: e }, + ); + } + if (grpcError.code === grpc.status.UNIMPLEMENTED) { + throw new Error(grpcError.details || `The rewind operation is not supported by the backend.`, { cause: e }); + } + if (grpcError.code === grpc.status.CANCELLED) { + throw new Error(`The rewind operation for '${instanceId}' was canceled.`, { cause: e }); + } } - } - throw e; - } + return false; + }, + ); } /** @@ -756,28 +763,28 @@ export class TaskHubGrpcClient { ClientLogs.restartingInstance(this._logger, instanceId, restartWithNewInstanceId); - try { - const res = await this._call( - this._stub.restartInstance.bind(this._stub), - req, - this._metadataGenerator, - ); - return res.getInstanceid(); - } catch (e) { - if (e instanceof Error && "code" in e) { - const grpcError = e as grpc.ServiceError; - if (grpcError.code === grpc.status.NOT_FOUND) { - throw new Error(`An orchestration with the instanceId '${instanceId}' was not found.`, { cause: e }); - } - if (grpcError.code === grpc.status.FAILED_PRECONDITION) { - throw new Error(`An orchestration with the instanceId '${instanceId}' cannot be restarted.`, { cause: e }); - } - if (grpcError.code === grpc.status.CANCELLED) { - throw new Error(`The restartOrchestration operation was canceled.`, { cause: e }); + const res = await this._call( + this._stub.restartInstance.bind(this._stub), + req, + this._metadataGenerator, + undefined, + (e: unknown) => { + if (e instanceof Error && "code" in e) { + const grpcError = e as grpc.ServiceError; + if (grpcError.code === grpc.status.NOT_FOUND) { + throw new Error(`An orchestration with the instanceId '${instanceId}' was not found.`, { cause: e }); + } + if (grpcError.code === grpc.status.FAILED_PRECONDITION) { + throw new Error(`An orchestration with the instanceId '${instanceId}' cannot be restarted.`, { cause: e }); + } + if (grpcError.code === grpc.status.CANCELLED) { + throw new Error(`The restartOrchestration operation was canceled.`, { cause: e }); + } } - } - throw e; - } + return false; + }, + ); + return res.getInstanceid(); } /** diff --git a/packages/durabletask-js/test/client-error-cause.spec.ts b/packages/durabletask-js/test/client-error-cause.spec.ts index 7be54bbb..c5d4942f 100644 --- a/packages/durabletask-js/test/client-error-cause.spec.ts +++ b/packages/durabletask-js/test/client-error-cause.spec.ts @@ -2,7 +2,10 @@ // Licensed under the MIT License. import * as grpc from "@grpc/grpc-js"; -import { TaskHubGrpcClient } from "../src"; +import { EventEmitter } from "events"; +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import { NoOpLogger, PayloadStore, TaskHubGrpcClient } from "../src"; +import * as pb from "../src/proto/orchestrator_service_pb"; /** * Creates a mock gRPC ServiceError with the specified status code and details. @@ -22,6 +25,232 @@ function getStub(client: TaskHubGrpcClient): any { return (client as any)._stub; } +class ErrorDomainPayloadStore extends PayloadStore { + readonly values = new Map(); + upload = jest.fn(async (value: string, _signal?: AbortSignal) => { + const token = `payload:${this.values.size}`; + this.values.set(token, value); + return token; + }); + download = jest.fn(async (token: string, _signal?: AbortSignal) => { + if (!this.values.has(token)) throw new Error(`Missing payload: ${token}`); + return this.values.get(token)!; + }); + isKnownPayloadToken(value: string): boolean { + return value.startsWith("payload:"); + } +} + +type ManagementRequest = pb.RewindInstanceRequest | pb.RestartInstanceRequest; +type ManagementResponse = pb.RewindInstanceResponse | pb.RestartInstanceResponse; +type ManagementCallback = (error: grpc.ServiceError | null, response: ManagementResponse) => void; + +const managementOperations: { + name: "rewind" | "restart"; + rpc: "rewindInstance" | "restartInstance"; + invoke: (client: TaskHubGrpcClient) => Promise; + response: () => ManagementResponse; + mappings: { code: grpc.status; message: string }[]; +}[] = [ + { + name: "rewind", + rpc: "rewindInstance", + invoke: (client: TaskHubGrpcClient) => client.rewindInstance("test-instance", "test reason"), + response: () => new pb.RewindInstanceResponse(), + mappings: [ + { code: grpc.status.NOT_FOUND, message: "An orchestration with the instanceId 'test-instance' was not found." }, + { code: grpc.status.FAILED_PRECONDITION, message: "backend details" }, + { code: grpc.status.UNIMPLEMENTED, message: "backend details" }, + { code: grpc.status.CANCELLED, message: "The rewind operation for 'test-instance' was canceled." }, + ], + }, + { + name: "restart", + rpc: "restartInstance", + invoke: (client: TaskHubGrpcClient) => client.restartOrchestration("test-instance", true), + response: () => new pb.RestartInstanceResponse().setInstanceid("restarted-instance"), + mappings: [ + { code: grpc.status.NOT_FOUND, message: "An orchestration with the instanceId 'test-instance' was not found." }, + { + code: grpc.status.FAILED_PRECONDITION, + message: "An orchestration with the instanceId 'test-instance' cannot be restarted.", + }, + { code: grpc.status.CANCELLED, message: "The restartOrchestration operation was canceled." }, + ], + }, +]; + +describe.each([true, false])("unary management error domains with payloads enabled=%s", (payloadsEnabled) => { + let client: TaskHubGrpcClient; + let store: ErrorDomainPayloadStore; + let metadataGenerator: jest.Mock, []>; + const call: grpc.ClientUnaryCall = Object.assign(new EventEmitter(), { + cancel: jest.fn(), + getPeer: () => "test", + getAuthContext: () => null, + }); + + beforeEach(() => { + store = new ErrorDomainPayloadStore(); + metadataGenerator = jest.fn(async () => new grpc.Metadata()); + client = new TaskHubGrpcClient({ + payloads: payloadsEnabled ? { store, thresholdBytes: 1 } : undefined, + metadataGenerator, + logger: new NoOpLogger(), + }); + }); + + afterEach(() => { + client["_stub"].close(); + }); + + if (payloadsEnabled) { + it("externalizes eligible input once while retrying only RPCs with fresh metadata", async () => { + const request = new pb.CreateInstanceRequest() + .setName("workflow") + .setInstanceid("instance") + .setInput(new StringValue().setValue('"input"')); + const response = new pb.CreateInstanceResponse().setInstanceid("instance"); + const failure = Object.freeze(createGrpcError(grpc.status.DEADLINE_EXCEEDED)); + const sentTokens: (string | undefined)[] = []; + const rpc = jest.fn< + grpc.ClientUnaryCall, + [ + pb.CreateInstanceRequest, + grpc.Metadata, + (error: grpc.ServiceError | null, res: pb.CreateInstanceResponse) => void, + ] + >((req, _metadata, callback) => { + sentTokens.push(req.getInput()?.getValue()); + callback(sentTokens.length < 3 ? failure : null, response); + return call; + }); + metadataGenerator.mockImplementation(async () => { + const metadata = new grpc.Metadata(); + metadata.set("attempt", String(metadataGenerator.mock.calls.length)); + return metadata; + }); + const handleRpcError = jest.fn((error: unknown, signal?: AbortSignal) => { + expect(error).toBe(failure); + expect(signal).toBe(store.upload.mock.calls[0][1]); + return true; + }); + const resolve = jest.spyOn(client["_payloads"]!, "resolve"); + const controller = new AbortController(); + + await expect(client["_call"](rpc, request, metadataGenerator, controller.signal, handleRpcError)).resolves.toBe( + response, + ); + + expect(store.upload).toHaveBeenCalledTimes(1); + expect(store.values.get("payload:0")).toBe('"input"'); + expect(sentTokens).toEqual(["payload:0", "payload:0", "payload:0"]); + expect(rpc.mock.calls.every(([req]) => req === request)).toBe(true); + expect(rpc.mock.calls.map(([, metadata]) => metadata.get("attempt"))).toEqual([["1"], ["2"], ["3"]]); + expect(handleRpcError).toHaveBeenCalledTimes(2); + expect(resolve).toHaveBeenCalledTimes(1); + expect(resolve.mock.invocationCallOrder[0]).toBeGreaterThan(rpc.mock.invocationCallOrder[2]); + }); + } + + describe.each(managementOperations)("$name", ({ name, rpc: rpcName, invoke, response, mappings }) => { + let rpc: jest.Mock; + + beforeEach(() => { + rpc = jest.fn( + (_req, _metadata, callback) => { + callback(null, response()); + return call; + }, + ); + Object.defineProperty(client["_stub"], rpcName, { value: rpc }); + }); + + it.each(mappings)( + "preserves a reused frozen metadata error with mapped status $code without an RPC", + async ({ code }) => { + const failure = Object.freeze(createGrpcError(code, "metadata failed")); + metadataGenerator.mockRejectedValue(failure); + + await expect(invoke(client)).rejects.toBe(failure); + await expect(invoke(client)).rejects.toBe(failure); + expect(metadataGenerator).toHaveBeenCalledTimes(2); + expect(rpc).not.toHaveBeenCalled(); + expect(store.upload).toHaveBeenCalledTimes(payloadsEnabled && name === "rewind" ? 2 : 0); + expect(store.download).not.toHaveBeenCalled(); + }, + ); + + it.each(mappings)("maps genuine RPC status $code with the original frozen cause", async ({ code, message }) => { + const failure = Object.freeze(createGrpcError(code, "backend details")); + rpc.mockImplementation((_req, _metadata, callback) => { + callback(failure, response()); + return call; + }); + + const pending = invoke(client); + await expect(pending).rejects.toBeInstanceOf(Error); + await expect(pending).rejects.toMatchObject({ message, cause: failure }); + expect(rpc).toHaveBeenCalledTimes(1); + expect(metadataGenerator).toHaveBeenCalledTimes(1); + }); + + it("preserves an unmapped RPC error without retry or mutation", async () => { + const failure = Object.freeze(createGrpcError(grpc.status.INTERNAL, "unknown scheduler error")); + rpc.mockImplementation((_req, _metadata, callback) => { + callback(failure, response()); + return call; + }); + + await expect(invoke(client)).rejects.toBe(failure); + expect(rpc).toHaveBeenCalledTimes(1); + }); + + it("preserves successful request options and result", async () => { + const result = await invoke(client); + const request = rpc.mock.calls[0][0]; + expect(request.getInstanceid()).toBe("test-instance"); + if (request instanceof pb.RewindInstanceRequest) { + expect(request.getReason()?.getValue()).toBe(payloadsEnabled ? "payload:0" : "test reason"); + expect(store.upload).toHaveBeenCalledTimes(payloadsEnabled ? 1 : 0); + expect(result).toBeUndefined(); + } else { + expect(request.getRestartwithnewinstanceid()).toBe(true); + expect(store.upload).not.toHaveBeenCalled(); + expect(result).toBe("restarted-instance"); + } + expect(rpc).toHaveBeenCalledTimes(1); + }); + + if (payloadsEnabled) { + it.each(mappings)("does not translate a mapped-status $code abort reason from the RPC", async ({ code }) => { + const reason = Object.freeze(createGrpcError(code, "caller canceled")); + rpc.mockImplementation((_req, _metadata, callback) => { + client["_payloadCancellation"].abort(reason); + callback(createGrpcError(grpc.status.CANCELLED), response()); + return call; + }); + + await expect(invoke(client)).rejects.toBe(reason); + expect(rpc).toHaveBeenCalledTimes(1); + expect(metadataGenerator).toHaveBeenCalledTimes(1); + }); + } + + if (payloadsEnabled && name === "rewind") { + it.each(mappings)("preserves upload status $code without invoking metadata or the RPC", async ({ code }) => { + const failure = Object.freeze(createGrpcError(code, "upload failed")); + store.upload.mockRejectedValue(failure); + + await expect(invoke(client)).rejects.toBe(failure); + expect(store.upload).toHaveBeenCalledTimes(1); + expect(metadataGenerator).not.toHaveBeenCalled(); + expect(rpc).not.toHaveBeenCalled(); + }); + } + }); +}); + describe("TaskHubGrpcClient error cause preservation", () => { let client: TaskHubGrpcClient; @@ -51,10 +280,7 @@ describe("TaskHubGrpcClient error cause preservation", () => { }); it("should preserve error cause for FAILED_PRECONDITION gRPC errors", async () => { - const grpcError = createGrpcError( - grpc.status.FAILED_PRECONDITION, - "Orchestration is running", - ); + const grpcError = createGrpcError(grpc.status.FAILED_PRECONDITION, "Orchestration is running"); getStub(client).rewindInstance = (_req: any, _metadata: any, callback: any) => { callback(grpcError, null); @@ -150,10 +376,7 @@ describe("TaskHubGrpcClient error cause preservation", () => { }); it("should preserve error cause for FAILED_PRECONDITION gRPC errors", async () => { - const grpcError = createGrpcError( - grpc.status.FAILED_PRECONDITION, - "Orchestration is still running", - ); + const grpcError = createGrpcError(grpc.status.FAILED_PRECONDITION, "Orchestration is still running"); getStub(client).restartInstance = (_req: any, _metadata: any, callback: any) => { callback(grpcError, null); diff --git a/packages/durabletask-js/test/client-wait.spec.ts b/packages/durabletask-js/test/client-wait.spec.ts index e3db56c9..f71f933a 100644 --- a/packages/durabletask-js/test/client-wait.spec.ts +++ b/packages/durabletask-js/test/client-wait.spec.ts @@ -8,6 +8,7 @@ import { TaskHubGrpcClient } from "../src/client/client"; import { OrchestrationState } from "../src/orchestration/orchestration-state"; import { TimeoutError } from "../src/exception/timeout-error"; import { NoOpLogger } from "../src/types/logger.type"; +import { PayloadStore } from "../src/payloads/payload-store"; import * as pb from "../src/proto/orchestrator_service_pb"; type Wait = ( @@ -33,6 +34,152 @@ function completedResponse(): pb.GetInstanceResponse { return response; } +class UnaryWaitPayloadStore extends PayloadStore { + upload = jest.fn(async (_value: string, _signal?: AbortSignal) => "payload:input"); + download = jest.fn(async (_token: string, _signal?: AbortSignal) => '"done"'); + isKnownPayloadToken(value: string): boolean { + return value.startsWith("payload:"); + } +} + +describe("payload-enabled unary completion pipeline", () => { + let client: TaskHubGrpcClient; + let store: UnaryWaitPayloadStore; + let callback: Callback; + const cancel = jest.fn(); + const call: grpc.ClientUnaryCall = Object.assign(new EventEmitter(), { + cancel, + getPeer: () => "test", + getAuthContext: () => null, + }); + + const rpc = jest.fn(); + const metadataGenerator = jest.fn, []>(); + + beforeEach(() => { + jest.useFakeTimers(); + cancel.mockReset(); + store = new UnaryWaitPayloadStore(); + metadataGenerator.mockReset().mockImplementation(async () => { + const metadata = new grpc.Metadata(); + metadata.set("attempt", String(metadataGenerator.mock.calls.length)); + return metadata; + }); + rpc.mockReset().mockImplementation((_req, _metadata, cb) => { + callback = cb; + return call; + }); + client = new TaskHubGrpcClient({ + payloads: { store, thresholdBytes: 1 }, + metadataGenerator, + logger: new NoOpLogger(), + }); + Object.defineProperty(client["_stub"], "waitForInstanceCompletion", { value: rpc }); + }); + + afterEach(() => { + client["_stub"].close(); + jest.useRealTimers(); + }); + + it("does not retry a frozen metadata deadline after a real RPC retry", async () => { + const failure = Object.freeze(grpcError(grpc.status.DEADLINE_EXCEEDED)); + metadataGenerator.mockResolvedValueOnce(new grpc.Metadata()).mockRejectedValue(failure); + const pending = client.waitForOrchestrationCompletion("instance", true, 5); + const assertion = expect(pending).rejects.toBe(failure); + await jest.advanceTimersByTimeAsync(0); + callback(grpcError(grpc.status.DEADLINE_EXCEEDED), new pb.GetInstanceResponse()); + await jest.advanceTimersByTimeAsync(100); + await assertion; + await jest.advanceTimersByTimeAsync(1000); + + expect(metadataGenerator).toHaveBeenCalledTimes(2); + expect(rpc).toHaveBeenCalledTimes(1); + expect(store.upload).not.toHaveBeenCalled(); + expect(store.download).not.toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(0); + }); + + it("reuses the request across RPC retries, regenerates metadata, and hydrates only the successful response", async () => { + const pending = client.waitForOrchestrationCompletion("instance", true, 5); + await jest.advanceTimersByTimeAsync(0); + const response = completedResponse(); + response.getOrchestrationstate()!.setOutput(new StringValue().setValue("payload:output")); + callback(grpcError(grpc.status.DEADLINE_EXCEEDED), response); + await jest.advanceTimersByTimeAsync(100); + callback(grpcError(grpc.status.DEADLINE_EXCEEDED), response); + await jest.advanceTimersByTimeAsync(200); + callback(null, response); + await expect(pending).resolves.toMatchObject({ serializedOutput: '"done"' }); + + expect(rpc).toHaveBeenCalledTimes(3); + expect(rpc.mock.calls.every(([request]) => request === rpc.mock.calls[0][0])).toBe(true); + expect(rpc.mock.calls.map(([, metadata]) => metadata.get("attempt"))).toEqual([["1"], ["2"], ["3"]]); + expect(store.upload).not.toHaveBeenCalled(); + expect(store.download).toHaveBeenCalledTimes(1); + expect(jest.getTimerCount()).toBe(0); + }); + + it.each(["metadata", "rpc", "backoff", "hydration"] as const)( + "stop cancels %s without starting later metadata or RPC attempts", + async (stage) => { + let resolveMetadata: ((metadata: grpc.Metadata) => void) | undefined; + if (stage === "metadata") { + metadataGenerator.mockImplementation(() => new Promise((resolve) => (resolveMetadata = resolve))); + } + if (stage === "hydration") { + store.download.mockImplementation( + (_token, signal) => + new Promise((_resolve, reject) => { + signal!.addEventListener("abort", () => reject(signal!.reason), { once: true }); + }), + ); + } + const pending = client.waitForOrchestrationCompletion("instance", true, 5); + const rejected = pending.catch((error: unknown) => error); + await jest.advanceTimersByTimeAsync(0); + if (stage === "backoff") { + callback(grpcError(grpc.status.DEADLINE_EXCEEDED), new pb.GetInstanceResponse()); + } else if (stage === "hydration") { + const response = completedResponse(); + response.getOrchestrationstate()!.setOutput(new StringValue().setValue("payload:output")); + callback(null, response); + } + await jest.advanceTimersByTimeAsync(0); + const stopping = client.stop(); + resolveMetadata?.(new grpc.Metadata()); + await jest.advanceTimersByTimeAsync(10000); + await stopping; + + expect(await rejected).toBe(client["_payloadCancellation"].signal.reason); + expect(metadataGenerator).toHaveBeenCalledTimes(1); + expect(rpc).toHaveBeenCalledTimes(stage === "metadata" ? 0 : 1); + expect(cancel).toHaveBeenCalledTimes(stage === "rpc" ? 1 : 0); + expect(store.download).toHaveBeenCalledTimes(stage === "hydration" ? 1 : 0); + expect(jest.getTimerCount()).toBe(0); + }, + ); + + it.each(["abort", "timeout"] as const)("does not restart after %s in payload-enabled backoff", async (cause) => { + const controller = new AbortController(); + const reason = Object.freeze(grpcError(grpc.status.DEADLINE_EXCEEDED)); + const pending = client.waitForOrchestrationCompletion("instance", true, 0.05, controller.signal); + const rejected = pending.catch((error: unknown) => error); + await jest.advanceTimersByTimeAsync(0); + callback(grpcError(grpc.status.DEADLINE_EXCEEDED), new pb.GetInstanceResponse()); + await jest.advanceTimersByTimeAsync(0); + if (cause === "abort") controller.abort(reason); + await jest.advanceTimersByTimeAsync(10000); + + if (cause === "abort") expect(await rejected).toBe(reason); + else expect(await rejected).toBeInstanceOf(TimeoutError); + expect(metadataGenerator).toHaveBeenCalledTimes(1); + expect(rpc).toHaveBeenCalledTimes(1); + expect(cancel).not.toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(0); + }); +}); + describe.each([ ["waitForOrchestrationStart", "waitForInstanceStart", false, "start"], ["waitForOrchestrationCompletion", "waitForInstanceCompletion", true, "complete"],