Skip to content

fix(api): add safe JSON parsing for upstream provider responses#64

Merged
pescn merged 2 commits into
mainfrom
fix/safe-json-parsing
Jan 28, 2026
Merged

fix(api): add safe JSON parsing for upstream provider responses#64
pescn merged 2 commits into
mainfrom
fix/safe-json-parsing

Conversation

@pescn
Copy link
Copy Markdown
Contributor

@pescn pescn commented Jan 28, 2026

Summary

  • Adds safeParseToolArgs() and parseJsonResponse() utilities in backend/src/utils/json.ts to handle malformed JSON from upstream LLM providers gracefully
  • Replaces 15 bare JSON.parse calls across the backend with safe alternatives: tool arguments return {} on failure, response bodies re-throw with descriptive errors, and error bodies use inline try-catch with format-specific fallbacks
  • Prevents unhandled SyntaxError crashes when providers return HTML error pages, truncated responses, or other non-JSON content

Test plan

  • bun run check — type checking passes
  • bun run lint — linting passes (0 new warnings)
  • bun run build — build succeeds
  • Manual test: verify graceful handling when upstream returns HTML instead of JSON
  • Manual test: verify tool calls with malformed arguments degrade to empty {}

🤖 Generated with Claude Code

Summary by CodeRabbit

发行说明

  • Bug Fixes

    • 改进了对上游返回不可解析或格式错误 JSON 的处理,遇到解析失败时返回结构化错误而非抛出异常
    • 增强了工具参数与响应的解析健壮性,减少因恶意或异常输入导致的失败
  • Refactor

    • 统一并替换了散落的直接解析逻辑,采用集中化的安全解析工具
  • Tests

    • 新增单元与集成测试覆盖各类 JSON 边界与错误场景
  • Chores

    • 添加了用于运行测试的脚本命令

✏️ Tip: You can customize this high-level summary in your review settings.

Unhandled JSON.parse calls crash with SyntaxError when upstream LLM
providers return malformed JSON (HTML error pages, truncated responses).
Adds safeParseToolArgs (returns {} on failure) for tool arguments and
parseJsonResponse (descriptive re-throw) for response bodies. Wraps
error body parsing in try-catch with format-specific fallbacks.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 28, 2026

📝 Walkthrough

Walkthrough

在多个适配器和 API 路由中引入并使用两个新的 JSON 工具函数 safeParseToolArgsparseJsonResponse,将原有的直接 JSON.parse 调用替换为更安全的解析逻辑,并在上游错误处理处增加解析失败的降级/标准化错误返回。

Changes

Cohort / File(s) 变更摘要
新增 JSON 工具
backend/src/utils/json.ts
新增 safeParseToolArgs(jsonString)(解析失败返回空对象并记录警告)和 parseJsonResponse<T>(text, context)(解析失败抛出含上下文的错误并记录日志)。
上游适配器(响应解析)
backend/src/adapters/upstream/openai.ts, backend/src/adapters/upstream/openai-responses.ts, backend/src/adapters/upstream/anthropic.ts
将上游响应的 JSON.parse(text) 替换为 parseJsonResponse<T>(text, context);在工具调用参数解析处使用 safeParseToolArgs(...)
请求适配器 / 工具调用
backend/src/adapters/request/openai-chat.ts, backend/src/adapters/tools.ts, backend/src/utils/reqIdHandler.ts
将对工具调用参数的直接 JSON.parse 替换为 safeParseToolArgs,移除/替换本地不安全解析辅助实现。
API 路由:错误处理与流式路径
backend/src/api/v1/completions.ts, backend/src/api/v1/messages.ts, backend/src/api/v1/responses.ts
在处理上游错误(stream/non-stream)时为上游 body 的 JSON 解析增加 try/catch;解析失败时返回标准化的“unparseable_error”结构而不是抛出解析异常。
测试与包脚本
backend/src/utils/json.test.ts, backend/src/utils/json.integration.test.ts, backend/package.json
添加单元与集成测试覆盖新解析工具的多种成功/失败场景;在 package.json 中新增 test: "bun test" 脚本。

