fix: throw InvalidOperationException when MarkItDownMcpReader encounters IsError. - #7627
fix: throw InvalidOperationException when MarkItDownMcpReader encounters IsError.#7627aaleemsolutions wants to merge 2 commits into
Conversation
|
@dotnet-policy-service agree |
jeffhandley
left a comment
jeffhandley
left a comment
There was a problem hiding this comment.
I’m not sure InvalidOperationException is the right exception type for this failure. @adamsitnik, do you have thoughts on what callers should receive here?
I’m also concerned about including the MCP server’s raw error text in the exception message. It may contain sensitive details such as file paths or parser diagnostics, and exception messages can flow through logs, telemetry, IngestionResult.Exception, or potentially all the way to an end user. Since MCP is an implementation detail of this reader, could we expose a stable, sanitized conversion error instead?
| // FIX: Check if the tool execution returned an error (handles bool?) | ||
| if (result.IsError == true) |
There was a problem hiding this comment.
This code comment shouldn't have "FIX: " in it and the "(handles bool?)" is a bit confusing on first glance. The comment is actually superfluous anyway.
| // FIX: Check if the tool execution returned an error (handles bool?) | |
| if (result.IsError == true) | |
| if (result.IsError == true) |
3 participants
This PR resolves an issue where MarkItDownMcpReader silently ignores tool errors during document conversion.
Related Issues
Closes #7618
Microsoft Reviewers: Open in CodeFlow