Skip to content

[durable-functions v4] isClassicOrchestrator can't distinguish a plain sync single-arg core-native orchestrator from a classic one #321

Description

@YunchuWang

Summary

isClassicOrchestrator (in packages/azure-functions-durable/src/orchestration-context.ts) cannot reliably classify a plain, synchronous, single-argument, non-generator function. For that one shape it falls back to arity (handler.length <= 1 → classic), which misroutes a core-native (ctx) => ctx.getInput() into the classic wrapper. The handler then receives the classic { df, log, ... } context instead of the core OrchestrationContext, so any call to a core method (ctx.getInput(), ctx.callActivity(...), ctx.newGuid(), …) throws TypeError: ctx.<method> is not a function.

Raised by the automated reviewer on PR #282 (orchestration-context.ts:279).

Why this is genuinely ambiguous (not a quick fix)

Detection is by function kind, which is correct for every shape except plain sync non-generators:

Handler shape constructor.name Correct routing Detected correctly?
async function*(ctx, input) AsyncGeneratorFunction core-native ✅
async (ctx, input) => … AsyncFunction core-native ✅
function*(context) GeneratorFunction classic v3 ✅
(ctx) => value (plain sync, arity 1) Function ambiguous ❌ arity fallback

The ambiguity is real because a plain sync single-arg function is the identical shape for two opposite intents:

  • classic: (context) => context.df.getInput() — wants the classic { df, log } context, and
  • core-native: (ctx) => ctx.getInput() — wants the core OrchestrationContext.

There is currently an intentional unit test asserting the classic reading of this shape — packages/azure-functions-durable/test/unit/orchestration-context.spec.ts ("exposes a replay-safe logger as context.log/error on the classic context") registers (context) => "logged" and asserts it is wrapped with the classic context. So flipping the arity default to core-native is not a safe one-line change — it reverses documented behavior and breaks that test.

v3 ground truth (rules out the "classic non-generator" reading)

Verified against Azure/azure-functions-durable-js v3.x branch (src/orchestrations/TaskOrchestrationExecutor.ts):

  • execute(...) types the handler as fn: (context: OrchestrationContext) => IterableIterator<unknown>.
  • It calls this.generator = fn(context) as Generator<TaskBase, any, any> and drives it via this.generator.next(...).

A classic v3 orchestrator is therefore always a single-argument sync generator (function*(context)); a plain non-generator function was never a valid v3 orchestrator (its return value has no .next() to drive). This means the classic interpretation of the ambiguous shape is a compat-invented convenience, not a v3 requirement — but the compat has already committed to it via the test above, so both readings now have a stake.

Severity: low

Only a trivial, purely synchronous, single-arg core-native orchestrator is affected — one that does no await/yield (so it schedules no durable work) and reads a synchronous core method, e.g. (ctx) => ctx.getInput() or (ctx) => ctx.newGuid(). Every realistic orchestrator is classified correctly:

  • anything async → core-native ✅
  • any (ctx, input) (arity 2) → core-native ✅
  • any generator (real durable work) → classic ✅

Options

  1. Flip the arity default (return false at the fallback): one line, but reverses the documented classic reading and breaks the existing classic-non-generator test. Would need that test removed/repurposed and the wrapOrchestrator docs updated. Picks the core-native victim over the classic one.
  2. Dual context for the ambiguous case: route plain sync single-arg handlers through a context that satisfies both surfaces (core OrchestrationContext methods and { df, log, … }). Satisfies both intents, but adds Proxy/this-binding complexity and needs a replay-safety review (a "classic" body could reach raw core methods).
  3. Explicit tagging at registration (recommended long-term): stop guessing — have the classic registration entrypoint mark handlers as classic (e.g. a hidden flag or a dedicated wrapper), so classification is a fact, not a heuristic. Larger change (threads a flag through the registration API) but removes the ambiguity permanently.

Recommendation

Track here rather than expand PR #282 (scope: core helpers + CI + release pipeline). Preferred direction is (3) for correctness, with (1) as an acceptable interim if the project decides core-native ergonomics outweigh the classic-non-generator convenience. The corresponding PR #282 review thread is being resolved with a pointer to this issue.

Refs: PR #282; branch yunchuwang/functions-grpc-support @ e3bd426.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions