Skip to content

Commit 3147754

Browse files
dahliaclaude
andcommitted
Harden log rendering and storage against edge cases
LogStore.get() now returns a shallow copy so callers cannot mutate the internal array. The trace detail page uses a safeISOString helper that falls back to "(invalid)" for NaN or otherwise unparseable timestamps instead of throwing a RangeError. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent af14782 commit 3147754

3 files changed

Lines changed: 82 additions & 5 deletions

File tree

packages/debugger/src/mod.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,6 +1148,65 @@ test("trace detail page shows log records", async () => {
11481148
ok(html.includes("log record"));
11491149
});
11501150

1151+
test("log API returns a snapshot: mutating the array does not affect store", async () => {
1152+
const { federation } = createMockFederation();
1153+
const exporter = createMockExporter();
1154+
const dbg = createFederationDebugger(federation, { exporter });
1155+
1156+
const traceId = "aaaa1111bbbb2222cccc3333dddd4444";
1157+
dbg.sink({
1158+
category: ["fedify"],
1159+
level: "info",
1160+
message: ["original"],
1161+
rawMessage: "original",
1162+
timestamp: Date.now(),
1163+
properties: { traceId, spanId: "1234567890abcdef" },
1164+
});
1165+
1166+
// First fetch: get logs
1167+
const r1 = await dbg.fetch(
1168+
new Request(`https://example.com/__debug__/api/logs/${traceId}`),
1169+
{ contextData: undefined },
1170+
);
1171+
const logs1 = (await r1.json()) as SerializedLogRecord[];
1172+
strictEqual(logs1.length, 1);
1173+
1174+
// Second fetch: logs should still have 1 entry (not affected by prior read)
1175+
const r2 = await dbg.fetch(
1176+
new Request(`https://example.com/__debug__/api/logs/${traceId}`),
1177+
{ contextData: undefined },
1178+
);
1179+
const logs2 = (await r2.json()) as SerializedLogRecord[];
1180+
strictEqual(logs2.length, 1);
1181+
strictEqual(logs2[0].message, "original");
1182+
});
1183+
1184+
test("trace detail page handles invalid log timestamp gracefully", async () => {
1185+
const { federation } = createMockFederation();
1186+
const exporter = createMockExporter();
1187+
const dbg = createFederationDebugger(federation, { exporter });
1188+
1189+
const traceId = "aaaa1111bbbb2222cccc3333dddd4444";
1190+
// Push a log with NaN timestamp (simulates corrupted data)
1191+
dbg.sink({
1192+
category: ["fedify"],
1193+
level: "info",
1194+
message: ["test message"],
1195+
rawMessage: "test message",
1196+
timestamp: NaN,
1197+
properties: { traceId, spanId: "1234567890abcdef" },
1198+
});
1199+
1200+
const request = new Request(
1201+
`https://example.com/__debug__/traces/${traceId}`,
1202+
);
1203+
// Should not throw — the page must render without crashing
1204+
const response = await dbg.fetch(request, { contextData: undefined });
1205+
strictEqual(response.status, 200);
1206+
const html = await response.text();
1207+
ok(html.includes("test message"));
1208+
});
1209+
11511210
test("trace detail page shows empty log message", async () => {
11521211
const { federation } = createMockFederation();
11531212
const exporter = createMockExporter();

packages/debugger/src/mod.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ class LogStore {
112112
}
113113
}
114114

115-
get(traceId: string): SerializedLogRecord[] {
116-
return this.#logs.get(traceId) ?? [];
115+
get(traceId: string): readonly SerializedLogRecord[] {
116+
const logs = this.#logs.get(traceId);
117+
return logs != null ? [...logs] : [];
117118
}
118119
}
119120

packages/debugger/src/views/trace-detail.tsx

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@ import type { TraceActivityRecord } from "@fedify/fedify/otel";
55
import type { SerializedLogRecord } from "../mod.tsx";
66
import { Layout } from "./layout.tsx";
77

8+
/**
9+
* Safely formats a timestamp (milliseconds since epoch) as an ISO string.
10+
* Returns `"(invalid)"` if the timestamp is not a finite number or produces
11+
* an invalid `Date`.
12+
*/
13+
function safeISOString(timestamp: number): string {
14+
if (!Number.isFinite(timestamp)) return "(invalid)";
15+
try {
16+
return new Date(timestamp).toISOString();
17+
} catch {
18+
return "(invalid)";
19+
}
20+
}
21+
822
/**
923
* Props for the {@link TraceDetailPage} component.
1024
*/
@@ -22,7 +36,7 @@ export interface TraceDetailPageProps {
2236
/**
2337
* The list of log records for this trace.
2438
*/
25-
logs: SerializedLogRecord[];
39+
logs: readonly SerializedLogRecord[];
2640

2741
/**
2842
* The path prefix for the debug dashboard.
@@ -179,8 +193,11 @@ export const TraceDetailPage: FC<TraceDetailPageProps> = (
179193
{logs.map((log, i) => (
180194
<tr key={i} class={`log-${log.level}`}>
181195
<td>
182-
<time datetime={new Date(log.timestamp).toISOString()}>
183-
{new Date(log.timestamp).toISOString().slice(11, 23)}
196+
<time datetime={safeISOString(log.timestamp)}>
197+
{(() => {
198+
const iso = safeISOString(log.timestamp);
199+
return iso === "(invalid)" ? iso : iso.slice(11, 23);
200+
})()}
184201
</time>
185202
</td>
186203
<td>

0 commit comments

Comments
 (0)