mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(sessions): log warning when parseJsonlEntries skips malformed JSONL lines (#98669)
* fix(sessions): log warning when parseJsonlEntries skips malformed lines * fix(sessions): also warn in buildSessionInfo for malformed JSONL lines * fix(sessions): warn in parseSessionEntries for malformed JSONL lines * test(sessions): add warning regression tests for parseSessionEntries * test(sessions): add parseJsonlEntries warning regression test Add test verifying parseJsonlEntries logs warning for malformed JSONL lines via loadEntriesFromFile, covering the second of three instrumented session JSONL readers. Co-Authored-By: Claude <noreply@anthropic.com> * test(sessions): update parseSessionEntries warning expectations after rebase * style(sessions): remove unnecessary type assertion in test * test(sessions): add buildSessionInfo warning regression tests via SessionManager.list --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,7 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { withOwnedSessionTranscriptWrites } from "../../config/sessions/transcript-write-context.js";
|
||||
import * as Logger from "../../logger.js";
|
||||
import { isTranscriptOnlyOpenClawAssistantMessage } from "../../shared/transcript-only-openclaw-assistant.js";
|
||||
import { prepareSessionManagerForRun } from "../embedded-agent-runner/session-manager-init.js";
|
||||
import { repairSessionFileIfNeeded } from "../session-file-repair.js";
|
||||
@@ -13,6 +14,7 @@ import {
|
||||
CURRENT_SESSION_VERSION,
|
||||
findMostRecentSession,
|
||||
loadEntriesFromFile,
|
||||
parseSessionEntries,
|
||||
SessionManager,
|
||||
type SessionEntry,
|
||||
} from "./session-manager.js";
|
||||
@@ -2647,6 +2649,140 @@ describe("SessionManager.open", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseSessionEntries", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("parses valid JSONL lines without logging warnings", () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const content = [
|
||||
JSON.stringify({ type: "session", id: "s1" }),
|
||||
JSON.stringify({ type: "message", id: "m1" }),
|
||||
].join("\n");
|
||||
|
||||
const entries = parseSessionEntries(content);
|
||||
|
||||
expect(entries).toHaveLength(2);
|
||||
expect(warnSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("logs a warning and skips malformed JSONL lines while preserving valid entries", () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const content = [
|
||||
JSON.stringify({ type: "session", id: "s1" }),
|
||||
"not valid json {{{",
|
||||
JSON.stringify({ type: "message", id: "m1" }),
|
||||
].join("\n");
|
||||
|
||||
const entries = parseSessionEntries(content);
|
||||
|
||||
expect(entries).toHaveLength(2);
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("parseJsonlEntries: skipped 1 malformed JSONL line"),
|
||||
);
|
||||
});
|
||||
|
||||
it("reports the correct skip count for multiple malformed lines", () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const content = [
|
||||
"bad line 1",
|
||||
JSON.stringify({ type: "session", id: "s1" }),
|
||||
"bad line 2",
|
||||
"bad line 3",
|
||||
].join("\n");
|
||||
|
||||
const entries = parseSessionEntries(content);
|
||||
|
||||
expect(entries).toHaveLength(1);
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("parseJsonlEntries: skipped 3 malformed JSONL line"),
|
||||
);
|
||||
});
|
||||
|
||||
it("skips empty lines without counting them as malformed", () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const content = [
|
||||
"",
|
||||
JSON.stringify({ type: "session", id: "s1" }),
|
||||
"",
|
||||
JSON.stringify({ type: "message", id: "m1" }),
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
const entries = parseSessionEntries(content);
|
||||
|
||||
expect(entries).toHaveLength(2);
|
||||
expect(warnSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("parseJsonlEntries logs warning for malformed lines via loadEntriesFromFile", async () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const dir = await makeTempDir();
|
||||
const sessionFile = path.join(dir, "session.jsonl");
|
||||
const header = buildSessionHeader(dir);
|
||||
const content = [
|
||||
JSON.stringify(header),
|
||||
"not valid json {{{",
|
||||
JSON.stringify(buildMessageEntry(1, null)),
|
||||
].join("\n");
|
||||
await fs.writeFile(sessionFile, content, "utf8");
|
||||
|
||||
const entries = loadEntriesFromFile(sessionFile);
|
||||
|
||||
expect(entries.length).toBeGreaterThanOrEqual(1);
|
||||
expect(warnSpy).toHaveBeenCalled();
|
||||
expect(
|
||||
warnSpy.mock.calls.some((call) =>
|
||||
call[0].includes("parseJsonlEntries: skipped 1 malformed JSONL line"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("buildSessionInfo logs warning for malformed lines via SessionManager.list", async () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const dir = await makeTempDir();
|
||||
const sessionFile = path.join(dir, "session.jsonl");
|
||||
const header = buildSessionHeader(dir);
|
||||
const content = [
|
||||
JSON.stringify(header),
|
||||
"not valid json {{{",
|
||||
JSON.stringify(buildMessageEntry(1, null)),
|
||||
].join("\n");
|
||||
await fs.writeFile(sessionFile, content, "utf8");
|
||||
|
||||
const sessions = await SessionManager.list(dir, dir);
|
||||
|
||||
expect(sessions).toHaveLength(1);
|
||||
expect(warnSpy).toHaveBeenCalled();
|
||||
expect(
|
||||
warnSpy.mock.calls.some((call) =>
|
||||
call[0].includes("buildSessionInfo: skipped 1 malformed JSONL line"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("buildSessionInfo does not log warning for clean session listing", async () => {
|
||||
const warnSpy = vi.spyOn(Logger, "logWarn").mockImplementation(() => {});
|
||||
const dir = await makeTempDir();
|
||||
const sessionFile = path.join(dir, "session.jsonl");
|
||||
const header = buildSessionHeader(dir);
|
||||
const content = [JSON.stringify(header), JSON.stringify(buildMessageEntry(1, null))].join("\n");
|
||||
await fs.writeFile(sessionFile, content, "utf8");
|
||||
|
||||
const sessions = await SessionManager.list(dir, dir);
|
||||
|
||||
expect(sessions).toHaveLength(1);
|
||||
// buildSessionInfo must not log any warning for a clean listing.
|
||||
const buildSessionInfoCalls = warnSpy.mock.calls.filter((call) =>
|
||||
call[0].includes("buildSessionInfo"),
|
||||
);
|
||||
expect(buildSessionInfoCalls).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
function readMessageContent(entry: SessionEntry): unknown {
|
||||
const content = (entry as { message: { content: unknown } }).message.content;
|
||||
if (Array.isArray(content)) {
|
||||
|
||||
@@ -38,6 +38,7 @@ import {
|
||||
} from "../../config/sessions/transcript-write-context.js";
|
||||
import { CURRENT_SESSION_VERSION } from "../../config/sessions/version.js";
|
||||
import type { ImageContent, Message, TextContent } from "../../llm/types.js";
|
||||
import { logWarn } from "../../logger.js";
|
||||
import { getAgentDir as getDefaultAgentDir, getSessionsDir } from "../config.js";
|
||||
import {
|
||||
type AgentMessage,
|
||||
@@ -938,6 +939,7 @@ function freezeJsonLikeValue(value: unknown, seen = new WeakSet<object>()): void
|
||||
function parseJsonlEntries(content: string): FileEntry[] {
|
||||
const entries: FileEntry[] = [];
|
||||
const lines = content.trim().split("\n");
|
||||
let skipped = 0;
|
||||
|
||||
for (const line of lines) {
|
||||
if (!line.trim()) {
|
||||
@@ -947,10 +949,20 @@ function parseJsonlEntries(content: string): FileEntry[] {
|
||||
const entry = JSON.parse(line) as FileEntry;
|
||||
entries.push(normalizeLoadedFileEntry(entry));
|
||||
} catch {
|
||||
// Skip malformed lines
|
||||
skipped++;
|
||||
}
|
||||
}
|
||||
|
||||
// Transcripts written by older code or repaired externally may contain
|
||||
// malformed entries that JSON.parse cannot deserialize. Warn once so the
|
||||
// operator knows data was skipped instead of silently dropping entries.
|
||||
if (skipped > 0) {
|
||||
logWarn(
|
||||
`parseJsonlEntries: skipped ${skipped} malformed JSONL line(s) — ` +
|
||||
`${entries.length} valid entries were loaded`,
|
||||
);
|
||||
}
|
||||
|
||||
return entries;
|
||||
}
|
||||
|
||||
@@ -1283,6 +1295,7 @@ async function buildSessionInfo(filePath: string): Promise<SessionInfo | null> {
|
||||
const content = await readFile(filePath, "utf8");
|
||||
const entries: FileEntry[] = [];
|
||||
const lines = content.trim().split("\n");
|
||||
let skipped = 0;
|
||||
|
||||
for (const line of lines) {
|
||||
if (!line.trim()) {
|
||||
@@ -1291,10 +1304,17 @@ async function buildSessionInfo(filePath: string): Promise<SessionInfo | null> {
|
||||
try {
|
||||
entries.push(JSON.parse(line) as FileEntry);
|
||||
} catch {
|
||||
// Skip malformed lines
|
||||
skipped++;
|
||||
}
|
||||
}
|
||||
|
||||
if (skipped > 0) {
|
||||
logWarn(
|
||||
`buildSessionInfo: skipped ${skipped} malformed JSONL line(s) in ${filePath} — ` +
|
||||
`${entries.length} valid entries were loaded`,
|
||||
);
|
||||
}
|
||||
|
||||
if (entries.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user