Skip to content

Throw an error when llm client fails with image converter. - #2476

Merged
afourney merged 1 commit into
mainfrom
llm_image_err
Sep 12, 2026
Merged

Throw an error when llm client fails with image converter.#2476
afourney merged 1 commit into
mainfrom
llm_image_err

Conversation

@afourney

Copy link
Copy Markdown
Member

This reverses #1948 -- an overly-broad try/catch was the wrong solution here.

In this PR, the README is updated to explain how to adjust the automatic retry behavior of the OpenAI client, and how MarkItDown handles errors in cases where all retries fail.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused implementation matches the documented behavior and has comprehensive regression coverage.

Pull request overview

Removes overly broad image-caption error suppression so MarkItDown can attempt fallback converters or report conversion failure.

Changes:

  • Propagates LLM client errors from ImageConverter.
  • Documents OpenAI retry configuration and failure behavior.
  • Tests propagation, fallback conversion, and unchanged success paths.
File summaries
File Description
README.md Documents retries and exhausted-error handling.
packages/markitdown/src/markitdown/converters/_image_converter.py Stops suppressing LLM client exceptions.
packages/markitdown/tests/test_image_converter.py Covers failure, fallback, success, and no-LLM behavior.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@afourney
afourney merged commit 3a6ce44 into main Sep 12, 2026
24 checks passed
@afourney
afourney deleted the llm_image_err branch September 12, 2026 19:11
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