diff --git a/AGENTS.md b/AGENTS.md index 7ce37fd66..b5333dc06 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -631,6 +631,9 @@ mock.module("./some-module", () => ({ * **CLI telemetry DSN is public write-only — safe to embed in install script**: The CLI's Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key already baked into every binary. Safe to hardcode in install scripts. Opt-out: \`SENTRY\_CLI\_NO\_TELEMETRY=1\`. + +* **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Authentication in \`src/lib/db/auth.ts\` follows layered precedence: \`SENTRY\_AUTH\_TOKEN\` env var > \`SENTRY\_TOKEN\` env var > SQLite-stored OAuth token. \`getEnvToken()\` reads env vars with \`.trim()\` (empty/whitespace = unset). \`AuthSource\` type tracks provenance: \`"env:SENTRY\_AUTH\_TOKEN"\` | \`"env:SENTRY\_TOKEN"\` | \`"oauth"\`. \`ENV\_SOURCE\_PREFIX = "env:"\` constant is used for parsing source strings (use \`.length\` not hardcoded 4). \`getActiveEnvVarName()\` is the shared helper for commands needing the env var name — calls \`getEnvToken()\` directly (no DB fallback). Env tokens bypass all refresh/expiry logic. \`isEnvTokenActive()\` is the guard for auth commands. Logout must NOT clear stored auth when env token active — just inform user to unset the env var. \`getEnvToken\`/\`isEnvTokenActive\` stay in \`db/auth.ts\` despite not touching DB, because they're tightly coupled with \`getAuthToken\`/\`getAuthConfig\`/\`refreshToken\`. + * **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow. diff --git a/src/commands/log/list.ts b/src/commands/log/list.ts index 4afc82242..d060d5b72 100644 --- a/src/commands/log/list.ts +++ b/src/commands/log/list.ts @@ -90,8 +90,9 @@ function parseFollow(value: string): number { */ type LogLike = { timestamp: string; - /** Nanosecond-precision timestamp used for dedup in follow mode */ - timestamp_precise: number; + /** Nanosecond-precision timestamp used for dedup in follow mode. + * Optional because TraceLog may omit it when the API response doesn't include it. */ + timestamp_precise?: number; severity?: string | null; message?: string | null; trace?: string | null; @@ -194,6 +195,11 @@ type FollowConfig = { fetch: (statsPeriod: string, afterTimestamp?: number) => Promise; /** Extract only the genuinely new entries from a poll response */ extractNew: (logs: T[], lastTimestamp: number) => T[]; + /** + * Called with the initial batch of logs before polling begins. + * Use this to seed dedup state (e.g., tracking seen log IDs). + */ + onInitialLogs?: (logs: T[]) => void; }; /** @@ -252,23 +258,38 @@ function executeFollowMode( pendingTimer = setTimeout(poll, pollIntervalMs); } - function writeNewLogs(newLogs: T[]) { - const newestLog = newLogs[0]; - if (newestLog) { - if (!(flags.json || headerPrinted)) { - stdout.write(table ? table.header() : formatLogsHeader()); - headerPrinted = true; + /** Find the highest timestamp_precise in a batch, or undefined if none have it. */ + function maxTimestamp(logs: T[]): number | undefined { + let max: number | undefined; + for (const l of logs) { + if (l.timestamp_precise !== undefined) { + max = + max === undefined + ? l.timestamp_precise + : Math.max(max, l.timestamp_precise); } - const chronological = [...newLogs].reverse(); - writeLogs({ - stdout, - logs: chronological, - asJson: flags.json, - table, - includeTrace: config.includeTrace, - }); - lastTimestamp = newestLog.timestamp_precise; } + return max; + } + + function writeNewLogs(newLogs: T[]) { + if (newLogs.length === 0) { + return; + } + + if (!(flags.json || headerPrinted)) { + stdout.write(table ? table.header() : formatLogsHeader()); + headerPrinted = true; + } + const chronological = [...newLogs].reverse(); + writeLogs({ + stdout, + logs: chronological, + asJson: flags.json, + table, + includeTrace: config.includeTrace, + }); + lastTimestamp = maxTimestamp(newLogs) ?? lastTimestamp; } async function poll() { @@ -311,9 +332,8 @@ function executeFollowMode( table, includeTrace: config.includeTrace, }); - if (initialLogs[0]) { - lastTimestamp = initialLogs[0].timestamp_precise; - } + lastTimestamp = maxTimestamp(initialLogs) ?? lastTimestamp; + config.onInitialLogs?.(initialLogs); scheduleNextPoll(); }) .catch((error: unknown) => { @@ -451,6 +471,9 @@ export const listCommand = buildListCommand("log", { if (flags.follow) { const traceId = flags.trace; + // Track IDs of logs seen without timestamp_precise so they are + // shown once but not duplicated on subsequent polls. + const seenWithoutTs = new Set(); await executeFollowMode({ stdout, stderr, @@ -464,7 +487,24 @@ export const listCommand = buildListCommand("log", { statsPeriod, }), extractNew: (logs, lastTs) => - logs.filter((l) => l.timestamp_precise > lastTs), + logs.filter((l) => { + if (l.timestamp_precise !== undefined) { + return l.timestamp_precise > lastTs; + } + // No precise timestamp — deduplicate by id + if (seenWithoutTs.has(l.id)) { + return false; + } + seenWithoutTs.add(l.id); + return true; + }), + onInitialLogs: (logs) => { + for (const l of logs) { + if (l.timestamp_precise === undefined) { + seenWithoutTs.add(l.id); + } + } + }, }); } else { await executeTraceSingleFetch(stdout, org, flags.trace, flags); diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index 020139867..a83988f3b 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -359,9 +359,14 @@ export async function apiRequestToRegion( if (schema) { const result = schema.safeParse(data); if (!result.success) { - // Treat schema validation failures as API errors so they surface cleanly - // through the central error handler rather than showing a raw ZodError - // stack trace. This guards against unexpected API response format changes. + // Attach structured Zod issues to the Sentry event so we can diagnose + // exactly which field(s) failed validation — the ApiError.detail string + // alone may not be visible in the Sentry issue overview. + Sentry.setContext("zod_validation", { + endpoint, + status: response.status, + issues: result.error.issues.slice(0, 10), + }); throw new ApiError( `Unexpected response format from ${endpoint}`, response.status, diff --git a/src/types/sentry.ts b/src/types/sentry.ts index ee2294e59..b4d9599e7 100644 --- a/src/types/sentry.ts +++ b/src/types/sentry.ts @@ -480,8 +480,10 @@ export const SentryLogSchema = z "sentry.item_id": z.string(), /** ISO timestamp of the log entry */ timestamp: z.string(), - /** Nanosecond-precision timestamp for accurate ordering and filtering */ - timestamp_precise: z.number(), + /** Nanosecond-precision timestamp for accurate ordering and filtering. + * Coerced from string because the API may return large integers as strings + * to avoid precision loss beyond Number.MAX_SAFE_INTEGER. */ + timestamp_precise: z.coerce.number(), /** Log message content */ message: z.string().nullable().optional(), /** Log severity level (error, warning, info, debug, etc.) */ @@ -516,8 +518,10 @@ export const DetailedSentryLogSchema = z "sentry.item_id": z.string(), /** ISO timestamp of the log entry */ timestamp: z.string(), - /** Nanosecond-precision timestamp for accurate ordering */ - timestamp_precise: z.number(), + /** Nanosecond-precision timestamp for accurate ordering. + * Coerced from string because the API may return large integers as strings + * to avoid precision loss beyond Number.MAX_SAFE_INTEGER. */ + timestamp_precise: z.coerce.number(), /** Log message content */ message: z.string().nullable().optional(), /** Log severity level (error, warning, info, debug, etc.) */ @@ -584,18 +588,24 @@ export const TraceLogSchema = z .object({ /** Unique identifier for this log entry */ id: z.string(), - /** Numeric ID of the project this log belongs to */ - "project.id": z.number(), + /** Numeric ID of the project this log belongs to. + * Coerced from string because some API responses return numeric IDs as strings. */ + "project.id": z.coerce.number(), /** The 32-character hex trace ID this log is associated with */ trace: z.string(), - /** Numeric OTel severity level (e.g., 9 = INFO, 13 = WARN, 17 = ERROR) */ - severity_number: z.number(), + /** Numeric OTel severity level (e.g., 9 = INFO, 13 = WARN, 17 = ERROR). + * Optional because not all log entries include this field. + * Coerced from string for resilience against API format variations. */ + severity_number: z.coerce.number().optional(), /** Severity label (e.g., "info", "warn", "error") */ severity: z.string(), /** ISO 8601 timestamp */ timestamp: z.string(), - /** High-precision timestamp in nanoseconds */ - timestamp_precise: z.number(), + /** High-precision timestamp in nanoseconds. + * Optional because some API responses may omit it. + * Coerced from string because nanosecond timestamps (≈1.7e18 in 2026) + * exceed Number.MAX_SAFE_INTEGER and APIs may return them as strings. */ + timestamp_precise: z.coerce.number().optional(), /** Log message content */ message: z.string().nullable().optional(), }) diff --git a/test/commands/log/list.test.ts b/test/commands/log/list.test.ts index 0ee17951e..489e7270a 100644 --- a/test/commands/log/list.test.ts +++ b/test/commands/log/list.test.ts @@ -784,7 +784,8 @@ describe("listCommand.func — follow mode (standard)", () => { }); test("passes afterTimestamp to poll calls", async () => { - // sampleLogs[0] has the highest timestamp_precise (newest-first from API) + // maxTimestamp scans the entire batch for the highest timestamp_precise + const maxTs = Math.max(...sampleLogs.map((l) => l.timestamp_precise)); listLogsSpy.mockResolvedValueOnce(sampleLogs).mockResolvedValueOnce([]); resolveOrgProjectSpy.mockResolvedValue({ org: ORG, project: PROJECT }); @@ -797,10 +798,10 @@ describe("listCommand.func — follow mode (standard)", () => { sigint.trigger(); await promise; - // Poll call (index 1) should include afterTimestamp from first log + // Poll call (index 1) should include afterTimestamp from max in batch const pollCall = listLogsSpy.mock.calls[1]; expect(pollCall).toBeDefined(); - expect(pollCall[2].afterTimestamp).toBe(sampleLogs[0].timestamp_precise); + expect(pollCall[2].afterTimestamp).toBe(maxTs); expect(pollCall[2].statsPeriod).toBe("10m"); }); diff --git a/test/lib/api-client.test.ts b/test/lib/api-client.test.ts index 237d3f9ac..dff9b3f79 100644 --- a/test/lib/api-client.test.ts +++ b/test/lib/api-client.test.ts @@ -1574,4 +1574,79 @@ describe("listTraceLogs", () => { const url = new URL(capturedUrl); expect(url.searchParams.get("statsPeriod")).toBe("14d"); }); + + test("coerces string numeric fields to numbers (API resilience)", async () => { + const stringFieldsData = { + data: [ + { + id: "log001", + "project.id": "123", + trace: "aaaa1111bbbb2222cccc3333dddd4444", + severity_number: "9", + severity: "info", + timestamp: "2025-01-30T14:32:15+00:00", + timestamp_precise: "1738247535000000000", + message: "Request received", + }, + ], + meta: { fields: { id: "string" } }, + }; + + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { + const req = new Request(input, init); + if (req.url.includes("/trace-logs/")) { + return new Response(JSON.stringify(stringFieldsData), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response(JSON.stringify({}), { status: 200 }); + }; + + const result = await listTraceLogs( + "my-org", + "aaaa1111bbbb2222cccc3333dddd4444" + ); + expect(result).toHaveLength(1); + expect(typeof result[0]["project.id"]).toBe("number"); + expect(result[0]["project.id"]).toBe(123); + expect(typeof result[0].severity_number).toBe("number"); + expect(result[0].severity_number).toBe(9); + expect(typeof result[0].timestamp_precise).toBe("number"); + }); + + test("accepts responses with missing optional fields", async () => { + const minimalData = { + data: [ + { + id: "log001", + "project.id": 123, + trace: "aaaa1111bbbb2222cccc3333dddd4444", + severity: "info", + timestamp: "2025-01-30T14:32:15+00:00", + message: "Test", + }, + ], + }; + + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { + const req = new Request(input, init); + if (req.url.includes("/trace-logs/")) { + return new Response(JSON.stringify(minimalData), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response(JSON.stringify({}), { status: 200 }); + }; + + const result = await listTraceLogs( + "my-org", + "aaaa1111bbbb2222cccc3333dddd4444" + ); + expect(result).toHaveLength(1); + expect(result[0].severity_number).toBeUndefined(); + expect(result[0].timestamp_precise).toBeUndefined(); + expect(result[0].severity).toBe("info"); + }); }); diff --git a/test/lib/trace-log-schema.property.test.ts b/test/lib/trace-log-schema.property.test.ts new file mode 100644 index 000000000..7b0787b8a --- /dev/null +++ b/test/lib/trace-log-schema.property.test.ts @@ -0,0 +1,176 @@ +/** + * Property-Based Tests for TraceLogSchema + * + * Verifies that `TraceLogSchema` (and related log schemas) correctly + * coerce string-typed numeric fields and accept optional fields, + * guarding against API response format variations (CLI-BH). + */ + +import { describe, expect, test } from "bun:test"; +import { + constant, + assert as fcAssert, + nat, + oneof, + option, + property, + string, + tuple, +} from "fast-check"; +import { + DetailedSentryLogSchema, + SentryLogSchema, + TraceLogSchema, + TraceLogsResponseSchema, +} from "../../src/types/sentry.js"; +import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; + +/** + * Arbitrary that produces a value as either a number or its string + * representation — simulating APIs that may return numeric fields + * as strings (especially for large nanosecond timestamps). + */ +function numberOrString(arb = nat()) { + return oneof(arb, arb.map(String)); +} + +/** + * Arbitrary that produces a number as either the number itself, its + * string representation, or undefined — simulating optional numeric + * fields that the API may omit or serialize as strings. + */ +function optionalNumberOrString(arb = nat()) { + return oneof(arb, arb.map(String), constant(undefined)); +} + +/** Arbitrary for a valid trace-log entry with mixed number/string numeric fields */ +const traceLogEntryArb = tuple( + numberOrString(), + optionalNumberOrString(), + optionalNumberOrString(), + option(string(), { nil: undefined }) +).map(([projectId, sevNum, tsPrecise, msg]) => ({ + id: "test-log-id", + "project.id": projectId, + trace: "aaaa1111bbbb2222cccc3333dddd4444", + severity_number: sevNum, + severity: "info", + timestamp: "2025-01-30T14:32:15+00:00", + timestamp_precise: tsPrecise, + message: msg ?? null, +})); + +describe("property: TraceLogSchema coercion", () => { + test("always parses successfully with number or string numeric fields", () => { + fcAssert( + property(traceLogEntryArb, (entry) => { + const result = TraceLogSchema.safeParse(entry); + expect(result.success).toBe(true); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("coerced output has number types for numeric fields (when present)", () => { + fcAssert( + property(traceLogEntryArb, (entry) => { + const result = TraceLogSchema.safeParse(entry); + if (!result.success) return; + + expect(typeof result.data["project.id"]).toBe("number"); + + if (result.data.severity_number !== undefined) { + expect(typeof result.data.severity_number).toBe("number"); + } + if (result.data.timestamp_precise !== undefined) { + expect(typeof result.data.timestamp_precise).toBe("number"); + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("preserves passthrough fields", () => { + const entry = { + id: "test", + "project.id": 1, + trace: "aaaa1111bbbb2222cccc3333dddd4444", + severity: "info", + timestamp: "2025-01-30T14:32:15+00:00", + extra_field: "should be preserved", + }; + const result = TraceLogSchema.safeParse(entry); + expect(result.success).toBe(true); + if (result.success) { + expect((result.data as Record).extra_field).toBe( + "should be preserved" + ); + } + }); +}); + +describe("property: TraceLogsResponseSchema", () => { + test("accepts response with mixed string/number fields in data array", () => { + fcAssert( + property(traceLogEntryArb, (entry) => { + const response = { data: [entry] }; + const result = TraceLogsResponseSchema.safeParse(response); + expect(result.success).toBe(true); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("accepts response with empty data array", () => { + const result = TraceLogsResponseSchema.safeParse({ data: [] }); + expect(result.success).toBe(true); + }); + + test("accepts response with optional meta", () => { + const result = TraceLogsResponseSchema.safeParse({ + data: [], + meta: { fields: { id: "string" }, units: {} }, + }); + expect(result.success).toBe(true); + }); +}); + +describe("property: SentryLogSchema coercion", () => { + test("coerces string timestamp_precise to number", () => { + fcAssert( + property(numberOrString(), (tsPrecise) => { + const entry = { + "sentry.item_id": "test-id", + timestamp: "2025-01-30T14:32:15+00:00", + timestamp_precise: tsPrecise, + }; + const result = SentryLogSchema.safeParse(entry); + expect(result.success).toBe(true); + if (result.success) { + expect(typeof result.data.timestamp_precise).toBe("number"); + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); + +describe("property: DetailedSentryLogSchema coercion", () => { + test("coerces string timestamp_precise to number", () => { + fcAssert( + property(numberOrString(), (tsPrecise) => { + const entry = { + "sentry.item_id": "test-id", + timestamp: "2025-01-30T14:32:15+00:00", + timestamp_precise: tsPrecise, + }; + const result = DetailedSentryLogSchema.safeParse(entry); + expect(result.success).toBe(true); + if (result.success) { + expect(typeof result.data.timestamp_precise).toBe("number"); + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +});