MiniMax-AI / MiniMax-AI/MiniMax-MCP
refactor(exceptions): MinimaxRequestError is a subclass of MinimaxAPIError, conflating two error categories
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 hypotheticaltry/except MinimaxAPIError: retry()would also catchMinimaxRequestError, which is the wrong behavior.
Suggested fix
Two options, in order of preference:
- Make them siblings under
MinimaxError(rename the current base toMinimaxErroror add a new neutral base). BothMinimaxAPIErrorandMinimaxRequestErrorshould be siblings, not parent/child. - At minimum, update existing
except MinimaxAPIError as e:blocks toexcept (MinimaxAPIError, MinimaxRequestError) as e:(orexcept 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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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