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

.NET out-of-proc error parsing fixes #2763

Merged
merged 3 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- Fix issue with isolated entities: custom deserialization was not working because IServices was not passed along (https://github.com/Azure/azure-functions-durable-extension/pull/2686)
- Fix issue with `string` activity input having extra quotes (https://github.com/Azure/azure-functions-durable-extension/pull/2708)
- Fix issue with out-of-proc entity operation errors: success/failure details of individual operations in a batch was not processed correctly (https://github.com/Azure/azure-functions-durable-extension/pull/2752)
- Fix issues with .NET Isolated out-of-process error parsing (see https://github.com/Azure/azure-functions-durable-extension/issues/2711)

### Breaking Changes

Expand Down
27 changes: 22 additions & 5 deletions src/WebJobs.Extensions.DurableTask/OutOfProcMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -655,31 +655,48 @@ private static bool TrySplitExceptionTypeFromMessage(
[NotNullWhen(true)] out string? exceptionType,
[NotNullWhen(true)] out string? exceptionMessage)
{
// Example exception messages:
// In certain situations, like when the .NET Isolated worker is configured with
// WorkerOptions.EnableUserCodeException = true, the exception message we get from the .NET Isoalted
cgillum marked this conversation as resolved.
Show resolved Hide resolved
// worker looks like this:
// "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;
}
Comment on lines +661 to +669
Copy link
Contributor

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

Copy link
Member Author

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.


// The following are the more common cases that we expect to see, which will be common across a
// variety of language workers:
// .NET : System.ApplicationException: Kah-BOOOOM!!
// Java : SQLServerException: Invalid column name 'status'.
// Python : ResourceNotFoundError: The specified blob does not exist. RequestId:8d5a2c9b-b01e-006f-33df-3f7a2e000000 Time:2022-03-25T00:31:24.2003327Z ErrorCode:BlobNotFound Error:None
// Node : SyntaxError: Unexpected token N in JSON at position 12768

// From the above examples, they all follow the same pattern of {ExceptionType}: {Message}
// From the above examples, they all follow the same pattern of {ExceptionType}: {Message}.
// However, some exception types override the ToString() method and do something custom, in which
// case the message may not be in the expected format. In such cases we won't be able to distinguish
// the exception type.
string delimeter = ": ";
int endExceptionType = exception.IndexOf(delimeter);
if (endExceptionType < 0)
{
exceptionType = null;
exceptionMessage = null;
exceptionMessage = exception;
return false;
}

exceptionType = exception[..endExceptionType];
exceptionType = exception[..endExceptionType].TrimEnd();

// The .NET Isolated language worker strangely includes the stack trace in the exception message.
// To avoid bloating the payload with redundant information, we only consider the first line.
int startMessage = endExceptionType + delimeter.Length;
int endMessage = exception.IndexOf('\n', startMessage);
if (endMessage < 0)
{
exceptionMessage = exception[startMessage..];
exceptionMessage = exception[startMessage..].TrimEnd();
}
else
{
Expand Down
Loading