Azure / azure-functions-durable-js

JavaScript library for using the Durable Functions bindings
https://www.npmjs.com/package/durable-functions
MIT License
131 stars 47 forks source link

Double quotes added to input passed from suborchestrator to activity #436

Closed hossam-nasr closed 1 year ago

hossam-nasr commented 1 year ago

Describe the bug If input is passed from an orchestration -> suborchestration -> activity function, there are extra double quotes added to the input. There seems to be some extra layer of serialization/deserialization happening here.

Investigative information

To Reproduce Steps to reproduce the behavior:

  1. Create a simple orchestration that calls a suborchestration with some input, which in turn calls an activity function with the same input. Here, I am using the SayHelloWithSubOrchestration orchestration example in the samples:

SayHelloWithSubOrchestration/index.js:

const df = require("durable-functions");

module.exports = df.orchestrator(function* (context) {
    const input = context.df.getInput();

    const output = yield context.df.callSubOrchestrator("SayHelloWithActivity", input);
    return output;
});

SayHelloWithActivity/index.js:

const df = require("durable-functions");

module.exports = df.orchestrator(function* (context) {
    const input = context.df.getInput();

    const output = yield context.df.callActivity("Hello", input);
    return output;
});

Hello/index.js:

module.exports = function (context) {
    return `Hello ${context.bindings.name}!`;
};
  1. Call the SayHelloWithSubOrchestrator orchestration with some input. If running locally, make the following HTTP request:
POST http://localhost:7071/api/orchestrators/SayHelloWithSubOrchestrator

"Cairo"

Expected behavior Output of the orchestration should be: "Hello Cairo!"

Actual behavior Output of the orchestration instead is: "Hello \"Cairo\"!" with extra double quotes around the input.

davidmrdavid commented 1 year ago

@hossam-nasr: Good catch. I think it would useful to see what the sub-orchestrator output looks like in the History. I suspect that sub-orchestrator results, in the History, are doubly serialized by default. Perhaps we can make a special case here to doubly de-serialize those outputs.

This would be a breaking change for V2 of the SDK, so I'd include it only in V3.

Finally, we can also investigate why the Extension leads to doubly-serialized results in sub-orchestrators and fix this there...but that's a larger change with a larger impact scope, so I wouldn't prioritize this just yet. Still worth investigating at that level to understand the root cause, and file it in the Extension repo once we understand it.

hossam-nasr commented 1 year ago

We discussed this offline. Since this is a breaking change, let's settle for just fixing this for v3, and let's aim to get the fix in before preview release