Repository: Azure/azure-functions-durable-js
Pull request #114 (2019-10-25 21:33:34)

Rework generator result handling logic
Fixes continueAsNew() for 1.8.0 and later, as well as handles some edge
cases, like customers yielding non-Task objects and returning unyielded
Tasks. Addresses #57 and #81.


Patch:
diff --git a/src/orchestrator.ts b/src/orchestrator.ts
index 1c7948a..1317372 100644
--- a/src/orchestrator.ts
+++ b/src/orchestrator.ts
@@ -86,6 +86,29 @@ export class Orchestrator {
             let g = gen.next(undefined);
 
             while (true) {
+
+                if (!(g.value instanceof Task || g.value instanceof TaskSet)) {
+                    if (!g.done) {
+                        // The orchestrator must have yielded a non-Task related type,
+                        // so just return execution flow with what they yielded back.
+                        g = gen.next(g.value);
+                        continue;
+                    } else {
+                        log("Iterator is done");
+                        // The customer returned an absolute type.
+                        context.done(
+                            null,
+                            new OrchestratorState({
+                                isDone: true,
+                                output: g.value,
+                                actions,
+                                customStatus: this.customStatus,
+                            }),
+                        );
+                        return;
+                    }
+                }
+
                 partialResult = g.value as Task | TaskSet;
                 if (partialResult instanceof Task && partialResult.action) {
                     actions.push([ partialResult.action ]);
@@ -93,7 +116,21 @@ export class Orchestrator {
                     actions.push(partialResult.actions);
                 }
 
-                if (this.shouldFinish(partialResult)) {
+                // Return continue as new events as completed, as the execution itself is now completed.
+                if (partialResult instanceof Task && partialResult.action instanceof ContinueAsNewAction) {
+                    context.done(
+                        null,
+                        new OrchestratorState({
+                            isDone: true,
+                            output: undefined,
+                            actions,
+                            customStatus: this.customStatus,
+                        }),
+                    );
+                    return;
+                }
+
+                if (!partialResult.isCompleted) {
                     context.done(
                         null,
                         new OrchestratorState({
@@ -106,7 +143,7 @@ export class Orchestrator {
                     return;
                 }
 
-                if ((partialResult instanceof Task || partialResult instanceof TaskSet) && partialResult.isFaulted) {
+                if (partialResult.isFaulted) {
                     if (!gen.throw) {
                         throw new Error("Cannot properly throw the execption returned by customer code");
                     }
@@ -114,13 +151,16 @@ export class Orchestrator {
                     continue;
                 }
 
+                // Handles the case where an orchestration completes with a return value of a
+                // completed (non-faulted) task. This shouldn't generally happen as hopefully the customer
+                // would yield the task before returning out of the generator function.
                 if (g.done) {
                     log("Iterator is done");
                     context.done(null,
                         new OrchestratorState({
                             isDone: true,
                             actions,
-                            output: g.value,
+                            output: partialResult.result,
                             customStatus: this.customStatus,
                         }),
                     );
@@ -736,9 +776,4 @@ export class Orchestrator {
             if (val) { val.IsProcessed = true; }
         });
     }
-
-    private shouldFinish(result: unknown): boolean {
-        return result && Object.prototype.hasOwnProperty.call(result, "isCompleted") && !(result as Task).isCompleted
-            || result instanceof Task && result.action instanceof ContinueAsNewAction;
-    }
 }


Test patch:
diff --git a/test/integration/orchestrator-spec.ts b/test/integration/orchestrator-spec.ts
index f506a91..9aa2b2f 100644
--- a/test/integration/orchestrator-spec.ts
+++ b/test/integration/orchestrator-spec.ts
@@ -40,6 +40,29 @@ describe("Orchestrator", () => {
         );
     });
 
+    it("handles a simple orchestration function (no activity functions), with yield of non-Task object", async () => {
+        const orchestrator = TestOrchestrations.SayHelloInlineInproperYield;
+        const name = "World";
+        const mockContext = new MockContext({
+            context: new DurableOrchestrationBindingInfo(
+                TestHistories.GetOrchestratorStart(
+                    "SayHelloInlineInproperYield",
+                    moment.utc().toDate(),
+                    name),
+                name,
+            ),
+        });
+        orchestrator(mockContext);
+
+        expect(mockContext.doneValue).to.be.deep.equal(
+            new OrchestratorState({
+                isDone: true,
+                actions: [],
+                output: `Hello, ${name}!`,
+            }),
+        );
+    });
+
     falsyValues.forEach((falsyValue) => {
         it(`handles an orchestration function that returns ${falsyValue === "" ? "empty string" : falsyValue}`, async () => {
             const orchestrator = TestOrchestrations.PassThrough;
@@ -388,6 +411,33 @@ describe("Orchestrator", () => {
             );
         });
 
+        it("handles a completed activity function by returning instead of yielding", async () => {
+            const orchestrator = TestOrchestrations.SayHelloWithActivityDirectReturn;
+            const name = "World";
+            const mockContext = new MockContext({
+                context: new DurableOrchestrationBindingInfo(
+                    TestHistories.GetSayHelloWithActivityReplayOne(
+                        "SayHelloWithActivityDirectReturn",
+                        moment.utc().toDate(),
+                        name),
+                    name,
+                ),
+            });
+
+            orchestrator(mockContext);
+
+            expect(mockContext.doneValue).to.be.deep.equal(
+                new OrchestratorState({
+                    isDone: true,
+                    actions:
+                    [
+                        [ new CallActivityAction("Hello", name) ],
+                    ],
+                    output: `Hello, ${name}!`,
+                }),
+            );
+        });
+
         it("handles a completed series of activity functions", async () => {
             const orchestrator = TestOrchestrations.SayHelloSequence;
             const mockContext = new MockContext({
@@ -1241,7 +1291,8 @@ describe("Orchestrator", () => {
 
             expect(mockContext.doneValue).to.be.deep.equal(
                 new OrchestratorState({
-                    isDone: false,
+                    // Is Done needs to be marked as true for 1.8.0 and later to properly process continueAsNew
+                    isDone: true,
                     output: undefined,
                     actions:
                     [
diff --git a/test/testobjects/TestOrchestrations.ts b/test/testobjects/TestOrchestrations.ts
index 2b9dcea..d3cbffc 100644
--- a/test/testobjects/TestOrchestrations.ts
+++ b/test/testobjects/TestOrchestrations.ts
@@ -93,12 +93,22 @@ export class TestOrchestrations {
         return `Hello, ${input}!`;
     });
 
+    public static SayHelloInlineInproperYield: any = df.orchestrator(function*(context: any) {
+        const input = yield context.df.getInput();
+        return `Hello, ${input}!`;
+    });
+
     public static SayHelloWithActivity: any = df.orchestrator(function*(context: any) {
         const input = context.df.getInput();
         const output = yield context.df.callActivity("Hello", input);
         return output;
     });
 
+    public static SayHelloWithActivityDirectReturn: any = df.orchestrator(function*(context: any) {
+        const input = context.df.getInput();
+        return context.df.callActivity("Hello", input);
+    });
+
     public static SayHelloWithActivityRetry: any = df.orchestrator(function*(context: any) {
         const input = context.df.getInput();
         const retryOptions = new df.RetryOptions(10000, 2);