Sequence Diagram(s)

sequenceDiagram
    rect rgba(200,220,255,0.5)
    participant Client
    end
    rect rgba(200,255,200,0.5)
    participant API as "API Route\n(completions/messages/responses)"
    end
    rect rgba(255,220,200,0.5)
    participant Utils as "utils/json\n(safeParseToolArgs,\nparseJsonResponse)"
    end
    rect rgba(255,200,220,0.5)
    participant Upstream as "Upstream\n(OpenAI/Anthropic)"
    end

    Client->>API: 发起请求/工具调用
    API->>Upstream: 转发请求并接收响应文本
    Upstream-->>API: 返回响应文本或错误体
    API->>Utils: 使用 parseJsonResponse(text, context)
    alt 解析成功
        Utils-->>API: 返回解析结果
        API->>Client: 返回正常响应
    else 解析失败
        Utils-->>API: 抛出含上下文的错误
        API->>API: 捕获并构造标准化错误对象 (unparseable_error)
        API->>Client: 返回标准化错误响应
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 分钟

Possibly related issues

Possibly related PRs

诗歌

🐰 ✨
我是一只小兔龟速翻译,
JSON 汩汩流,别让它惊嚇,
解析稳如胡萝卜堆,
错误归位不再慌,
工具调用,兔子为你护航 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR标题准确概括了主要变更:引入安全的JSON解析机制以处理上游提供商的响应,这与整个变更集的核心目的完全一致。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
backend/src/utils/json.test.ts (1)

131-161: 建议使用更清晰的断言方式表达"不应到达此处"的意图。

expect(true).toBe(false) 虽然可行,但语义不够直观。Bun 的测试框架支持 expect.assertions(n) 或者可以直接让代码抛出明确错误。

♻️ 建议的改进
  test("should include preview in error message for short text", () => {
    const input = "Not JSON at all";
-   try {
-     parseJsonResponse(input, "TestAPI");
-     expect(true).toBe(false); // Should not reach here
-   } catch (error) {
-     expect(error).toBeInstanceOf(Error);
-     expect((error as Error).message).toContain("Not JSON at all");
-   }
+   expect(() => parseJsonResponse(input, "TestAPI")).toThrow();
+   try {
+     parseJsonResponse(input, "TestAPI");
+   } catch (error) {
+     expect(error).toBeInstanceOf(Error);
+     expect((error as Error).message).toContain("Not JSON at all");
+   }
  });

或者保留现有结构,但使用更明确的失败提示:

-     expect(true).toBe(false); // Should not reach here
+     throw new Error("Expected parseJsonResponse to throw, but it did not");
backend/src/utils/json.integration.test.ts (1)

93-99: 建议明确测试期望行为。

当前测试只验证返回值是对象类型,但没有验证具体值。如果 Bun 的 JSON 解析器接受尾随逗号,应明确测试期望的解析结果。

♻️ 建议的改进
    test("should handle trailing commas", () => {
      const malformed = '{"query": "test", "limit": 10,}';
      const result = safeParseToolArgs(malformed);
-     // Note: Some JSON parsers accept trailing commas, Bun's might too
-     // But we're testing that it doesn't crash
-     expect(typeof result).toBe("object");
+     // Bun's JSON parser rejects trailing commas per JSON spec
+     // If it parses successfully in future Bun versions, this test will catch that change
+     expect(result).toEqual({});
    });

或者如果确实需要兼容两种行为:

    test("should handle trailing commas without crashing", () => {
      const malformed = '{"query": "test", "limit": 10,}';
      const result = safeParseToolArgs(malformed);
-     // Note: Some JSON parsers accept trailing commas, Bun's might too
-     // But we're testing that it doesn't crash
-     expect(typeof result).toBe("object");
+     // Either parses successfully or returns empty object - both are acceptable
+     expect(result).toBeTypeOf("object");
+     expect(result).not.toBeNull();
    });
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c149143 and 14540fd.

