Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for extensions to wrap function invocation delegate #1666

Open
jviau opened this issue Jun 22, 2023 · 4 comments
Open

Allow for extensions to wrap function invocation delegate #1666

jviau opened this issue Jun 22, 2023 · 4 comments
Labels
enhancement New feature or request extensions: durable-functions Items related to Durable Functions support needs-discussion team-issue

Comments

@jviau
Copy link
Contributor

jviau commented Jun 22, 2023

Background

Durable functions for dotnet isolated has a need for wrapping all [OrchestrationTrigger] function invocations with additional logic. Today we perform this wrapping via middleware. However, it was discovered this is extremely fragile for durables case because we need to tightly control the precise async behavior in our logic. This has led to non-determinism issues in durable, which arise from the customer introducing any custom middleware between the Durable middleware and the function invocation. This makes orchestrations fragile and is a less than ideal experience for customers to put it lightly.

Proposed Solution

Provide an API where an extension can wrap a function invocation's delegate with a custom delegate. This would most make sense to be doable as part of the middleware. So durable could retain its middleware, but instead of wrapping the rest of the pipeline it performs some action on the FunctionContext (maybe a new IInvocationFeatures from .Features)?

Alternative Solutions

Controlling middleware order

If durable could control its middleware order, it could set its middleware to be the last. However, this is fragile as any other extension could come along and also declare its middleware to be last, and then we are back to square one with this issue.

Replacing function definition invocation

This would rely on introducing an extensibility model to function definition metadata generation and is most likely overkill for this scenario.

@horatiu-stoianovici
Copy link

Incompatibility with Azure App Configuration Refresh

Adding a bit more to this issue, working on an internal project in Microsoft, we are using Azure App Configuration for our application and are trying to migrate the durable function projects to the isolated model.

What I found is that it looks like you cannot use the documented way of achieving dynamic config by refreshing your configuration values for Azure App Configuration, since that injects its own middleware to refresh config behind the scenes.

This is a very tough issue to identify as I was getting very weird unexplained behavior when an eternal orchestration just freezes indefinitely when doing context.ContinueAsNew(). Eventually I somehow also got the The orchestrator function completed on a non - orchestrator thread! error, which lead me all the way here. Removing the App Configuration middleware got rid of all the weird behavior.

Are you aware of the issue and any potential workarounds here for Azure App Configuration users?

@jviau
Copy link
Contributor Author

jviau commented Dec 4, 2023

@horatiu-stoianovici you can add your own middleware and manually use the app configs refresher and refresh only when it is not an orchestration trigger.

@max-huenink
Copy link

max-huenink commented Feb 14, 2024

I have a potential alternative solution that feels more like a band-aid than a permanent solution, but would solve this problem for anyone using custom middleware. I'm open to feedback on this approach.

My solution is to change the UseMiddleware and UseWhen extension methods from WorkerMiddlewareWorkerApplicationBuilderExtensions.c to only invoke middleware when the function context doesn't have an orchestration trigger, using the IsOrchestration method mentioned above. However, this would break the existing durable functions middleware added here DurableTaskExtensionStartup.cs. I've thought of two solutions to this new problem, but I don't know which is a better approach.

  1. Add a new method UseDurableMiddleware that doesn't have the check mentioned above. Then DurableTaskExtensionStartup.cs would use this new method. Existing consumers wouldn't need to change.
  2. Add a parameter to the existing UseMiddleware and UseWhen functions, bool useInOrchestrations = false. Then DurableTaskExtensionStartup.cs would specify true for this parameter. Existing consumers wouldn't need to change.

@LockTar
Copy link

LockTar commented Jul 12, 2024

@horatiu-stoianovici you can add your own middleware and manually use the app configs refresher and refresh only when it is not an orchestration trigger.

@jviau I'm using an injected EF DbContext in my middleware. I need to read and write some data from and to a database for an Activity and an (sub)Orchestration.
I did a .Wait() on my call to fix the issue. Not ideal but I thought it worked. Now, I'm trying to do a fan out by creating multiple sub orchestrations and I get this DbContext threading issue. Any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request extensions: durable-functions Items related to Durable Functions support needs-discussion team-issue
Projects
None yet
Development

No branches or pull requests

7 participants