Skip to content

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

@TumCucTom

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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions