Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/fixes/7845-log-detail-structured-error.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- fix(dashboard): Request Logs detail modal no longer crashes when the persisted error is a structured object (#7845)
5 changes: 3 additions & 2 deletions src/shared/components/RequestLoggerDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
getProtocolColor,
} from "@/shared/constants/colors";
import { formatDuration, formatApiKeyLabel, maskAccount } from "@/shared/utils/formatting";
import { formatErrorForDisplay } from "@/shared/utils/formatting";

// ─── Payload Code Block ─────────────────────────────────────────────────────

Expand Down Expand Up @@ -617,8 +618,8 @@ export default function RequestLoggerDetail({
<div className="text-[10px] text-red-600 dark:text-red-400 uppercase tracking-wider mb-1 font-bold">
Error
</div>
<div className="text-sm text-red-600 dark:text-red-300 font-mono">
{detail?.error || log.error}
<div className="text-sm text-red-600 dark:text-red-300 font-mono whitespace-pre-wrap break-words">
{formatErrorForDisplay(detail?.error || log.error)}
</div>
</div>
)}
Expand Down
17 changes: 17 additions & 0 deletions src/shared/utils/formatting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,20 @@ export function formatResetCountdown(resetAt: string | number | null | undefined
if (minutes > 0) return `${minutes}m ${seconds}s`;
return `${seconds}s`;
}

/**
* Coerces a persisted log `error` field into safe display text. It is
* legitimate for this field to be a structured object (e.g. `{ code,
* message }`) — only the RENDER needs to be a string, never the persisted
* artifact itself. Used by RequestLoggerDetail to avoid React error #31
* ("Objects are not valid as a React child") when rendering it. See #7845.
*/
export function formatErrorForDisplay(err: unknown): string | null {
if (err == null) return null;
if (typeof err === "string") return err;
try {
return JSON.stringify(err, null, 2);
} catch {
return String(err);
}
}
117 changes: 117 additions & 0 deletions tests/unit/ui/issue-7845-log-detail-structured-error.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// @vitest-environment jsdom
/**
* TDD regression for #7845: opening the detail modal for a failed
* /dashboard/logs entry crashes the dashboard (React error #31 —
* "Objects are not valid as a React child") when the persisted artifact's
* `error` field is a structured object (e.g. `{ code, message }`) instead of
* a plain string.
*
* Root cause: `RequestLoggerDetail` rendered `{detail?.error || log.error}`
* directly as a React child. The list summary keeps `error_summary` as a
* string, but `/api/logs/{id}` returns the persisted structured `error`
* object for detail view — React throws when that object hits the child
* position (error #31).
*
* Fix: coerce any non-string `error` payload to a formatted JSON string
* before rendering, while leaving plain string errors rendered verbatim
* (no added JSON quoting).
*/
import React from "react";
import { act } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it } from "vitest";

const RequestLoggerDetail = (
await import("../../../src/shared/components/RequestLoggerDetail.tsx")
).default;

let container: HTMLElement;
let root: Root;

function baseLog(overrides: Record<string, unknown> = {}) {
return {
id: "log-1",
status: 500,
method: "POST",
path: "/v1/chat/completions",
model: "gpt-test",
provider: "openai",
timestamp: new Date().toISOString(),
duration: 42,
tokens: { in: 1, out: 2 },
...overrides,
};
}

const noop = () => {};

beforeEach(() => {
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
});

afterEach(async () => {
if (root) {
await act(async () => {
root.unmount();
});
}
container?.remove();
});

describe("RequestLoggerDetail structured error rendering (#7845)", () => {
it("renders a structured object error without throwing, showing its content in the DOM", async () => {
const structuredError = {
code: "codex_ws_provider_required",
message: "Responses WebSocket bridge only supports Codex models, got proxy",
};
const log = baseLog({ error: structuredError });
const detail = { ...log, error: structuredError };

let renderError: unknown = null;
await act(async () => {
try {
root.render(
<RequestLoggerDetail
log={log}
detail={detail}
loading={false}
debugEnabled={false}
onClose={noop}
onCopy={async () => true}
/>
);
} catch (err) {
renderError = err;
}
});

expect(renderError).toBeNull();
expect(container.textContent).toContain("codex_ws_provider_required");
expect(container.textContent).toContain(
"Responses WebSocket bridge only supports Codex models, got proxy"
);
});

it("keeps rendering a plain string error verbatim, with no added JSON quoting", async () => {
const log = baseLog({ error: "upstream timeout" });
const detail = { ...log, error: "upstream timeout" };

await act(async () => {
root.render(
<RequestLoggerDetail
log={log}
detail={detail}
loading={false}
debugEnabled={false}
onClose={noop}
onCopy={async () => true}
/>
);
});

expect(container.textContent).toContain("upstream timeout");
expect(container.textContent).not.toContain('"upstream timeout"');
});
});
Loading