Skip to content

refactor: simplify error message formatting in exception handling #7881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wxg0103 merged 1 commit into from
Nov 13, 2026
+109 −308
Merged

refactor: simplify error message formatting in exception handling #7881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/models_provider/base_model_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ def classify_dynamic_model_id(model_id: str):
return ModelTypeConst.EMBEDDING
if "rerank" in model_id:
return ModelTypeConst.RERANKER
if "bge" in model_id:
# 智源 BGE 系列（bge-small/bge-m3/bge-large 等）为向量模型，
# 放在 rerank 判断之后，避免 bge-reranker-* 被误判为向量模型。
return ModelTypeConst.EMBEDDING
if any(k in model_id for k in ("whisper", "asr", "transcribe", "paraformer")):
return ModelTypeConst.STT
if any(k in model_id for k in ("tts", "speech", "voice", "cosyvoice", "sambert")):

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the diff

1. Functional issue / fragile substring match
if "bge" in model_id: uses a plain substring check on the raw string. This is case-sensitive and unanchored, so it will:

  • Miss uppercase or mixed-case IDs like BGE-M3, BGE-Large.
  • False-positive on unrelated IDs that happen to contain the letters bge as part of another token (e.g. ...subgemini..., vendor prefixes, version suffixes). Low probability today, but brittle.

Suggested: normalize and use a clearer token match, e.g.

mid = model_id.lower()
if mid.startswith("bge") or "-bge" in mid:
    return ModelTypeConst.EMBEDDING

2. Ordering dependency is implicit and undocumented in code structure
The comment explains why bge follows rerank, which is good, but the correctness now relies entirely on statement order. If someone later reorders or adds another prefix check (e.g. "m3" → EMBEDDING, or moving embedding checks), bge-reranker-* could regress silently. Consider making the reranker exclusion explicit:

if "bge" in model_id and "rerank" not in model_id:
    return ModelTypeConst.EMBEDDING

This makes intent self-contained and order-independent.

3. Consistency with existing style
Other branches use any(k in model_id for k in (...)). A single-entry tuple is unnecessary; a plain in check is fine. But mixing styles may invite future inconsistencies — acceptable as-is.

4. No test coverage shown
Since this changes classification of a whole model family (bge-small, bge-m3, bge-large, and critically bge-reranker-*), add unit tests asserting at minimum:

  • bge-m3 → EMBEDDING
  • bge-reranker-v2-m3 → RERANKER (regression guard)
  • An unrelated ID containing bge does not get misrouted if relevant.

5. Coverage gap (not caused by this diff)
bge is ForceAI/BAAI naming; other embedding families (e.g. gte, e5, m3e, jina-embed, text-embedding) aren't handled. If the goal is general robustness, consider a shared constant list rather than ad-hoc branches.

No blocking bugs — the change works for the documented cases. Main risks are the case-sensitive/unanchored match and the order-dependent logic.

Expand Down
2 changes: 1 addition & 1 deletion apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
f"Verification failed, please check whether the parameters are correct: {e}",
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change removes a translated, user-friendly message ("Verification failed, please check whether the parameters are correct: {error}") and replaces it with raw exception text str(e).

Issues / irregularities:

  1. Loss of internationalization (i18n). The original used gettext(...) so the message could be localized; the new version drops that entirely. If the app supports multiple languages, this is a regression.

  2. Less user-friendly error output. str(e) may expose internal validation details or produce cryptic messages (e.g., Pydantic-style "value is not a valid integer") instead of a consistent, context-aware message. This hurts UX and can leak implementation details.

  3. Inconsistent error format. Other places likely still wrap errors in a friendly prefix; this creates inconsistent API error responses.

  4. Potential information disclosure. Raw exception strings may reveal field names, library internals, or stack-related info—minor security/UX concern in an API response.

