Skip to content

[durable-functions v4] isClassicOrchestrator misclassifies core-native sync single-arg orchestrators #319

Description

@YunchuWang

Follow-up from PR #282 (automated-reviewer thread, P2/robustness)

In the new durable-functions v4 provider (packages/azure-functions-durable), isClassicOrchestrator (src/orchestration-context.ts) decides whether a registered handler is a classic v3 orchestrator (must be wrapOrchestrator-wrapped and handed a { df, log } classic context) or a core-native one (passed straight through and handed the core OrchestrationContext).

Current logic:

const kind = handler.constructor?.name;
if (kind === "AsyncGeneratorFunction" || kind === "AsyncFunction") return false; // core-native
if (kind === "GeneratorFunction") return true;                                   // classic v3
return handler.length <= 1;  // plain SYNC non-generator: fall back to arity

The gap

A core-native sync single-arg orchestrator — e.g. (ctx) => ctx.instanceId — has constructor.name === "Function", so it reaches the arity fallback, handler.length <= 1 is true, and it is classified as classic. It then gets wrapped and receives a { df, log } object instead of the core OrchestrationContext, breaking at runtime.

This is a real case because core supports non-generator orchestrators. The executor (packages/durabletask-js/src/worker/orchestration-executor.ts:286-298) does:

const result = await fn(ctx, input);
const isAsyncGenerator = typeof result?.[Symbol.asyncIterator] === "function";
if (isAsyncGenerator) { await ctx.run(result); }
else { ctx.setComplete(result, ORCHESTRATION_STATUS_COMPLETED); } // plain value -> complete

So (ctx) => value and async (ctx) => value are both valid core-native orchestrators. The classifier already treats the async form as core-native (via AsyncFunction), but the sync form falls through to arity and is mis-flagged.

Why it is not a one-line fix (needs a policy decision)

A plain sync, non-generator, single-arg function is genuinely ambiguous:

  • classic degenerate: (context) => context.df.getInput() — a classic orchestrator that never yields (unusual, but the wrapped { df } context makes it "work").
  • core-native: (ctx) => ctx.instanceId — expects the core context.

Classic v3 orchestrators that do any durable work are always generators (they must yield), so they are already caught by the GeneratorFunction branch. That argues the sync fallback should default to core-native (return false). But that would break the degenerate-classic case above. The tie-break is a policy call.

Options

  1. Default sync non-generator single-arg to core-native (return false), documenting that classic orchestrators must be generators. (Simplest; breaks only the degenerate no-yield classic form.)
  2. Require an explicit registration signal (e.g. app.orchestration classic vs a core-native registration path) instead of inferring from the handler shape.
  3. Keep current behavior and document that core-native orchestrators must be async/generator (never plain sync) when registered through the compat provider.

Acceptance

  • Decide the tie-break policy (1/2/3) for ambiguous sync single-arg handlers.
  • Add unit coverage for: async (ctx)=>, (ctx, input)=>, function*(ctx), async function*(ctx), and the ambiguous (ctx)=> case.

Links

Follow-up to an automated-reviewer thread on PR #282.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions