fix(proxy): persist Fake 200 error detail to DB/Redis and display in dashboard#814
fix(proxy): persist Fake 200 error detail to DB/Redis and display in dashboard#814
Conversation
…dashboard When upstream returns HTTP 200 with an error body (Fake 200), the detected error detail was only logged but not persisted. This caused the dashboard to show the raw detection code (e.g. "FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY") instead of the actual upstream error text (e.g. "Invalid API key"). - Store "CODE: detail" in errorMessage for DB/Redis persistence - Show actual detail (not code) in LogicTraceTab provider chain - Extract code prefix in SummaryTab for correct i18n lookup
Summary of ChangesHello @ding113, 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 resolves a user experience issue where detailed error messages for "Fake 200" responses were not being properly stored or displayed in the dashboard. Previously, only a generic error code was visible, hindering effective diagnosis. The changes ensure that specific upstream error details are now consistently persisted and correctly presented in both the Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
📝 WalkthroughWalkthrough更新了 FAKE_200 相关错误消息的提取与传递：在响应处理器中将上游错误构造成 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of losing valuable error details for 'Fake 200' responses. The changes ensure that the full error detail is persisted to the database and Redis by formatting the main error message as <code>: <detail>. For a cleaner user experience in the LogicTraceTab, the provider chain error message is updated to show just the error detail. The corresponding frontend change in SummaryTab.tsx is well-implemented, correctly parsing the new error message format while maintaining backward compatibility. The code is clear, and the solution is robust. I have no further suggestions.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| statusCode: effectiveStatusCode, | ||
| statusCodeInferred, | ||
| errorMessage: detected.code, | ||
| errorMessage: detected.detail ?? detected.code, |
There was a problem hiding this comment.
Breaks fake 200 UI in provider-chain-popover
This change sets the provider chain's errorMessage to detected.detail ?? detected.code. When detected.detail is present (e.g., "Invalid API key"), the value no longer starts with "FAKE_200_".
However, provider-chain-popover.tsx relies on item.errorMessage.startsWith("FAKE_200_") in three places (lines 131-136, 542-553) to detect fake 200 errors and render warning banners. After this change, those checks will evaluate to false for any fake 200 with a detail string, silently breaking:
hasFake200PostStreamFailure(line 131) — the fake 200 warning section won't renderfake200CodeForDisplay(line 134) — the reason text won't render- Per-item fake 200 indicator (line 542) — individual chain items won't show the indicator
Additionally, there's an inconsistency within this file: at line 281, the !streamEndedNormally path passes errorMessage (which uses the "CODE: detail" format from line 188 and starts with FAKE_200_), while this path passes just the detail text. The popover will behave differently depending on which code path was taken.
Consider either:
- Using a consistent format for both DB and chain (e.g.,
"CODE: detail"everywhere, and extracting the code in the UI as done inSummaryTab.tsx) - Updating
provider-chain-popover.tsxto handle the new format
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/response-handler.ts
Line: 333
Comment:
**Breaks fake 200 UI in provider-chain-popover**
This change sets the provider chain's `errorMessage` to `detected.detail ?? detected.code`. When `detected.detail` is present (e.g., `"Invalid API key"`), the value no longer starts with `"FAKE_200_"`.
However, `provider-chain-popover.tsx` relies on `item.errorMessage.startsWith("FAKE_200_")` in three places (lines 131-136, 542-553) to detect fake 200 errors and render warning banners. After this change, those checks will evaluate to `false` for any fake 200 with a detail string, silently breaking:
1. `hasFake200PostStreamFailure` (line 131) — the fake 200 warning section won't render
2. `fake200CodeForDisplay` (line 134) — the reason text won't render
3. Per-item fake 200 indicator (line 542) — individual chain items won't show the indicator
Additionally, there's an inconsistency within this file: at line 281, the `!streamEndedNormally` path passes `errorMessage` (which uses the `"CODE: detail"` format from line 188 and starts with `FAKE_200_`), while this path passes just the detail text. The popover will behave differently depending on which code path was taken.
Consider either:
- Using a consistent format for both DB and chain (e.g., `"CODE: detail"` everywhere, and extracting the code in the UI as done in `SummaryTab.tsx`)
- Updating `provider-chain-popover.tsx` to handle the new format
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx (1)
72-73:fake200Code的 else 分支应返回null而非errorMessage该变量仅在
isFake200PostStreamFailure && fake200Code条件下使用（第 75 行），当isFake200PostStreamFailure为false时其值永远不会被读取。当前 else 分支返回errorMessage会导致变量名与内容不符（持有任意错误消息），并存在冗余的&& errorMessage守卫（isFake200PostStreamFailure已隐含errorMessage为非空字符串）。♻️ 建议的简化方式
- const fake200Code = - isFake200PostStreamFailure && errorMessage ? errorMessage.split(": ")[0] : errorMessage; + const fake200Code = isFake200PostStreamFailure ? errorMessage!.split(": ")[0] : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx around lines 72 - 73, The variable fake200Code is misnamed because its else branch returns errorMessage; change the initialization of fake200Code so that it returns the parsed code (errorMessage.split(": ")[0]) when isFake200PostStreamFailure is true and otherwise returns null, and remove the redundant "&& errorMessage" guard when checking isFake200PostStreamFailure since that condition already implies a non-empty errorMessage; update usages that check "isFake200PostStreamFailure && fake200Code" to rely on fake200Code being null when not applicable.src/app/v1/_lib/proxy/response-handler.ts (1)
188-188:"CODE: detail"格式仅应用于 fake-200 路径，与非 200 状态码路径不一致第 188 行的修改正确地将
detected.detail持久化到 DB/Redis，但同一函数内的非 200 状态码路径（第 196-203 行）仍仅使用detected.code，不包含detail：// 第 199 行（未改动） errorMessage = detected.isError ? detected.code : `HTTP ${upstreamStatusCode}`;
handleNonStream中的类似路径（第 855 行）同样未做更新。如果希望所有错误路径均在 Dashboard 中展示详细信息，建议对这些路径做相同处理。此外，AI 摘要称该变更"同时影响……流自然结束后的非 200 处理路径"，但第 199 行并未标注为已修改（无
~）— AI 摘要与实际代码不符。♻️ 可选：对非 200 SSE 路径应用相同的格式
- errorMessage = detected.isError ? detected.code : `HTTP ${upstreamStatusCode}`; + errorMessage = detected.isError + ? (detected.detail ? `${detected.code}: ${detected.detail}` : detected.code) + : `HTTP ${upstreamStatusCode}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/response-handler.ts` at line 188, The non-200 error paths still construct errorMessage using only detected.code (e.g., the assignment using detected.isError ? detected.code : `HTTP ${upstreamStatusCode}` and the equivalent branch inside handleNonStream), so update those assignments to include detected.detail when present using the same format as the fake-200 path (i.e., use detected.detail ? `${detected.code}: ${detected.detail}` : detected.code for detected errors, and for non-detected errors keep the existing `HTTP ${upstreamStatusCode}` behavior), and also correct the AI summary/comment that claims the non-200 SSE path was modified so it accurately reflects which lines/functions changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx:
- Around line 72-73: The variable fake200Code is misnamed because its else
branch returns errorMessage; change the initialization of fake200Code so that it
returns the parsed code (errorMessage.split(": ")[0]) when
isFake200PostStreamFailure is true and otherwise returns null, and remove the
redundant "&& errorMessage" guard when checking isFake200PostStreamFailure since
that condition already implies a non-empty errorMessage; update usages that
check "isFake200PostStreamFailure && fake200Code" to rely on fake200Code being
null when not applicable.
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Line 188: The non-200 error paths still construct errorMessage using only
detected.code (e.g., the assignment using detected.isError ? detected.code :
`HTTP ${upstreamStatusCode}` and the equivalent branch inside handleNonStream),
so update those assignments to include detected.detail when present using the
same format as the fake-200 path (i.e., use detected.detail ? `${detected.code}:
${detected.detail}` : detected.code for detected errors, and for non-detected
errors keep the existing `HTTP ${upstreamStatusCode}` behavior), and also
correct the AI summary/comment that claims the non-200 SSE path was modified so
it accurately reflects which lines/functions changed.
…andle in popover
The previous commit used detected.detail ?? detected.code for the provider
chain errorMessage, which stripped the FAKE_200_ prefix and broke the
startsWith("FAKE_200_") checks in provider-chain-popover.tsx.
- Use "CODE: detail" format in addProviderToChain (consistent with DB)
- Extract code prefix via split(": ")[0] in popover for i18n lookup
- Extract code prefix in fake200CodeForDisplay for warning banner
| reason: t( | ||
| getFake200ReasonKey( | ||
| item.errorMessage, | ||
| item.errorMessage.split(": ")[0], |
There was a problem hiding this comment.
potential runtime error if item.errorMessage doesn't contain ": "
The code assumes all fake 200 errors have the new format, but old errors or errors without detail will cause .split(": ")[0] to return the full string. Line 138 handles this correctly by using optional chaining (?.errorMessage?.split), but this line doesn't.
| item.errorMessage.split(": ")[0], | |
| item.errorMessage?.split(": ")[0] ?? item.errorMessage, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
Line: 550
Comment:
potential runtime error if `item.errorMessage` doesn't contain ": "
The code assumes all fake 200 errors have the new format, but old errors or errors without detail will cause `.split(": ")[0]` to return the full string. Line 138 handles this correctly by using optional chaining (`?.errorMessage?.split`), but this line doesn't.
```suggestion
item.errorMessage?.split(": ")[0] ?? item.errorMessage,
```
How can I resolve this? If you propose a fix, please make it concise.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR addresses a legitimate UX issue where Fake 200 error details were being discarded. The implementation correctly persists the upstream error detail alongside the detection code in a "CODE: detail" format, improving visibility in both the dashboard summary and provider chain logs.
PR Size: XS
- Lines changed: 20 (12 additions, 8 deletions)
- Files changed: 3
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
No significant issues identified.
Review Coverage
- Logic and correctness - Clean. The new format "CODE: detail" correctly maintains backward compatibility via
startsWith(\"FAKE_200_\")checks. - Security (OWASP Top 10) - Clean. Error details are already sanitized via
sanitizeErrorTextForDetail()inupstream-error-detection.tsbefore being persisted. - Error handling - Clean. Proper null-safety with optional chaining (
item.errorMessage?.split(\"\")[0]) handles both new and old format gracefully. - Type safety - Clean. No unsafe
anyusage; leverages existing TypeScript types. - Documentation accuracy - Clean. No misleading comments or documentation drift.
- Test coverage - Adequate. Existing tests in
error-details-dialog.test.tsxandprovider-chain-popover.test.tsxverify Fake 200 detection behavior. - Code clarity - Good. The split pattern is straightforward and the data flow is clear.
Notable Implementation Details
-
response-handler.ts: Now persists
\"\${detected.code}: \${detected.detail}\"when detail exists, ensuring the actual upstream error is stored in DB/Redis. -
SummaryTab.tsx: Extracts the code prefix using
errorMessage.split(\": \")[0]for i18n lookup while preserving backward compatibility. -
provider-chain-popover.tsx: Uses optional chaining (
item.errorMessage?.split(\": \")[0]) for safe extraction, handling cases where errorMessage might be undefined.
Automated review by Claude AI
| specialSettings && specialSettings.length > 0 ? JSON.stringify(specialSettings, null, 2) : null; | ||
| const isFake200PostStreamFailure = | ||
| typeof errorMessage === "string" && errorMessage.startsWith("FAKE_200_"); | ||
| const fake200Code = |
There was a problem hiding this comment.
[MEDIUM] [TEST-EDGE-CASE] No unit coverage for new "FAKE_200_*: detail" parsing
Why this is a problem: New logic now relies on errorMessage.split(": ")[0] to recover the FAKE_200 code for i18n lookups:
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx:72-77src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx:134-138and:549-552
Existing UI tests only exercise the raw code format (no ": detail" suffix), so a delimiter/format regression will slip:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx:400-414src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx:274-291
This conflicts with CLAUDE.md:
2. **Test Coverage** - All new features must have unit test coverage of at least 80%
Suggested fix: Add/extend tests to cover the new "CODE: detail" format.
// src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx
test("renders fake-200 detected reason when errorMessage uses CODE: detail", () => {
const html = renderWithIntl(
<ErrorDetailsDialog
externalOpen
statusCode={502}
errorMessage={"FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY: Invalid API key"}
providerChain={null}
sessionId={null}
/>
);
expect(html).toContain("Detected reason: JSON has non-empty error.message");
});// src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx
test("renders fake-200 detected reason when chain errorMessage uses CODE: detail", () => {
const html = renderWithIntl(
<ProviderChainPopover
chain={[
{
id: 1,
name: "p1",
reason: "retry_failed",
statusCode: 502,
errorMessage: "FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY: Invalid API key",
},
]}
finalProvider="p1"
/>
);
expect(html).toContain("Detected reason: JSON has non-empty error.message");
});
There was a problem hiding this comment.
Code Review Summary
This PR changes fake-200 error persistence to store CODE: detail and updates dashboard components to split the code prefix for i18n lookups. The behavior change is reasonable, but the new parsing path is not covered by existing UI unit tests, increasing regression risk if the delimiter/format changes.
PR Size: XS
- Lines changed: 20
- Files changed: 3
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 1 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx:72andsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx:134lack unit coverage for the new"FAKE_200_*: detail"parsing (confidence 85).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#814and updated stats:+12/-8(20 lines), 3 files; applied/confirmed labelsize/XS. - Posted 1 inline review comment on
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxcalling out missing unit coverage for the new"FAKE_200_*: detail"parsing (with concrete test snippets forerror-details-dialog.test.tsxandprovider-chain-popover.test.tsx). - Submitted the required “Code Review Summary” via
gh pr review.
1 participant
Summary
When upstream returns HTTP 200 but the JSON body contains
error.message(Fake 200 detection codeFAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY), the actual error text fromdetected.detailwas only logged vialogger.warnand then discarded. Onlydetected.codewas persisted.This caused two UX issues:
providerChain[].errorMessageshowed the raw code"FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY"instead of the actual upstream error (e.g. "Invalid API key")errorMessagein DB was just the code, losing the actual causeRelated Issues & PRs:
Changes
response-handler.tserrorMessagenow stores"CODE: detail"format (e.g."FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY: Invalid API key") so the actual error is persistedaddProviderToChainnow usesdetected.detail ?? detected.code, so LogicTraceTab shows the actual error textSummaryTab.tsx"CODE: detail"format before passing togetFake200ReasonKey()for correct i18n lookupisFake200PostStreamFailurecheck still works since the new format still starts with"FAKE_200_"Data Flow After Fix
Breaking Changes
None. This is a UX improvement that changes the internal format of
errorMessagepersistence but maintains backward compatibility:"FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY""FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY: Invalid API key"startsWith("FAKE_200_")check inSummaryTab.tsxcontinues to work correctlyTesting
Automated Tests
bun run typecheck- passedbun run lint- passed (no new warnings)bun run test- 348 files, 3234 tests passedManual Testing
{"error":{"message":"some error"}}with HTTP 200, verify SummaryTab and LogicTraceTab display correctlyChecklist
Description enhanced with related issue/PR links
Greptile Summary
Enhanced fake 200 error handling by persisting the actual upstream error detail (e.g., "Invalid API key") alongside the detection code in a
"CODE: detail"format, improving visibility in both the dashboard summary and provider chain logs.response-handler.tsto persist errors as"FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY: Invalid API key"instead of just"FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY"SummaryTab.tsxto extract the code prefix for i18n lookup while displaying the full detailprovider-chain-popover.tsxto handle the new format in fake 200 detection and display logicIssues Found:
provider-chain-popover.tsxlacks optional chaining and may cause runtime errors ifitem.errorMessageis undefined or doesn't contain ": "Confidence Score: 3/5
src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx- fix the optional chaining issue on line 550Important Files Changed
Last reviewed commit: 13563c9