Skip to content

Comments

fix: 增强错误解析以支持中转服务的嵌套错误结构#212

Merged
ding113 merged 2 commits intoding113:devfrom
Silentely:fix/cubence-error-parsing
Nov 26, 2025
Merged

fix: 增强错误解析以支持中转服务的嵌套错误结构#212
ding113 merged 2 commits intoding113:devfrom
Silentely:fix/cubence-error-parsing

Conversation

@Silentely
Copy link
Contributor

@Silentely Silentely commented Nov 26, 2025

Summary

Enhance error message extraction to support nested error structures from relay/proxy services.

摘要

  • 优先提取 upstream_error 中的错误信息
  • 解决 cubence 等中转服务的错误显示问题
  • 修复日志显示"you appear to be making a request using a non-Claude code cli client, which is prohibited",但前端未正确提取该错误,仅提取第一层error.message
  • 保持向后兼容的错误提取逻辑

Problem

When using relay services (like cubence), error responses have a nested structure where the actual error message is wrapped inside upstream_error. The previous implementation only extracted the outer error message, missing the real error cause.

Example nested error structure:

{
  "error": {
    "message": "请求目标分享错误",
    "type": "bad_share",
    "upstream_error": {
      "error": {
        "message": "you appear to be making a request using a non-Claude code cli client, which is prohibited"
      }
    }
  }
}

The frontend was displaying: "请求目标分享错误" instead of the actual error: "you appear to be making a request using a non-Claude code cli client, which is prohibited"

Solution

Enhanced extractErrorMessage() function in src/actions/providers.ts to:

  1. First check for upstream_error.error.message (deeply nested)
  2. Then check for upstream_error.message (shallow nested)
  3. Fall back to original error extraction logic if no upstream error found

This maintains backward compatibility while properly surfacing the root cause of errors from relay services.

Changes

  • src/actions/providers.ts: Added upstream_error extraction logic before the existing candidates array processing

Testing

  • Unit tests added/updated
  • Manual testing performed
  • No breaking changes (backward compatible)

- 优先提取 upstream_error 中的错误信息
- 解决 cubence 等中转服务的错误显示问题
- 修复前端显示"you appear to be making a request using a non-Claude code cli client, which is prohibited"
- 保持向后兼容的错误提取逻辑

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Silentely, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

此拉取请求旨在改进系统的错误解析机制,使其能够更准确地识别和显示来自中转服务的复杂嵌套错误。通过优先处理 upstream_error 字段中的详细错误信息,解决了之前前端仅显示通用错误而忽略深层具体错误的问题,从而提升了用户体验和问题诊断的效率。

Highlights

  • 增强错误解析: 优先从 upstream_error 字段中提取错误信息,以正确处理来自中转服务的嵌套错误结构,确保前端能够显示最具体的错误原因。
  • 解决中转服务错误显示问题: 修复了 Cubence 等中转服务返回的错误信息,此前前端可能只显示通用错误(如“请求目标分享错误”),而忽略了更深层的具体错误(如“you appear to be making a request using a non-Claude code cli client, which is prohibited”)。
  • 保持向后兼容性: 在引入新的错误解析逻辑的同时,确保了原有的错误提取机制仍然有效,以兼容不同格式的错误响应。
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@ding113 ding113 added size/XS Extra Small PR (< 50 lines) bug Something isn't working labels Nov 26, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

本次 PR 旨在增强错误解析逻辑,以支持中转服务返回的嵌套错误结构。您通过优先提取 upstream_error 中的信息,解决了 cubence 等服务的错误显示问题,并保持了对旧有错误格式的兼容性。代码改动集中在 extractErrorMessage 函数中,逻辑清晰。我提出了一项关于代码可读性和可维护性的建议,通过减少代码嵌套层级来简化错误提取逻辑。总体而言,这是一次很有价值的改进。

@ding113
Copy link
Owner

ding113 commented Nov 26, 2025

🔒 Security Scan Results

No security vulnerabilities detected

This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes.

PR Analysis Summary

Changes reviewed: src/actions/providers.ts - Enhancement to extractErrorMessage function to extract nested error messages from upstream relay services (upstream_error field).

Scanned Categories

  • ✅ Injection attacks (SQL, NoSQL, Command, LDAP, etc.)
  • ✅ Authentication and session management
  • ✅ Sensitive data exposure
  • ✅ Access control and authorization
  • ✅ Security misconfiguration
  • ✅ Cross-site scripting (XSS)
  • ✅ Insecure deserialization
  • ✅ SSRF and path traversal
  • ✅ Cryptographic weaknesses

Security Context Notes

The modified function is used exclusively in admin-only endpoints (protected by session role check at line 1628). The changes:

  1. Add a local obj variable for type casting consistency
  2. Extend error extraction to handle nested upstream_error structures from relay services
  3. Maintain existing error extraction logic for backwards compatibility