Suggestions:

  • Keep the gettext wrapper for i18n, e.g.: 
    gettext("Verification failed, please check whether the parameters are correct: {error}").format(error=str(e))
  • If you want cleaner logs vs. responses, pass a generic translated message to the client and log str(e) separately at debug level.
  • Ensure str(e) is safe/sanitized if it must be returned.
  • If the intent was to avoid double-translation or verbose messages, consider a dedicated translated fallback like gettext("Invalid parameters") rather than raw exception text.

Minor: No functional bug introduced (exception type and code ValidCode.valid_error.value unchanged), but the behavioral change in message content is not obviously justified by the diff alone.

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/itv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this diff, here are the observations:

Potential Issues

1. Loss of Context / Localization Regression

The biggest concern is losing user-friendly context. The original message:

  • Used gettext() → i18n/localization support
  • Included a descriptive prefix: "Verification failed, please check whether the parameters are correct"
  • Appended the specific error via {error}

The new version returns only str(e), which:

  • Breaks i18n — raw exception strings won't be translated
  • Gives the end user only a bare technical error with no guidance
  • Makes it harder to distinguish "verification failed" errors from other API exceptions in logs/UI (no stable message prefix)

2. Raw Exception Messages Leaking to Clients

str(e) may expose internal details (stack fragments, field names, encoding internals like Pydantic/Django validation messages). This can be an information disclosure risk. The original phrasing at least framed it as a generic user-facing error.

3. Inconsistent Error Contract

Other branches of is_valid presumably raise AppApiException with a standard message. If one path only returns str(e), clients that pattern-match on the message prefix will behave inconsistently.

4. str(e) May Be Empty

Some exceptions (e.g., bare ValueError()) yield "". Then the exception carries an empty reason, making debugging harder than before.

Suggestions

  1. Keep the localized wrapper rather than dropping it:

    gettext("Verification failed, please check whether the parameters are correct: {error}").format(error=str(e))

    If translation is genuinely unnecessary, at minimum keep a stable, documented prefix.

  2. Separate user-facing vs. internal detail: Log the full repr(e) (or traceback), but return a sanitized/masked message to the client.

  3. Guard against empty messages:

    error_detail = str(e) or type(e).__name__

  4. Consider structured errors: pass an error code + safe message instead of free-form strings, so frontend can localize and avoid trusting server text.

  5. If intentional (e.g., migrating away from gettext for error payloads), ensure this change is applied consistently across all AppApiException call sites, not just this one — otherwise you create mixed conventions.

Minor Optimization

  • No performance concern here; the removal itself is cheaper, but correctness/i18n costs outweigh any micro-gain.

Bottom line: This change appears to be a regression in i18n and user experience, and potentially a minor info-disclosure risk. Recommend reverting unless it's part of a coordinated refactor with sanitization elsewhere.

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/reranker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/stt/asr_stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False
return True
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/stt/default_stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False
return True
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/stt/omni_stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False
return True
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/stt/stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/tti.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/tts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aliyun_bai_lian_model_provider/credential/ttv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/anthropic_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/anthropic_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aws_bedrock_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aws_bedrock_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/aws_bedrock_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
3 changes: 1 addition & 2 deletions apps/models_provider/impl/aws_bedrock_model_provider/credential/reranker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: %(error)s")
% {"error": str(e)},
str(e),
)
else:
return False
Expand Down
2 changes: 1 addition & 1 deletion apps/models_provider/impl/azure_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct"),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/azure_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
2 changes: 1 addition & 1 deletion apps/models_provider/impl/azure_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct"),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/azure_model_provider/credential/stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/azure_model_provider/credential/tti.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/azure_model_provider/credential/tts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/deepseek_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/reranker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/tti.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/docker_ai_model_provider/credential/tts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/gemini_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
_("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/gemini_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/gemini_model_provider/credential/itv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
return False

Expand Down
4 changes: 1 addition & 3 deletions apps/models_provider/impl/gemini_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ def is_valid(
if raise_exception:
raise AppApiException(
ValidCode.valid_error.value,
gettext("Verification failed, please check whether the parameters are correct: {error}").format(
error=str(e)
),
str(e),
)
else:
return False
Expand Down
Loading
Loading