Skip to content

Fix InvalidCastException for history event type mismatch#1018

Merged
cgillum merged 1 commit intomainfrom
mssql-198
Dec 16, 2023
Merged

Fix InvalidCastException for history event type mismatch#1018
cgillum merged 1 commit intomainfrom
mssql-198

Conversation

@cgillum
Copy link
Member

@cgillum cgillum commented Dec 16, 2023

While investigating microsoft/durabletask-mssql#198, I noticed that some of the new distributed tracing code casts history events based on certain assumptions, which aren't always true in production. The issue can result in a poison message with the following error:

System.InvalidCastException: Unable to cast object of type 'DurableTask.Core.History.TaskCompletedEvent' to type 'DurableTask.Core.History.TaskScheduledEvent'.
at DurableTask.Core.TaskOrchestrationDispatcher.ReconcileMessagesWithState(TaskOrchestrationWorkItem workItem, String dispatcher, ErrorPropagationMode errorPropagationMode, LogHelper logHelper) in //src/DurableTask.Core/TaskOrchestrationDispatcher.cs:line 860
at DurableTask.Core.TaskOrchestrationDispatcher.OnProcessWorkItemAsync(TaskOrchestrationWorkItem workItem) in //src/DurableTask.Core/TaskOrchestrationDispatcher.cs:line 345
at DurableTask.Core.TaskOrchestrationDispatcher.OnProcessWorkItemAsync(TaskOrchestrationWorkItem workItem)
at DurableTask.Core.TaskOrchestrationDispatcher.OnProcessWorkItemSessionAsync(TaskOrchestrationWorkItem workItem) in //src/DurableTask.Core/TaskOrchestrationDispatcher.cs:line 211
at DurableTask.Core.WorkItemDispatcher`1.ProcessWorkItemAsync(WorkItemDispatcherContext context, Object workItemObj) in //src/DurableTask.Core/WorkItemDispatcher.cs:line 373

This PR replaces the cast operation with a type-safe check, removing the possibility of InvalidCastException.

Note that the root cause of the bug observed by the user is probably specific to the MSSQL backend: it might not be deserializing history events correctly, resulting in unexpected Event ID matches. Regardless, I felt it made sense to make a fix here for defense in depth.

Copy link
Collaborator

@bachuv bachuv left a comment

Choose a reason for hiding this comment

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

Thanks for making this change!

@cgillum cgillum merged commit c6ab576 into main Dec 16, 2023
@cgillum cgillum deleted the mssql-198 branch December 16, 2023 01:30
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