diff --git a/packages/azure-functions-durable/CHANGELOG.md b/packages/azure-functions-durable/CHANGELOG.md index 00858aea..7b9aadaa 100644 --- a/packages/azure-functions-durable/CHANGELOG.md +++ b/packages/azure-functions-durable/CHANGELOG.md @@ -2,4 +2,14 @@ ## TBD +- Change ([#321](https://github.com/microsoft/durabletask-js/issues/321)): a plain **synchronous, + single-argument, non-generator** orchestrator `(ctx) => value` is now always treated as a + **core-native** orchestrator and receives the core `OrchestrationContext` (`instanceId`, + `callActivity`, `newGuid`, …). This fixes #321, where such a handler previously received only the + classic `{ df, log }` context, so core members read as `undefined` / threw `TypeError`. + - **BREAKING:** a *classic* v3 orchestrator written as a plain **non-generator** function that uses + `context.df.*` / `context.log` in this exact shape (sync, single-arg, non-generator) no longer + receives the classic context. Convert it to the standard classic **generator** form (`function*`, + unaffected) or to core-native (`ctx.*`). Sync generators, `async` orchestrators, async generators, + and 2-argument `(ctx, input)` handlers are all unchanged. - Details to be finalized at release time. diff --git a/packages/azure-functions-durable/README.md b/packages/azure-functions-durable/README.md index 49adb637..d45ea14d 100644 --- a/packages/azure-functions-durable/README.md +++ b/packages/azure-functions-durable/README.md @@ -52,6 +52,14 @@ changed: (testing utilities), `ManagedIdentityTokenSource`, and the entity-lock types above. `TaskFailedError` is re-exported from the core SDK (aggregate failures surface as JS-native `AggregateError`); use the core `TestOrchestrationWorker` / `TestOrchestrationClient` for orchestration unit tests. +- **A plain non-generator classic orchestrator is no longer supported.** A classic v3 orchestrator + written as a *synchronous, single-argument, non-generator* function `(context) => context.df.*` + (one that never `yield`s) is now treated as a **core-native** orchestrator and receives the core + `OrchestrationContext`, which has no `.df`. This resolves + [#321](https://github.com/microsoft/durabletask-js/issues/321), where a core-native + `(ctx) => ctx.instanceId` was mis-routed to the classic context. Standard classic orchestrators — + sync **generators** (`function*`) using `context.df.*` — are unaffected; convert any non-generator + classic orchestrator to generator form, or to the core-native `ctx.*` API. ## Getting started diff --git a/packages/azure-functions-durable/src/orchestration-context.ts b/packages/azure-functions-durable/src/orchestration-context.ts index 14339102..efef5321 100644 --- a/packages/azure-functions-durable/src/orchestration-context.ts +++ b/packages/azure-functions-durable/src/orchestration-context.ts @@ -218,8 +218,11 @@ export type ClassicOrchestrator = ( * generator to `yield` durable tasks), so `async` unambiguously means core-native. * - `function*` (classic v3 sync generator) is wrapped; the engine cannot drive a sync generator, so * the wrapper delegates to it via `yield*`. - * - A plain SYNC (non-generator) function is ambiguous, so fall back to arity: a lone `context` - * parameter is treated as classic, `(ctx, input)` as core-native. + * - A plain SYNC (non-generator) function is treated as CORE-NATIVE and passes through unchanged, + * regardless of arity, receiving the core {@link OrchestrationContext}. This resolves #321 + * (`(ctx) => ctx.instanceId` now works). A classic v3 orchestrator written as a plain + * non-generator using `context.df.*` is NO LONGER supported — convert it to a `function*` + * generator (the standard classic shape, unaffected) or to the core-native `ctx.*` API. */ export function wrapOrchestrator(handler: TOrchestrator | ClassicOrchestrator): TOrchestrator { if (typeof handler === "function" && isClassicOrchestrator(handler)) { @@ -232,21 +235,7 @@ export function wrapOrchestrator(handler: TOrchestrator | ClassicOrchestrator): ctx: OrchestrationContext, input: unknown, ): AsyncGenerator, unknown, unknown> { - const df = new DurableOrchestrationContext(ctx, input); - // Replay-safe logger: output is suppressed while the engine replays history, matching the - // .NET/Python providers. Core Logger has no `trace`/plain `log`, so log→info and trace→debug. - const logger = ctx.createReplaySafeLogger(new ConsoleLogger()); - const toArgs = (a: unknown[]) => a as [string, ...unknown[]]; - const classicCtx: ClassicOrchestrationContext = { - df, - log: (...a) => logger.info(...toArgs(a)), - info: (...a) => logger.info(...toArgs(a)), - warn: (...a) => logger.warn(...toArgs(a)), - error: (...a) => logger.error(...toArgs(a)), - debug: (...a) => logger.debug(...toArgs(a)), - trace: (...a) => logger.debug(...toArgs(a)), - }; - const result = classic(classicCtx); + const result = classic(createClassicContext(ctx, input)); if (isDrivableGenerator(result)) { return yield* result; } @@ -257,6 +246,31 @@ export function wrapOrchestrator(handler: TOrchestrator | ClassicOrchestrator): return handler as TOrchestrator; } +/** + * @hidden + * Builds the classic v3 context (`df` + replay-safe log helpers) handed to a wrapped classic + * orchestrator. Only sync generators (`function*`) are wrapped, so this context is only ever given + * to a classic body that uses `context.df.*` / `context.log`. Core-native orchestrators (async, + * async generators, and plain sync non-generators) are never wrapped — they receive the core + * {@link OrchestrationContext} directly. + */ +function createClassicContext(ctx: OrchestrationContext, input: unknown): ClassicOrchestrationContext { + const df = new DurableOrchestrationContext(ctx, input); + // Replay-safe logger: output is suppressed while the engine replays history, matching the + // .NET/Python providers. Core Logger has no `trace`/plain `log`, so log→info and trace→debug. + const logger = ctx.createReplaySafeLogger(new ConsoleLogger()); + const toArgs = (a: unknown[]) => a as [string, ...unknown[]]; + return { + df, + log: (...a) => logger.info(...toArgs(a)), + info: (...a) => logger.info(...toArgs(a)), + warn: (...a) => logger.warn(...toArgs(a)), + error: (...a) => logger.error(...toArgs(a)), + debug: (...a) => logger.debug(...toArgs(a)), + trace: (...a) => logger.debug(...toArgs(a)), + }; +} + /** * @hidden * Classifies a handler as a classic v3 orchestrator (must be wrapped) versus a core-native one @@ -274,9 +288,11 @@ function isClassicOrchestrator(handler: TOrchestrator | ClassicOrchestrator): bo if (kind === "GeneratorFunction") { return true; // classic v3: a sync generator the engine can't drive on its own. } - // Plain SYNC (non-generator) function: kind is ambiguous, so fall back to arity. A lone - // `context` parameter is the classic v3 shape; `(ctx, input)` is core-native. - return handler.length <= 1; + // Plain SYNC (non-generator) function is CORE-NATIVE regardless of arity: it passes through + // unchanged and receives the core OrchestrationContext. Only a sync generator (function*) is + // classic (handled above). A classic v3 orchestrator written as a plain non-generator using + // `context.df.*` is no longer supported — see #321 and the package README/CHANGELOG. + return false; } /** diff --git a/packages/azure-functions-durable/test/unit/orchestration-context.spec.ts b/packages/azure-functions-durable/test/unit/orchestration-context.spec.ts index e1d39137..a21cd118 100644 --- a/packages/azure-functions-durable/test/unit/orchestration-context.spec.ts +++ b/packages/azure-functions-durable/test/unit/orchestration-context.spec.ts @@ -272,9 +272,9 @@ describe("wrapOrchestrator", () => { }); it("exposes a replay-safe logger as context.log/error on the classic context", async () => { - // A classic orchestrator may be a plain (non-generator) function that returns a value; the - // wrapper still invokes it with the full classic context, so log wiring is exercised here. - const classic = (context: ClassicOrchestrationContext): string => { + // A classic orchestrator is a sync generator; the wrapper invokes it with the full classic + // context, so log wiring is exercised here. + const classic = function* (context: ClassicOrchestrationContext): Generator, string, unknown> { context.log("hi"); context.error("boom"); return "logged"; @@ -292,6 +292,36 @@ describe("wrapOrchestrator", () => { expect(replaySafeLogger.info).toHaveBeenCalledWith("hi"); expect(replaySafeLogger.error).toHaveBeenCalledWith("boom"); }); + + it("passes a plain sync single-arg core-native orchestrator through unchanged (#321)", () => { + // #321: a plain sync, single-argument, non-generator `(ctx) => value` is CORE-NATIVE and passes + // through wrapOrchestrator UNCHANGED (identity), receiving the core OrchestrationContext directly. + // Before the fix it was mis-routed to the classic `{ df, log }` context, so `ctx.newGuid()` threw + // and `ctx.instanceId` was `undefined`. + const native = (ctx: OrchestrationContext): string => `${ctx.instanceId}:${ctx.newGuid()}`; + + const wrapped = wrapOrchestrator(native as unknown as TOrchestrator); + expect(wrapped).toBe(native as unknown as TOrchestrator); // passthrough (identity), not wrapped + + const { ctx, raw } = createFakeCoreContext(); + expect((wrapped as unknown as (c: OrchestrationContext) => string)(ctx)).toBe("instance-1:guid-1"); + expect(raw.newGuid).toHaveBeenCalledTimes(1); + }); + + it("treats a non-generator classic-style single-arg orchestrator as core-native (breaking; #321)", () => { + // BREAKING: a classic v3 orchestrator written as a plain NON-generator `(context) => context.df.*` + // is now treated as core-native and passes through unchanged — it receives the core context, which + // has no `.df`. Standard classic orchestrators are sync GENERATORS (function*) and are unaffected. + const classicLike = ((context: ClassicOrchestrationContext): string => + `v:${String((context as unknown as { df?: unknown }).df)}`) as unknown as TOrchestrator; + + const wrapped = wrapOrchestrator(classicLike); + expect(wrapped).toBe(classicLike); // passthrough, not wrapped + + const { ctx } = createFakeCoreContext(); + // The core context has no `df`, so a `.df`-using classic body degrades — the documented breaking change. + expect((wrapped as unknown as (c: OrchestrationContext) => string)(ctx)).toBe("v:undefined"); + }); }); describe("wrapOrchestrator end-to-end (real core executor)", () => { @@ -375,4 +405,29 @@ describe("wrapOrchestrator end-to-end (real core executor)", () => { await worker.stop(); } }); + + it("drives a plain sync single-argument core-native orchestrator through the executor (#321)", async () => { + // Regression (#321): a plain sync single-arg orchestrator `(ctx) => value` is the ambiguous shape. + // It is core-native and passes through unchanged, receiving the core context directly (so + // `ctx.instanceId` resolves). Before the fix it received the classic `{ df, log }` context, so + // `ctx.instanceId` was `undefined` and the orchestration completed with the garbage output + // "sync-native:undefined". + const backend = new InMemoryOrchestrationBackend(); + const worker = new TestOrchestrationWorker(backend); + + const syncNative = (ctx: OrchestrationContext): string => `sync-native:${ctx.instanceId}`; + worker.addNamedOrchestrator("syncNativeOrch", wrapOrchestrator(syncNative as unknown as TOrchestrator)); + + await worker.start(); + try { + const client = new TestOrchestrationClient(backend); + const id = await client.scheduleNewOrchestration("syncNativeOrch", undefined, "sync-instance"); + const state = await client.waitForOrchestrationCompletion(id, true, 10); + + expect(state?.runtimeStatus).toBe(OrchestrationStatus.COMPLETED); + expect(state?.serializedOutput).toBe(JSON.stringify("sync-native:sync-instance")); + } finally { + await worker.stop(); + } + }); }); diff --git a/test/e2e-functions/README.md b/test/e2e-functions/README.md index f2a393ae..0b765f30 100644 --- a/test/e2e-functions/README.md +++ b/test/e2e-functions/README.md @@ -66,6 +66,7 @@ tests are `it.skip` with a comment citing the same reason. | `orchestration-query.spec.ts` | `OrchestrationQueryTests` | `GetAllInstances` / `GetRunningInstances` | | `purge.spec.ts` | `PurgeInstancesTests` | purge by time / by entity id (purge-without-start-time #644 skipped) | | `class-based-entity.spec.ts` | `ClassBasedEntityTests` | class-based entity state | +| `orchestrator-shapes.spec.ts` | net-new (#321/#322) | `wrapOrchestrator` shape routing: classic v3 generator, core-native generator, and the ambiguous plain sync/async single-arg non-generator (`(ctx) => value`) reaching the core context | ### Node bug annotations honored diff --git a/test/e2e-functions/orchestrator-shapes.spec.ts b/test/e2e-functions/orchestrator-shapes.spec.ts new file mode 100644 index 00000000..5b3de1e0 --- /dev/null +++ b/test/e2e-functions/orchestrator-shapes.spec.ts @@ -0,0 +1,80 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +/** + * Functions host E2E — orchestrator-shape routing (#321/#322). + * + * Drives each orchestrator shape registered by the test-app's `OrchestratorShapes.ts` through the + * real `func start` host and asserts the correct output, proving `wrapOrchestrator` routes every + * shape correctly end-to-end (not only in the in-memory-executor unit tests). + * + * The headline case is `ShapeSyncNative` — the ambiguous plain sync, single-argument, non-generator + * shape from #321. Before #322 it was mis-detected as classic and silently completed with the + * garbage output `sync-native:undefined`; here we assert it returns `sync-native:`. + * + * Gated: skips cleanly unless the shared host was started by globalSetup (func + Azurite + a + * built/installed test-app all present). + */ + +import { + getOrchestrationDetails, + invokeHttpTrigger, + parseInstanceId, + parseStatusQueryGetUri, + readPreflight, + waitForOrchestrationState, +} from "./harness"; + +const preflight = readPreflight(); +const describeMaybe = preflight.ok ? describe : describe.skip; +const baseUrl = preflight.baseUrl ?? ""; + +if (!preflight.ok) { + console.warn(`[functions-e2e] orchestrator-shapes.spec skipped: ${preflight.reason}`); +} + +interface ShapeResult { + instanceId: string; + output: unknown; + outputString: string; +} + +async function startAndComplete(orchestrationName: string): Promise { + const response = await invokeHttpTrigger(baseUrl, "StartOrchestration", `?orchestrationName=${orchestrationName}`); + expect(response.status).toBe(202); // HttpStatusCode.Accepted + + const instanceId = parseInstanceId(response); + const statusQueryGetUri = parseStatusQueryGetUri(response); + await waitForOrchestrationState(statusQueryGetUri, "Completed", 30); + + const details = await getOrchestrationDetails(statusQueryGetUri); + return { instanceId, output: details.output, outputString: details.outputString }; +} + +describeMaybe("Functions host E2E — orchestrator shapes (#321/#322, AzureStorage)", () => { + it("classic v3 sync generator (context.df.*) routes to the classic context", async () => { + const { output } = await startAndComplete("ShapeClassicGenerator"); + expect(output).toBe("classic:echo:IN"); + }, 120_000); + + it("core-native async generator (single arg) is driven and round-trips an activity", async () => { + const { output } = await startAndComplete("ShapeNativeGenerator"); + expect(output).toBe("native-gen:echo:IN"); + }, 120_000); + + // Headline regression (#321/#322): before the fix this ambiguous shape received the classic + // { df, log } context (no instanceId) and silently completed with "sync-native:undefined". + it("plain sync single-arg non-generator reaches the core context (#321/#322)", async () => { + const { instanceId, output, outputString } = await startAndComplete("ShapeSyncNative"); + expect(instanceId).toBeTruthy(); + expect(output).toBe(`sync-native:${instanceId}`); + expect(outputString).not.toContain("undefined"); + }, 120_000); + + it("plain async single-arg non-generator reaches the core context", async () => { + const { instanceId, output, outputString } = await startAndComplete("ShapeAsyncNative"); + expect(instanceId).toBeTruthy(); + expect(output).toBe(`async-native:${instanceId}`); + expect(outputString).not.toContain("undefined"); + }, 120_000); +}); diff --git a/test/e2e-functions/test-app/src/functions/OrchestratorShapes.ts b/test/e2e-functions/test-app/src/functions/OrchestratorShapes.ts new file mode 100644 index 00000000..87f2d17a --- /dev/null +++ b/test/e2e-functions/test-app/src/functions/OrchestratorShapes.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +/** + * Orchestrator-shape routing coverage for the compat `durable-functions` package (#321/#322). + * + * The compat layer accepts BOTH the classic Durable Functions v3 orchestrator shape (a sync + * generator that uses `context.df.*`) AND the core-native durabletask-js shapes (async/sync, + * generator/non-generator, that use the core `OrchestrationContext` directly). `wrapOrchestrator` + * (packages/azure-functions-durable/src/orchestration-context.ts) must classify the shape and hand + * each orchestrator the right context. + * + * Regression guard for #321/#322: a plain SYNC, single-argument, NON-generator orchestrator + * `(ctx) => value` is the ambiguous shape. Before #322 it was mis-detected as classic and received a + * `{ df, log }` context with no core members, so `ctx.instanceId` was `undefined` and it silently + * completed with the garbage output `sync-native:undefined`. After #322 it is driven with the dual + * context and returns `sync-native:`. + * + * These functions are driven end-to-end through a real `func start` host by + * `test/e2e-functions/orchestrator-shapes.spec.ts`, using the generic `StartOrchestration` HTTP + * starter registered in `HelloCities.ts`. + */ + +import * as df from 'durable-functions'; +import { OrchestrationContext as DfOrchestrationContext } from 'durable-functions'; +import { OrchestrationContext } from '@microsoft/durabletask-js'; + +const echoActivity = 'ShapeEcho'; + +// Shared echo activity used by the generator shapes to prove they are actually driven (an activity +// round-trip) rather than short-circuited by the wrong context. +df.app.activity(echoActivity, { handler: (input: string): string => `echo:${input}` }); + +// 1) Classic v3 shape: sync generator using `context.df.*`. Must keep working (routed to classic). +df.app.orchestration('ShapeClassicGenerator', function* (context: DfOrchestrationContext) { + const result = (yield context.df.callActivity(echoActivity, 'IN')) as string; + return `classic:${result}`; +}); + +// 2) Core-native async generator, single arg: uses the core context (`ctx.callActivity`) directly. +// Arity is 1 (like classic) but it is NOT classic; it must pass through unchanged and be driven. +df.app.orchestration('ShapeNativeGenerator', async function* (ctx: OrchestrationContext) { + const result = (yield ctx.callActivity(echoActivity, 'IN')) as string; + return `native-gen:${result}`; +}); + +// 3) #321/#322 regression: plain SYNC, single-arg, NON-generator core-native orchestrator. It reads a +// CORE context member (`ctx.instanceId`); before #322 that was `undefined` (garbage output). +df.app.orchestration('ShapeSyncNative', (ctx: OrchestrationContext): string => `sync-native:${ctx.instanceId}`); + +// 4) Plain ASYNC, single-arg, NON-generator core-native orchestrator (never a classic shape). +df.app.orchestration( + 'ShapeAsyncNative', + async (ctx: OrchestrationContext): Promise => `async-native:${ctx.instanceId}`, +);