-
Notifications
You must be signed in to change notification settings - Fork 273
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
.NET out-of-proc error parsing fixes #2763
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'd love to revisit our approach to exception serialization further (not an easy problem by any means!) since I can see there's lots of edge cases, making it tricky to reason about.
// "Exception of type 'ExceptionSerialization.Function+UnknownException' was thrown." | ||
const string startMarker = "Exception of type '"; | ||
const string endMarker = "' was thrown."; | ||
if (exception.StartsWith(startMarker) && exception.EndsWith(endMarker)) | ||
{ | ||
exceptionType = exception[startMarker.Length..^endMarker.Length]; | ||
exceptionMessage = string.Empty; | ||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not for this PR, but I'd love to revisit this approach - something about string manipulations here makes me unsure about the robustness of our approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm uncomfortable with this too. I sent @mattchenderson a ping on Teams earlier to see if the Functions Host team might consider giving us a proper exception contract between the host and the worker in the future, which would eliminate the need for this kind of fragile string parsing.
Co-authored-by: David Justo <[email protected]>
Resolves #2711
There were a few edge cases that weren't being handled correctly when dealing with activity exceptions in .NET out-of-proc. The edge cases included:
Exception.ToString()
and change the exception message formattingWorkerOptions.EnableUserCodeException
In the above cases, what we receive from the .NET Isolated worker changes from what we expect, resulting in exception information missing when customers try to handle exceptions in their orchestrator function code. The changes in this PR are meant to cover these corner cases and provide users with more accurate exception information.
Pull request checklist
pending_docs.md
release_notes.md
/src/Worker.Extensions.DurableTask/AssemblyInfo.cs