📒 Files selected for processing (6)
  • backend/package.json
  • backend/src/api/v1/messages.ts
  • backend/src/api/v1/responses.ts
  • backend/src/utils/json.integration.test.ts
  • backend/src/utils/json.test.ts
  • backend/src/utils/json.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/src/utils/json.ts
  • backend/src/api/v1/responses.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-24T18:23:42.635Z
Learnt from: pescn
Repo: EM-GeekLab/NexusGate PR: 59
File: backend/src/api/v1/responses.ts:566-577
Timestamp: 2026-01-24T18:23:42.635Z
Learning: When using Elysia with the apiKeyPlugin, if a route option has checkApiKey: true, the apiKeyRecord parameter is guaranteed to be non-null. Do not add explicit null checks or non-null assertions for apiKeyRecord in such routes; rely on this contract and avoid unnecessary guards to improve readability. If there is any doubt about the guarantee in a specific place, add a runtime assertion at the boundary where the plugin is configured, but otherwise omit null checks for apiKeyRecord in backend code under backend/.

Applied to files:

  • backend/src/utils/json.test.ts
  • backend/src/utils/json.integration.test.ts
  • backend/src/api/v1/messages.ts
📚 Learning: 2026-01-24T22:33:16.471Z
Learnt from: pescn
Repo: EM-GeekLab/NexusGate PR: 59
File: backend/src/utils/streaming-context.ts:132-146
Timestamp: 2026-01-24T22:33:16.471Z
Learning: In NexusGate, for ReqId deduplication (specifically in streaming-context.ts and reqIdHandler.ts), when reqIdContext is present, finalizeReqId should use updateCompletion instead of addCompletions. This causes ReqId requests to not write srv_logs entries; errors are captured in the completion record's status (e.g., failed/aborted). This avoids duplicate srv_logs entries during retry scenarios and keeps the audit trail cleaner. Treat the completion record as the source of truth for ReqId request outcomes.

Applied to files:

  • backend/src/utils/json.test.ts
  • backend/src/utils/json.integration.test.ts
🧬 Code graph analysis (3)
backend/src/utils/json.test.ts (1)
backend/src/utils/json.ts (2)
  • safeParseToolArgs (15-40)
  • parseJsonResponse (47-63)
backend/src/utils/json.integration.test.ts (1)
backend/src/utils/json.ts (2)
  • parseJsonResponse (47-63)
  • safeParseToolArgs (15-40)
backend/src/api/v1/messages.ts (1)
backend/src/utils/json.ts (1)
  • safeParseToolArgs (15-40)
🔇 Additional comments (10)
backend/package.json (1)

12-12: LGTM!

测试脚本配置正确,与项目现有的 Bun 工具链保持一致。

backend/src/utils/json.test.ts (2)

8-92: LGTM!

safeParseToolArgs 的测试覆盖全面,包括有效 JSON、嵌套对象、格式错误、HTML 内容、截断 JSON、空字符串、标量值、null 和数组等各种边界情况。


94-181: LGTM!

parseJsonResponse 测试覆盖了成功解析、错误抛出、预览截断和错误原因保留等关键场景。

backend/src/api/v1/messages.ts (4)

43-43: LGTM!

正确引入 safeParseToolArgs 工具函数。


294-299: LGTM!

使用 safeParseToolArgs 替代直接 JSON.parse 是正确的改进。当 tc.function.argumentsundefinednull 时提供 "{}" 作为默认值,确保了解析的健壮性。


629-640: 流式请求错误处理改进良好。

添加的 try-catch 可以优雅地处理上游返回非 JSON 内容(如 HTML 错误页面)的情况,返回结构化的错误响应而不是崩溃。


728-739: 非流式请求错误处理与流式路径保持一致。

两个分支使用相同的错误处理模式,保证了 API 行为的一致性。

backend/src/utils/json.integration.test.ts (3)

9-58: LGTM!

真实世界 HTML 错误响应的测试覆盖了常见的上游错误场景(500、502、Cloudflare 522),这些正是本 PR 要解决的问题。


