Activity return value is passed back as null if it's an object that shares a key with the name of some output binding

[hossam-nasr]

Describe the bug If the return value of an Activity function is an object with at least one key that is the same as the name property of an output binding in the function.json file, the orchestration will always receive null as the result of the activity.

This is due to the way currently the node @azure/functions framework allows durable activities to pass results back to the orchestration, in this line here. This line is in place because Activity functions do not set an explicit output binding, but their output should still be set as the returnValue of the rpc invocation response.

In the check above, response.outputData.length == 0 will be false if the return value of the activity function is an object with at least one key that has the same name as the name property of one output binding, owing to this line here. The upshot of it is the framework considers this to be an object of output bindings rather than the output value of the function itself. This is a consequence of the many ways that the current model supports for returning output bindings.

Ideally, here's what I would propose:

• context.bindings.bindingName = value should be the only supported way to set output bindings in Durable Activities
• The return value of the Activity function should always be passed back as-is to the orchestrator

I understand that this may not be possible to implement because it could be considered a breaking change, or it could also not be worth our time investment to fix. In that case, I would propose that at the very least we should document this limitation somewhere, or log a warning message if we notice a durable activity doing this.

Investigative information

• Durable Functions extension version: 2.9.0
• durable-functions npm module version: 2.1.1
• Language (JavaScript/TypeScript) and version: TypeScript
• Node.js version: 16.13.0

To Reproduce Steps to reproduce the behavior:

1. Generate a simple hello sequence durable functions app:

DurableFunctionsOrchestratorJS/index.ts:

import * as df from "durable-functions";

const orchestrator = df.orchestrator(function* (context) {
  const outputs = [];

  // Replace "Hello" with the name of your Durable Activity Function.
  outputs.push(yield context.df.callActivity("Hello", "Tokyo"));
  outputs.push(yield context.df.callActivity("Hello", "Seattle"));
  outputs.push(yield context.df.callActivity("Hello", "Cairo"));

  return outputs;
});

export default orchestrator;

Hello/index.ts:

import { AzureFunction, Context } from "@azure/functions";

const activityFunction: AzureFunction = async function (
  context: Context
): Promise<Object> {
  const msg = `Hello ${context.bindings.name}!`;
  // output binding value should be set here
  context.bindings.blobOutput = msg;
  // this object as-is should be returned to the orchestration
  return {
    blobOutput: msg,
  };
};

export default activityFunction;

Hello/function.json:

{
  "bindings": [
    {
      "name": "name",
      "type": "activityTrigger",
      "direction": "in"
    },
    {
      "name": "blobOutput",
      "type": "blob",
      "direction": "out",
      "path": "somePath/someFile.txt",
      "connection": "someConnString"
    }
  ],
  "scriptFile": "../dist/Hello/index.js"
}

Note here that the name property of the blob output binding (blobOuput) matches the name of the key in the return value of the activity.

2. Try calling your orchestrator.

Expected behavior

The output of the orchestration should be:

[{ "blobOutput" : "Hello Tokyo!" }, { "blobOutput" : "Hello Seattle!" }, { "blobOutput" : "Hello Cairo!" }]

Actual behavior

The output of the orchestration is instead:

[null, null, null]

Known workarounds

1. Change the name of the output binding
2. Change the property name in the return value of the activity to not be the same as the name property of any output binding.

[hossam-nasr]

FYI @davidmrdavid @ejizba

[davidmrdavid]

Thanks @hossam-nasr. I do believe this is probably a breaking change, so I lean towards leaving it as is, and documenting it. I would actually suggest going a step further and adding logic to detect when this edge case is triggered, and warning the user about it.

[hossam-nasr]

Discussed offline. Let's not change the current behavior since it's a very narrow edge case (and a breaking change), but let's document it and throw a warning when we detect it