MiniMax-AI / MiniMax-AI/MiniMax-MCP

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

Open Beginner friendly
#89 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1.6k
Forks
284
PR merge metrics
No merged PRs in 30d

Description

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 #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

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in minimax_mcp/exceptions.py to inspect the current exception hierarchy, then review the MinimaxAPIError handlers in minimax_mcp/server.py. Choose the neutral-base approach described in the issue, preserving the distinct API and request categories, and verify that the existing PR #87 tests still pass with the intended exception behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.