168-177: 空字节测试设计合理。

允许成功或失败两种结果是合理的,因为不同 JSON 解析器对空字节的处理可能不同。测试确保了无论哪种情况都不会崩溃。


187-211: LGTM!

有效边界情况的测试(Unicode、转义字符、深度嵌套、数组)确保了正常输入的解析不受影响。

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello @pescn, 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!

This pull request significantly enhances the backend's resilience against malformed or unexpected responses from external LLM providers. By centralizing JSON parsing logic into new, safer utility functions, the system can now gracefully handle non-JSON content or corrupted data, preventing application crashes and providing clearer error feedback. This change improves the overall stability and user experience when interacting with upstream services.

Highlights

  • New JSON Parsing Utilities: Introduced safeParseToolArgs() and parseJsonResponse() in backend/src/utils/json.ts to provide robust and graceful handling of JSON parsing.
  • Replaced Direct JSON.parse Calls: Replaced 15 instances of bare JSON.parse across the backend with the new safe utilities, ensuring consistent error handling.
  • Improved Error Handling for Upstream Responses: Implemented specific error handling for malformed JSON from upstream LLM providers, preventing crashes and providing more descriptive error messages when non-JSON content (like HTML error pages or truncated responses) is received.
  • Degraded State for Tool Arguments: Tool arguments that fail to parse now gracefully degrade to an empty object ({}), maintaining system stability.
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.

Copy link
Copy Markdown

@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

This pull request introduces safe JSON parsing utilities to prevent crashes from malformed upstream responses, which is a great improvement for robustness. The changes are well-implemented, replacing bare JSON.parse calls with safer alternatives across various adapters and API endpoints. I've suggested one improvement to the new safeParseToolArgs utility to make it even more robust by ensuring it always returns an object, preventing potential runtime errors from valid but non-object JSON inputs, aligning with our guidelines for robust JSON parsing and ready-to-use helper function returns.

Comment thread backend/src/utils/json.ts
@pescn pescn linked an issue Jan 28, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor Author

@pescn pescn left a comment

Choose a reason for hiding this comment

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

Fixed: Added runtime type validation

Good catch! You're absolutely right that JSON.parse() can return scalar values, null, or arrays - not just objects.

For example:

  • JSON.parse("123")123 (number)
  • JSON.parse("null")null
  • JSON.parse("[1,2,3]") → array

If an LLM hallucinates and returns a scalar/array as tool arguments, this would cause a type safety issue.

Changes Applied

Updated safeParseToolArgs() to validate the parsed result:

const parsed: unknown = JSON.parse(jsonString);

// Validate that parsed result is a plain object (not null, array, or scalar)
if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) {
  return parsed as Record<string, unknown>;
}

// If LLM returned a scalar, array, or null as tool arguments, treat as malformed
logger.warn("Parsed tool arguments is not an object, returning empty object", {
  input: jsonString,
  parsedType: Array.isArray(parsed) ? "array" : typeof parsed,
  parsedValue: parsed,
});
return {};

Test Coverage

Added 6 new tests covering:

  • Scalar number ("123")
  • Scalar boolean ("true")
  • Scalar string ('"hello"')
  • null value
  • Arrays ("[1,2,3]")
  • Array of objects ('[{"name":"test"}]')

All 46 tests pass ✅

…e JSON parsing

- Add runtime type validation in safeParseToolArgs to reject scalars, arrays, and null
- Standardize error response format across all API endpoints with "unparseable_error" code
- Add comprehensive logging with full response bodies for debugging
- Create 46 unit and integration tests covering malformed JSON scenarios

This addresses code review feedback to ensure type safety when parsing tool arguments
from LLM responses that may return non-object values.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@pescn pescn merged commit dc8e30c into main Jan 28, 2026
2 checks passed
@pescn pescn deleted the fix/safe-json-parsing branch January 28, 2026 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

改进:在代码库中添加安全的 JSON 解析处理

1 participant