Repository navigation
fix(cli): restore omniroute logs command stream (#2756) #2810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| /** | ||
| * CLI Log Stream API — GET /api/cli-tools/logs | ||
| * | ||
| * Reads the application log file and returns matching entries. | ||
| * Called by the CLI `omniroute logs` command via | ||
| * `src/lib/cli-helper/log-streamer.ts`. | ||
| * | ||
| * Query params: | ||
| * - follow: boolean — kept for forward-compat; ignored in this | ||
| * implementation (streaming follow-mode is handled client-side | ||
| * by the log-streamer's ReadableStream). | ||
| * - filter: comma-separated strings — entries whose `component`, | ||
| * `module`, or `msg` fields match ANY token are included. | ||
| * Case-insensitive substring match. | ||
| * - limit: max number of entries to return — default 500, max 2000. | ||
| * | ||
| * Auth: Tier 3 MANAGEMENT — requireManagementAuth (same as all | ||
| * other /api/cli-tools/* routes). | ||
| * | ||
| * Note: this route reads logs only and spawns no child processes, | ||
| * so it does NOT require isLocalOnlyPath() classification. | ||
| */ | ||
|
|
||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { readFileSync, existsSync } from "fs"; | ||
| import { getAppLogFilePath } from "@/lib/logEnv"; | ||
| import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; | ||
| import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; | ||
|
|
||
| // Map pino numeric levels to string levels | ||
| const NUMERIC_LEVEL_MAP: Record<number, string> = { | ||
| 10: "trace", | ||
| 20: "debug", | ||
| 30: "info", | ||
| 40: "warn", | ||
| 50: "error", | ||
| 60: "fatal", | ||
| }; | ||
|
|
||
| function parseLevel(raw: string | number): string { | ||
| if (typeof raw === "number") { | ||
| return NUMERIC_LEVEL_MAP[raw] || "info"; | ||
| } | ||
| return String(raw).toLowerCase(); | ||
| } | ||
|
|
||
| function stringifyLogValue(value: unknown): string { | ||
| if (value === undefined || value === null) return ""; | ||
| if (typeof value === "string") return value; | ||
| if (value instanceof Error) return value.message || value.name; | ||
| if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") { | ||
| return String(value); | ||
| } | ||
| try { | ||
| const json = JSON.stringify(value); | ||
| return typeof json === "string" ? json : String(value); | ||
| } catch { | ||
| return String(value); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * GET /api/cli-tools/logs | ||
| */ | ||
| export async function GET(req: NextRequest) { | ||
| const authError = await requireManagementAuth(req); | ||
| if (authError) return authError; | ||
|
|
||
| try { | ||
| const { searchParams } = new URL(req.url); | ||
|
|
||
| // `follow` is accepted for forward-compat but not used server-side; | ||
| // the CLI's ReadableStream already handles reconnection client-side. | ||
| const _follow = searchParams.get("follow") === "true"; | ||
|
|
||
| // Comma-separated filter tokens (e.g. "router,oauth") | ||
| const filterRaw = searchParams.get("filter") || ""; | ||
| const filterTokens = filterRaw | ||
| .split(",") | ||
| .map((t) => t.trim().toLowerCase()) | ||
| .filter(Boolean); | ||
|
|
||
| const rawLimit = parseInt(searchParams.get("limit") || "500", 10); | ||
| const limit = Math.min(Number.isFinite(rawLimit) && rawLimit > 0 ? rawLimit : 500, 2000); | ||
|
|
||
| const logPath = getAppLogFilePath(); | ||
|
|
||
| if (!existsSync(logPath)) { | ||
| return NextResponse.json([], { status: 200 }); | ||
| } | ||
|
|
||
| const raw = readFileSync(logPath, "utf-8"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| const lines = raw.trim().split("\n").filter(Boolean); | ||
|
|
||
| const oneHourAgo = Date.now() - 60 * 60 * 1000; | ||
| const entries: Record<string, unknown>[] = []; | ||
|
|
||
| for (const line of lines) { | ||
| try { | ||
| const entry = JSON.parse(line) as Record<string, unknown>; | ||
|
|
||
| // Filter by time (last 1 hour) | ||
| const ts = entry.time || entry.timestamp; | ||
| if (ts) { | ||
| const entryTime = new Date(ts as string | number).getTime(); | ||
| if (entryTime < oneHourAgo) continue; | ||
| } | ||
|
|
||
| // Normalize fields | ||
| entry.level = parseLevel(entry.level as string | number); | ||
| entry.msg = stringifyLogValue(entry.msg ?? entry.message ?? ""); | ||
| entry.message = stringifyLogValue(entry.message ?? entry.msg); | ||
| if (entry.component !== undefined) entry.component = stringifyLogValue(entry.component); | ||
| if (entry.module !== undefined) entry.module = stringifyLogValue(entry.module); | ||
|
|
||
| // Normalize timestamp field | ||
| if (entry.time && !entry.timestamp) { | ||
| entry.timestamp = entry.time; | ||
| } | ||
|
|
||
| // Apply filter tokens — entry is included if ANY token matches | ||
| // component, module, or msg (case-insensitive substring) | ||
| if (filterTokens.length > 0) { | ||
| const haystack = [ | ||
| String(entry.component || ""), | ||
| String(entry.module || ""), | ||
| String(entry.msg || ""), | ||
| ] | ||
| .join(" ") | ||
| .toLowerCase(); | ||
|
|
||
| const matches = filterTokens.some((token) => haystack.includes(token)); | ||
| if (!matches) continue; | ||
| } | ||
|
|
||
| entries.push(entry); | ||
| } catch { | ||
| // Skip unparseable lines | ||
| } | ||
| } | ||
|
|
||
| // Return last N entries (most recent) | ||
| const result = entries.slice(-limit); | ||
|
Comment on lines
+98
to
+143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since Pino logs are written in chronological order, iterating from the end of the file (newest first) allows us to break early as soon as we encounter a log older than for (let i = lines.length - 1; i >= 0; i--) {
const line = lines[i];
try {
const entry = JSON.parse(line) as Record<string, unknown>;
// Filter by time (last 1 hour)
const ts = entry.time || entry.timestamp;
if (ts) {
const entryTime = new Date(ts as string | number).getTime();
if (entryTime < oneHourAgo) {
break; // Since logs are chronological, older lines can be skipped entirely
}
}
// Normalize fields
entry.level = parseLevel(entry.level as string | number);
entry.msg = stringifyLogValue(entry.msg ?? entry.message ?? "");
entry.message = stringifyLogValue(entry.message ?? entry.msg);
if (entry.component !== undefined) entry.component = stringifyLogValue(entry.component);
if (entry.module !== undefined) entry.module = stringifyLogValue(entry.module);
// Normalize timestamp field
if (entry.time && !entry.timestamp) {
entry.timestamp = entry.time;
}
// Apply filter tokens — entry is included if ANY token matches
// component, module, or msg (case-insensitive substring)
if (filterTokens.length > 0) {
const haystack = [
String(entry.component || ""),
String(entry.module || ""),
String(entry.msg || ""),
]
.join(" ")
.toLowerCase();
const matches = filterTokens.some((token) => haystack.includes(token));
if (!matches) continue;
}
entries.push(entry);
if (entries.length >= limit) {
break; // Stop once we have collected the requested limit of matching entries
}
} catch {
// Skip unparseable lines
}
}
// Return entries in chronological order (since we collected them newest-first)
const result = entries.reverse(); |
||
|
|
||
| return NextResponse.json(result, { | ||
| status: 200, | ||
| headers: { | ||
| "Cache-Control": "no-store, no-cache, must-revalidate", | ||
| }, | ||
| }); | ||
| } catch (err: unknown) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| return NextResponse.json( | ||
| { error: sanitizeErrorMessage(message) || "Failed to read logs" }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| /** | ||
| * Tests for /api/cli-tools/logs route (fix #2756). | ||
| * | ||
| * Verifies: | ||
| * - GET returns 200 with valid JSON array body. | ||
| * - `filter` param filters log lines by text. | ||
| * - Error responses do NOT leak stack traces (hard rule #12). | ||
| * - log-streamer.ts points to the correct URL (/api/cli-tools/logs). | ||
| * - Non-numeric `limit` param does NOT bypass the 2000-entry cap. | ||
| */ | ||
|
|
||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import fs from "node:fs"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
|
|
||
| import { updateSettings } from "../../src/lib/db/settings"; | ||
|
|
||
| const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cli-logs-route-")); | ||
| process.env.DATA_DIR = TEST_DATA_DIR; | ||
|
|
||
| // Write a small pino-format log file before route is imported | ||
| const logDir = path.join(process.cwd(), "logs", "application"); | ||
| fs.mkdirSync(logDir, { recursive: true }); | ||
| const logPath = path.join(logDir, "app.log"); | ||
| process.env.APP_LOG_FILE_PATH = logPath; | ||
|
Comment on lines
+23
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Writing the test log file directly to // Write a small pino-format log file before route is imported
const logPath = path.join(TEST_DATA_DIR, "app.log");
process.env.APP_LOG_FILE_PATH = logPath; |
||
|
|
||
| const now = Date.now(); | ||
| const lines = [ | ||
| JSON.stringify({ level: 30, msg: "provider connected", component: "router", time: now }), | ||
| JSON.stringify({ level: 40, msg: "rate limit hit", component: "rateLimit", time: now }), | ||
| JSON.stringify({ level: 20, msg: "debug trace output", component: "debug", time: now }), | ||
| "not-valid-json-should-be-skipped", | ||
| ]; | ||
| fs.writeFileSync(logPath, lines.join("\n") + "\n", "utf-8"); | ||
|
|
||
| const { GET } = await import( | ||
| "../../src/app/api/cli-tools/logs/route.ts" | ||
| ); | ||
|
|
||
| test.before(async () => { | ||
| await updateSettings({ requireLogin: false }); | ||
| }); | ||
|
|
||
| test.after(async () => { | ||
| await updateSettings({ requireLogin: true }); | ||
| fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); | ||
| try { | ||
| fs.unlinkSync(logPath); | ||
| } catch { | ||
| // best effort | ||
| } | ||
| }); | ||
|
|
||
| // Helper — make a request; auth passes because requireLogin is set to false in test.before | ||
| function makeReq(queryString = "") { | ||
| const url = `http://localhost/api/cli-tools/logs${queryString ? `?${queryString}` : ""}`; | ||
| return new Request(url); | ||
| } | ||
|
|
||
| test("GET /api/cli-tools/logs returns 200 with JSON array when log file exists", async () => { | ||
| const res = await GET(makeReq()); | ||
| assert.equal(res.status, 200); | ||
|
|
||
| const body = await res.json(); | ||
| assert.ok(Array.isArray(body), "body should be an array"); | ||
| assert.ok(body.length >= 3, `expected at least 3 entries, got ${body.length}`); | ||
| }); | ||
|
|
||
| test("GET /api/cli-tools/logs respects filter param", async () => { | ||
| const res = await GET(makeReq("filter=rateLimit")); | ||
| assert.equal(res.status, 200); | ||
|
|
||
| const body = await res.json(); | ||
| assert.ok(Array.isArray(body)); | ||
| // Only the "rate limit hit" entry matches component=rateLimit | ||
| assert.ok( | ||
| body.every((e: { component?: string; msg?: string }) => { | ||
| const comp = (e.component || "").toLowerCase(); | ||
| const msg = (e.msg || "").toLowerCase(); | ||
| return comp.includes("ratelimit") || msg.includes("ratelimit") || comp.includes("rate"); | ||
| }), | ||
| "filter should restrict results to matching component/message" | ||
| ); | ||
| }); | ||
|
|
||
| test("GET /api/cli-tools/logs returns empty array when log file does not exist", async () => { | ||
| const origPath = process.env.APP_LOG_FILE_PATH; | ||
| process.env.APP_LOG_FILE_PATH = "/tmp/omniroute-nonexistent-cli-logs-test.log"; | ||
|
|
||
| const res = await GET(makeReq()); | ||
| assert.equal(res.status, 200); | ||
| const body = await res.json(); | ||
| assert.ok(Array.isArray(body)); | ||
| assert.equal(body.length, 0); | ||
|
|
||
| process.env.APP_LOG_FILE_PATH = origPath; | ||
| }); | ||
|
|
||
| test("GET /api/cli-tools/logs error response does not leak stack traces (hard rule #12)", async () => { | ||
| // Simulate an internal error path by temporarily breaking the log path to a dir | ||
| const origPath = process.env.APP_LOG_FILE_PATH; | ||
| // Point to a directory so readFileSync throws | ||
| process.env.APP_LOG_FILE_PATH = TEST_DATA_DIR; | ||
|
|
||
| const res = await GET(makeReq()); | ||
| // Should respond with 500 or empty (route may handle gracefully), but must NOT leak stack | ||
| const text = await res.text(); | ||
| assert.ok(!text.includes(" at "), "Response must not contain stack trace frames"); | ||
|
|
||
| process.env.APP_LOG_FILE_PATH = origPath; | ||
| }); | ||
|
|
||
| test("GET /api/cli-tools/logs limit=abc does not bypass the 2000-entry cap", async () => { | ||
| // Write more than 2000 entries so we can verify the cap is applied | ||
| const manyLines: string[] = []; | ||
| for (let i = 0; i < 2100; i++) { | ||
| manyLines.push(JSON.stringify({ level: 30, msg: `entry ${i}`, component: "test", time: now })); | ||
| } | ||
| const origPath = process.env.APP_LOG_FILE_PATH; | ||
| const bigLogPath = path.join(logDir, "big.log"); | ||
| fs.writeFileSync(bigLogPath, manyLines.join("\n") + "\n", "utf-8"); | ||
| process.env.APP_LOG_FILE_PATH = bigLogPath; | ||
|
|
||
| try { | ||
| const res = await GET(makeReq("limit=abc")); | ||
| assert.equal(res.status, 200); | ||
| const body = await res.json(); | ||
| assert.ok(Array.isArray(body), "body should be an array"); | ||
| // Non-numeric limit must fall back to default (500), not bypass the cap with NaN | ||
| assert.ok( | ||
| body.length <= 500, | ||
| `Non-numeric limit=abc should fall back to default 500, got ${body.length}` | ||
| ); | ||
| } finally { | ||
| process.env.APP_LOG_FILE_PATH = origPath; | ||
| fs.unlinkSync(bigLogPath); | ||
| } | ||
| }); | ||
|
|
||
| test("log-streamer.ts calls /api/cli-tools/logs (correct URL, not the missing route)", async () => { | ||
| const { createLogStream } = await import("../../src/lib/cli-helper/log-streamer.ts"); | ||
| // Inspect the source to verify the URL used; we mock fetch to capture it | ||
| const captured: string[] = []; | ||
| const origFetch = globalThis.fetch; | ||
|
|
||
| globalThis.fetch = (async (url: string) => { | ||
| captured.push(typeof url === "string" ? url : String(url)); | ||
| // Return a mock Response with a body so the stream doesn't error immediately | ||
| return new Response(new ReadableStream({ start(c) { c.close(); } }), { status: 200 }); | ||
| }) as typeof fetch; | ||
|
|
||
| try { | ||
| const { stream, stop } = createLogStream({ baseUrl: "http://localhost:20128" }); | ||
| const reader = stream.getReader(); | ||
| // Consume until done (mock stream closes immediately) | ||
| await reader.read().catch(() => {}); | ||
| stop(); | ||
| } finally { | ||
| globalThis.fetch = origFetch; | ||
| } | ||
|
|
||
| assert.ok(captured.length > 0, "fetch should have been called"); | ||
| assert.ok( | ||
| captured.some((u) => u.includes("/api/cli-tools/logs")), | ||
| `Expected /api/cli-tools/logs in fetched URL, got: ${captured[0]}` | ||
| ); | ||
| assert.ok( | ||
| !captured.some((u) => u.includes("/api/logs/console")), | ||
| "log-streamer should not call /api/logs/console" | ||
| ); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent blocking the event loop, use the asynchronous
readFilefromfs/promisesinstead of the synchronousreadFileSync.