Skip to content

Error middleware enhancement #770

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MrZoidberg
Copy link
Contributor

This PR is adding a WorkflowInstance parameter to IWorkflowMiddlewareErrorHandler interface so that it's possible to log only exception, but also related workflow information like ID:

_log.LogError(ex, "An error occurred running workflow '{workflow}' middleware: {Message}", workflowInstance.Id, ex.Message);

WARNING: this is a breaking change in terms that any 3rd party implementations of IWorkflowMiddlewareErrorHandler won't work without changes.

/// <param name="ex">The exception to handle</param>
/// <returns>A task that completes when handling is done.</returns>
Task HandleAsync(Exception ex);
Task HandleAsync(WorkflowInstance workflowInstance, Exception ex);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we make this backward compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielgerlag I can introduce something like IWorkflowMiddlewareErrorHandlerV2 that will have an old and a new method. And update the usages. However, I don't like this approach. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants