`MarkItDownMcpReader` ignores tool IsError (returns error text as content)`
- Dominant language
- C#
- Stars
- 3.2k
- Forks
- 894
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 23
Description
### Description
`MarkItDownMcpReader` (`Microsoft.Extensions.DataIngestion.MarkItDown`)
1. **Failed conversions are silently returned as document content.** When the tool reports a failure, it returns a `CallToolResult` with `IsError = true` and the error message in `Content`. `ConvertToMarkdownAsync` returns the first text block **without checking `IsError`**, so the error string becomes the "markdown" body of the document. Downstream this produces a single bogus chunk and the ingestion is reported as successful.
### Reproduction Steps
Using `Microsoft.Extensions.DataIngestion.MarkItDown` `10.7.0-preview.1.26309.5` against a MarkItDown MCP server (`mcp/markitdown`, `--http`):
```csharp
var reader = new MarkItDownMcpReader(new Uri("http://localhost:3001/mcp"));
// Any input the server cannot convert makes the tool return IsError=true with the error in Content.
// Simplest reproducible case: a path the server cannot resolve.
var doc = await reader.ReadAsync(new FileInfo("does-not-exist-or-unconvertible.docx"), ct);
// No exception is thrown. doc has a single text element whose value is the tool's error string, e.g.:
// "Error executing tool convert_to_markdown: [Errno 2] No such file or directory: '...'"
// (verified directly: CallToolResult.IsError == true, and the message is carried in Content as text)
```
### Expected behavior
When the tool returns `IsError = true`, the reader should throw (or otherwise surface a failure) instead of returning the error text as document content.
### Actual behavior
The error message string is returned as the document body; no exception; the failure is invisible to callers and logs.
Relevant current code (`MarkItDownMcpReader.ConvertToMarkdownAsync`):
```csharp
Dictionary parameters = new() { ["uri"] = dataContent.Uri }; // always a data: URI
var result = await client.CallToolAsync("convert_to_markdown", parameters, cancellationToken);
// no check of result.IsError before returning the first text block
foreach (var content in result.Content)
if (content.Type == "text" && content is TextContentBlock textBlock)
return textBlock.Text;
```
### Regression?
Not a regression — `Microsoft.Extensions.DataIngestion.MarkItDown` is preview (`10.7.0-preview.1.26309.5`); behavior present since the reader was introduced.
### Known Workarounds
Bypass `MarkItDownMcpReader` for the conversion call: use `ModelContextProtocol` directly to invoke `convert_to_markdown`, check `CallToolResult.IsError`, then round-trip the returned Markdown back through `MarkItDownMcpReader`'s stream overload so the internal `MarkdownParser` still builds the `IngestionDocument`. Works, but reimplements the reader's core.
### Configuration
- .NET 9 (SDK 9.0.315), Windows 11 x64.
- `Microsoft.Extensions.DataIngestion.MarkItDown` `10.7.0-preview.1.26309.5`, `Microsoft.Extensions.AI` `10.7.0`, `ModelContextProtocol.Core` `1.2.0`.
- MarkItDown MCP server: `mcp/markitdown` `1.8.1`, `--http` (Streamable HTTP).
- Not configuration-specific.
### Other information
Proposed fixes:
- **`IsError` (pure bug, no API change):** in `ConvertToMarkdownAsync`, if `result.IsError == true`, throw an `InvalidOperationException` containing the returned text.
Contributor guide
Research direction
Start at MarkItDownMcpReader.ConvertToMarkdownAsync and inspect the CallToolAsync result handling. Reproduce the failure with a nonexistent or unconvertible path and verify that a result with IsError=true is surfaced as a failure rather than returned as document content. Done means callers receive the tool failure instead of a bogus markdown document.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100