From 7331a839803790e8a14d1e5d9a8441266b1e7891 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Fri, 12 Jun 2026 08:01:39 +0000 Subject: [PATCH 1/2] fix(logs): replace .parse() with .safeParse() to prevent ZodError crash on self-hosted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listLogs() and getLogsBatch() used raw .parse() which throws ZodError when the API returns a non-object response (e.g. HTML from a reverse proxy or plain text from an unsupported self-hosted endpoint). This was the only API module using .parse() — all others use .safeParse() via apiRequestToRegion's schema option. Added safeParseResponse() helper that: 1. Pre-checks typeof data for early, descriptive errors 2. Uses .safeParse() + wraps failures in ApiError 3. Attaches Zod issues to Sentry context for diagnostics Fixes #1095 --- src/lib/api/logs.ts | 57 +++++++++++++- test/lib/api/logs.test.ts | 158 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 2 deletions(-) create mode 100644 test/lib/api/logs.test.ts diff --git a/src/lib/api/logs.ts b/src/lib/api/logs.ts index 35085962d..f1a6a70fb 100644 --- a/src/lib/api/logs.ts +++ b/src/lib/api/logs.ts @@ -6,6 +6,9 @@ */ import { queryExploreEventsInTableFormat } from "@sentry/api"; +// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import +import * as Sentry from "@sentry/node-core/light"; +import type { z } from "zod"; import { DetailedLogsResponseSchema, @@ -16,6 +19,7 @@ import { type TraceLog, TraceLogsResponseSchema, } from "../../types/index.js"; +import { ApiError } from "../errors.js"; import { resolveOrgRegion } from "../region.js"; import { LOG_RETENTION_PERIOD } from "../retention.js"; import { isAllDigits } from "../utils.js"; @@ -45,6 +49,47 @@ const LOG_FIELDS = [ "message", ]; +/** + * Validate that the API returned an object before attempting Zod parsing. + * Self-hosted instances may return plain text or HTML when the logs dataset + * is unsupported or a reverse proxy intercepts the request. + */ +function assertObjectResponse(data: unknown, context: string): void { + if (typeof data !== "object" || data === null) { + throw new ApiError( + `${context}: unexpected response format`, + 0, + `Expected an object but received ${typeof data}. ` + + "This may indicate an incompatible self-hosted Sentry version or a proxy interfering with the response." + ); + } +} + +/** + * Safe-parse an API response with a Zod schema, throwing {@link ApiError} + * on validation failure instead of leaking a raw `ZodError`. + */ +function safeParseResponse( + schema: z.ZodType, + data: unknown, + context: string +): T { + assertObjectResponse(data, context); + const result = schema.safeParse(data); + if (!result.success) { + Sentry.setContext("zod_validation", { + context, + issues: result.error.issues.slice(0, 10), + }); + throw new ApiError( + `${context}: unexpected response format`, + 0, + result.error.message + ); + } + return result.data; +} + type ListLogsOptions = { /** Search query using Sentry query syntax */ query?: string; @@ -128,7 +173,11 @@ export async function listLogs( }); const data = unwrapResult(result, "Failed to list logs"); - const logsResponse = LogsResponseSchema.parse(data); + const logsResponse = safeParseResponse( + LogsResponseSchema, + data, + "Failed to list logs" + ); return logsResponse.data; } @@ -191,7 +240,11 @@ async function getLogsBatch( }); const data = unwrapResult(result, "Failed to get log"); - const logsResponse = DetailedLogsResponseSchema.parse(data); + const logsResponse = safeParseResponse( + DetailedLogsResponseSchema, + data, + "Failed to get log" + ); return logsResponse.data; } diff --git a/test/lib/api/logs.test.ts b/test/lib/api/logs.test.ts new file mode 100644 index 000000000..1ab8ec4d1 --- /dev/null +++ b/test/lib/api/logs.test.ts @@ -0,0 +1,158 @@ +/** + * Tests for listLogs and getLogs — guards against non-object SDK responses. + * + * CLI-20C: self-hosted instances can return non-object data (plain text, HTML) + * from the /events/?dataset=logs endpoint when the logs dataset is unsupported + * or a reverse proxy intercepts the request. Previously this crashed with an + * unhandled ZodError; now it throws a descriptive ApiError. + */ + +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { getLogs, listLogs } from "../../../src/lib/api/logs.js"; +import { setAuthToken } from "../../../src/lib/db/auth.js"; +import { ApiError } from "../../../src/lib/errors.js"; +import { mockFetch, useTestConfigDir } from "../../helpers.js"; + +useTestConfigDir("logs-api-test-"); + +let originalFetch: typeof globalThis.fetch; + +beforeEach(async () => { + originalFetch = globalThis.fetch; + await setAuthToken("fake-token-for-test", 3600); +}); + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +/** + * Mock fetch to return a fixed JSON body for all requests. + * The SDK parses the response via response.json(), so wrapping in + * JSON.stringify ensures the SDK receives the raw value as `data`. + */ +function mockOk(body: unknown) { + globalThis.fetch = mockFetch( + async () => + new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); +} + +describe("listLogs", () => { + test("returns logs when API returns a valid response", async () => { + mockOk({ + data: [ + { + "sentry.item_id": "log-001", + timestamp: "2025-01-30T14:32:15+00:00", + timestamp_precise: 1_770_060_419_044_800_300, + message: "Test log message", + severity: "info", + trace: "abc123def456abc123def456abc12345", + }, + ], + meta: { fields: {} }, + }); + + const logs = await listLogs("test-org", "test-project"); + expect(logs).toHaveLength(1); + expect(logs[0]["sentry.item_id"]).toBe("log-001"); + }); + + test("throws ApiError when API returns a string instead of object", async () => { + mockOk("Proxy error: upstream not found"); + + await expect(listLogs("test-org", "test-project")).rejects.toThrow( + ApiError + ); + + try { + await listLogs("test-org", "test-project"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + const apiError = error as ApiError; + expect(apiError.message).toContain("unexpected response format"); + expect(apiError.detail).toContain("received string"); + } + }); + + test("throws ApiError when API returns null", async () => { + mockOk(null); + + await expect(listLogs("test-org", "test-project")).rejects.toThrow( + ApiError + ); + }); + + test("throws ApiError when response has wrong shape", async () => { + mockOk({ wrong: "shape" }); + + await expect(listLogs("test-org", "test-project")).rejects.toThrow( + ApiError + ); + + try { + await listLogs("test-org", "test-project"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).message).toContain( + "unexpected response format" + ); + } + }); +}); + +describe("getLogs", () => { + test("returns logs when API returns a valid detailed response", async () => { + mockOk({ + data: [ + { + "sentry.item_id": "log-001", + timestamp: "2025-01-30T14:32:15+00:00", + timestamp_precise: 1_770_060_419_044_800_300, + message: "Test log message", + severity: "info", + trace: "abc123def456abc123def456abc12345", + project: "test-project", + environment: "production", + release: "1.0.0", + "sdk.name": "sentry.javascript.node", + "sdk.version": "8.0.0", + span_id: "abc123def456abc1", + "code.function": "main", + "code.file.path": "/app/index.ts", + "code.line.number": "42", + "sentry.otel.kind": "INTERNAL", + "sentry.otel.status_code": "OK", + "sentry.otel.instrumentation_scope.name": "my-app", + }, + ], + meta: { fields: {} }, + }); + + const logs = await getLogs("test-org", "test-project", ["log-001"]); + expect(logs).toHaveLength(1); + expect(logs[0]["sentry.item_id"]).toBe("log-001"); + }); + + test("throws ApiError when API returns a string instead of object", async () => { + mockOk("502 Bad Gateway"); + + await expect( + getLogs("test-org", "test-project", ["log-001"]) + ).rejects.toThrow(ApiError); + + try { + await getLogs("test-org", "test-project", ["log-001"]); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + const apiError = error as ApiError; + expect(apiError.message).toContain("unexpected response format"); + expect(apiError.detail).toContain("received string"); + expect(apiError.detail).toContain("self-hosted"); + } + }); +}); From cb4324d9745962691af3b4ac9d8e64e64f46d319 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Tue, 16 Jun 2026 03:25:06 +0000 Subject: [PATCH 2/2] fix: handle typeof null in assertObjectResponse error message --- src/lib/api/logs.ts | 2 +- test/lib/api/logs.test.ts | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lib/api/logs.ts b/src/lib/api/logs.ts index f1a6a70fb..d128091f2 100644 --- a/src/lib/api/logs.ts +++ b/src/lib/api/logs.ts @@ -59,7 +59,7 @@ function assertObjectResponse(data: unknown, context: string): void { throw new ApiError( `${context}: unexpected response format`, 0, - `Expected an object but received ${typeof data}. ` + + `Expected an object but received ${data === null ? "null" : typeof data}. ` + "This may indicate an incompatible self-hosted Sentry version or a proxy interfering with the response." ); } diff --git a/test/lib/api/logs.test.ts b/test/lib/api/logs.test.ts index 1ab8ec4d1..0e3b03914 100644 --- a/test/lib/api/logs.test.ts +++ b/test/lib/api/logs.test.ts @@ -85,6 +85,15 @@ describe("listLogs", () => { await expect(listLogs("test-org", "test-project")).rejects.toThrow( ApiError ); + + try { + await listLogs("test-org", "test-project"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + const apiError = error as ApiError; + expect(apiError.message).toContain("unexpected response format"); + expect(apiError.detail).toContain("received null"); + } }); test("throws ApiError when response has wrong shape", async () => {