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

Aperta Adatta ai principianti
#89 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
68/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Tranquilla
Stack tecnologico
python
Ambito
api, backend

Direzione di ricerca

Inizia da minimax_mcp/exceptions.py per esaminare l’attuale gerarchia delle eccezioni, quindi rivedi gli handler di MinimaxAPIError in minimax_mcp/server.py. Scegli l’approccio con base neutra descritto nell’issue, preserva le categorie distinte API e request e verifica che i test esistenti di PR #87 continuino a passare con il comportamento delle eccezioni previsto.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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

Lingua principale
Python
Stelle
1.6k
Fork
284
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di MiniMax-AI/MiniMax-MCP

Tutte le issue di MiniMax-AI/MiniMax-MCP

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.