The code follows existing security patterns in the codebase:

  • Error text is sanitized via sanitizeErrorTextForLogging() before logging
  • Admin authentication is required for all provider test operations
  • Input comes from upstream API providers that admins themselves configure

🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage

Copy link
Owner

@ding113 ding113 left a comment

Choose a reason for hiding this comment

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

📋 Code Review Summary

This PR enhances error message extraction to handle nested error structures from relay/proxy services like cubence. The implementation is straightforward and maintains backward compatibility with the existing error extraction logic.

🔍 Issues Found

  • Critical (🔴): 0 issues
  • High (🟠): 0 issues
  • Medium (🟡): 0 issues
  • Low (🟢): 0 issues

🎯 Priority Actions

No blocking issues identified. The PR is ready for merge.

💡 General Observations

  1. Code Quality: The new code follows the existing patterns in extractErrorMessage, using proper type guards before type assertions.

  2. Scope Handling: The two upstreamMessage variable declarations are in different scopes (nested vs parent block), which is syntactically correct.

  3. Backward Compatibility: The new upstream_error extraction is checked first, but falls back gracefully to the original candidate-based extraction if no upstream error is found.

  4. Minor Optimization: The PR also refactors the candidate getter calls to use the pre-cast obj variable instead of re-casting errorJson on each iteration.


🤖 Automated review by Claude AI - focused on identifying issues for improvement

- 减少嵌套层级,提高代码可读性
- 使用可选链操作符简化属性访问
- 保持相同的功能逻辑

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Owner

@ding113 ding113 left a comment

Choose a reason for hiding this comment

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

📋 Code Review Summary

This PR enhances error message extraction to properly surface nested error messages from relay/proxy services like Cubence. The implementation is straightforward and maintains backward compatibility.

🔍 Issues Found

  • Critical (🔴): 0 issues
  • High (🟠): 0 issues
  • Medium (🟡): 0 issues
  • Low (🟢): 0 issues

🎯 Priority Actions

No blocking issues identified. The PR is ready for merge.

💡 General Observations

  • The code correctly prioritizes upstream_error.error.message over upstream_error.message, which aligns with the nested structure shown in the PR description
  • The refactor to use a single const obj variable instead of repeated errorJson as Record<string, unknown> casts is a nice cleanup
  • Type safety is maintained through appropriate null checks and optional chaining
  • The existing normalizeErrorValue function is properly reused for consistent string handling

🤖 Automated review by Claude AI - focused on identifying issues for improvement

@ding113
Copy link
Owner

ding113 commented Nov 26, 2025

🔒 Security Scan Results

No security vulnerabilities detected

This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes.

Analysis Summary

The PR enhances error message extraction by adding support for nested upstream error structures:

  • Extracts from error.upstream_error.error.message
  • Extracts from error.upstream_error.message

Security Verification:

  1. Data Flow Analysis: The extracted error messages flow through extractErrorMessage()finalErrorDetail → API response → Frontend toast notification
  2. XSS Protection: React's default text rendering auto-escapes strings. No dangerouslySetInnerHTML is used for error display
  3. Input Handling: The function only reads from already-parsed JSON (JSON.parse(errorText)) - no prototype pollution or injection vectors
  4. Existing Pattern: This change follows the same extraction pattern used for other error fields - no new attack surface introduced

Scanned Categories

  • ✅ Injection attacks (SQL, NoSQL, Command, LDAP, etc.)
  • ✅ Authentication and session management
  • ✅ Sensitive data exposure
  • ✅ Access control and authorization
  • ✅ Security misconfiguration
  • ✅ Cross-site scripting (XSS)
  • ✅ Insecure deserialization
  • ✅ SSRF and path traversal
  • ✅ Cryptographic weaknesses

📋 OWASP Top 10 Coverage

  • A01: Injection - Clean (no dynamic queries, no command execution)
  • A02: Broken Authentication - N/A (no auth changes)
  • A03: Sensitive Data Exposure - Clean (existing sanitization for logs via sanitizeErrorTextForLogging)
  • A04: XML External Entities - N/A
  • A05: Broken Access Control - N/A (no access control changes)
  • A06: Security Misconfiguration - N/A
  • A07: XSS - Clean (React auto-escapes text content)
  • A08: Insecure Deserialization - Clean (reads from already-parsed JSON)
  • A09: Known Vulnerabilities - N/A (no dependency changes)
  • A10: Logging & Monitoring - Clean (existing logging preserved)

🛡️ Security Posture

Acceptable - This is a low-risk refactoring change that adds error extraction paths without introducing new attack vectors.


🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage

@ding113 ding113 merged commit bb2298b into ding113:dev Nov 26, 2025
7 checks passed
@Silentely Silentely deleted the fix/cubence-error-parsing branch November 27, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size/XS Extra Small PR (< 50 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants