Skip to content

refactor(exceptions): MinimaxRequestError is a subclass of MinimaxAPIError, conflating two error categories #89

Description

@TumCucTom

Summary

In minimax_mcp/exceptions.py:

class MinimaxAPIError(Exception):
    """Base exception for Minimax API errors."""
    pass

class MinimaxRequestError(MinimaxAPIError):
    """Request related errors."""
    pass

MinimaxRequestError is a subclass of MinimaxAPIError, but the two represent different error categories:

  • MinimaxAPIError — problems talking to the remote API (auth, network, 5xx, etc.). Worth retrying.
  • MinimaxRequestError — client-side problems (missing required field, bad combination, etc.). Never worth retrying.

Because of the inheritance, every except MinimaxAPIError block in minimax_mcp/server.py also catches MinimaxRequestError. This makes it impossible for callers to handle "transient" vs "permanent" failures differently — and it complicates retry logic, metrics, and error budgets.

Impact

  • All tool functions return the same generic error message format for both transient and permanent failures, even though clients might want to retry one and not the other.
  • The PR test: expand coverage to server.py and __main__.py (#72) #87 test suite had to be written around the inheritance — e.g. with pytest.raises(MinimaxRequestError) works, but a hypothetical try/except MinimaxAPIError: retry() would also catch MinimaxRequestError, which is the wrong behavior.

Suggested fix

Two options, in order of preference:

  1. Make them siblings under MinimaxError (rename the current base to MinimaxError or add a new neutral base). Both MinimaxAPIError and MinimaxRequestError should be siblings, not parent/child.
  2. At minimum, update existing except MinimaxAPIError as e: blocks to except (MinimaxAPIError, MinimaxRequestError) as e: (or except MinimaxError as e: after the refactor) and document the categories in each tool's docstring.

Option 1 is cleaner and a one-file change.

Discovered via

Issue filed as a follow-up to PR #87 (test coverage). Tests pass either way, but the inheritance makes the error-handling story confusing for new contributors.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions