Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ export function useConversationHistory({
context: isOlderPageError
? "conversation_history_older_page"
: "conversation_history_initial",
bestEffort: true,
});

if (!isOlderPageError) {
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/domains/settings/ai/web-search-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export function WebSearchCard() {
} catch (error) {
if (cancelled) return;
setWebSearchHasStoredKey(false);
captureError(error, { context: "settings-ai-web-search-read-secret" });
captureError(error, { context: "settings-ai-web-search-read-secret", bestEffort: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate or retry before suppressing secret-read errors

In platform mode this daemon call can fail with the org-header race that bestEffort now filters, but this effect is a one-shot read: it does not depend on useIsOrgReady() and has no retry/invalidation path when the org store finishes hydrating. In that scenario the catch above leaves webSearchHasStoredKey(false) and the settings card never re-reads the existing key, so suppressing the error here hides a state that can leave the UI incorrectly thinking no key is stored; please gate the request or retry before treating it as best-effort.

Useful? React with 👍 / 👎.

}
})();

Expand Down
97 changes: 96 additions & 1 deletion apps/web/src/lib/sentry/capture-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,109 @@ describe("isExpectedDaemonTransientError", () => {
).toBe(false);
});

test("returns false for non-ApiError instances", () => {
test("returns false for non-ApiError Error instances", () => {
expect(isExpectedDaemonTransientError(new Error("random error"))).toBe(
false,
);
expect(
isExpectedDaemonTransientError(new TypeError("Failed to fetch")),
).toBe(false);
});

test("returns false for primitives and null", () => {
expect(isExpectedDaemonTransientError("string error")).toBe(false);
expect(isExpectedDaemonTransientError(null)).toBe(false);
expect(isExpectedDaemonTransientError(undefined)).toBe(false);
expect(isExpectedDaemonTransientError(42)).toBe(false);
});

// HeyAPI throwOnError throws raw JSON response bodies ({detail: "..."})
// without wrapping in ApiError. These tests verify detection of the raw
// Django REST framework error shape.
test("returns true for raw {detail} with 503 startup message", () => {
expect(
isExpectedDaemonTransientError({
detail: "Your assistant is still starting up. Please try again in a moment.",
}),
).toBe(true);
});

test("returns true for raw {detail} with org-header message", () => {
expect(
isExpectedDaemonTransientError({
detail: "Vellum-Organization-Id header is required.",
}),
).toBe(true);
});

test("returns true for raw {detail} with 401 auth message", () => {
expect(
isExpectedDaemonTransientError({
detail: "Authentication credentials were not provided.",
}),
).toBe(true);
});

test("returns true for raw {detail} with bad gateway message", () => {
expect(
isExpectedDaemonTransientError({ detail: "Bad gateway" }),
).toBe(true);
expect(
isExpectedDaemonTransientError({ detail: "Bad Gateway" }),
).toBe(true);
});

test("returns false for raw {detail} with unknown message", () => {
expect(
isExpectedDaemonTransientError({ detail: "Internal Server Error" }),
).toBe(false);
expect(
isExpectedDaemonTransientError({ detail: "Permission denied." }),
).toBe(false);
});

test("returns false for raw object without detail field", () => {
expect(
isExpectedDaemonTransientError({ message: "something" }),
).toBe(false);
expect(isExpectedDaemonTransientError({})).toBe(false);
});

test("returns false for raw {detail} with non-string value", () => {
expect(
isExpectedDaemonTransientError({ detail: 503 }),
).toBe(false);
expect(
isExpectedDaemonTransientError({ detail: ["error1", "error2"] }),
).toBe(false);
});
});

describe("captureError with bestEffort and raw HeyAPI errors", () => {
test("silently drops raw {detail} daemon transient errors with bestEffort", () => {
captureExceptionMock.mockClear();
captureError(
{ detail: "Your assistant is still starting up. Please try again in a moment." },
{ context: "test-ctx", bestEffort: true },
);
expect(captureExceptionMock).not.toHaveBeenCalled();
});

test("reports raw {detail} daemon transient errors without bestEffort", () => {
captureExceptionMock.mockClear();
captureError(
{ detail: "Your assistant is still starting up." },
{ context: "test-ctx" },
);
expect(captureExceptionMock).toHaveBeenCalledTimes(1);
});

test("reports raw {detail} with unknown message even with bestEffort", () => {
captureExceptionMock.mockClear();
captureError(
{ detail: "Internal Server Error" },
{ context: "test-ctx", bestEffort: true },
);
expect(captureExceptionMock).toHaveBeenCalledTimes(1);
});
});
55 changes: 43 additions & 12 deletions apps/web/src/lib/sentry/capture-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ export function normalizeToError(value: unknown): Error {
return new Error(String(value));
}

/**
* Known daemon transient detail messages. HeyAPI's `throwOnError`
* throws the raw JSON body (`{detail: "..."}`) without the HTTP status
* code, so we fall back to substring matching on the `detail` value.
*
* Each entry is a lowercase substring that uniquely identifies one of
* the daemon's expected transient responses.
*/
const DAEMON_TRANSIENT_DETAILS: readonly string[] = [
"still starting up", // 503 — daemon booting
"organization-id header", // 400 — org store not hydrated
"authentication credentials", // 401 — auth session race
"bad gateway", // 502 — reverse proxy
];

/**
* Detects expected transient HTTP errors from daemon API calls that
* occur during normal startup sequences and auth-session hydration.
Expand All @@ -59,21 +74,37 @@ export function normalizeToError(value: unknown): Error {
* - **400 with org-header message** — Org store has not hydrated yet;
* the `Vellum-Organization-Id` header interceptor read `null`
*
* Only `ApiError` instances are matched. Other error types (TypeError,
* generic Error, plain objects) pass through — they represent network
* failures (handled by `isTransientNetworkError`) or application bugs.
* Matches both structured `ApiError` instances (from manual
* construction with `throwOnError: false`) and raw `{detail: string}`
* objects thrown by HeyAPI's `throwOnError: true`.
*
* Reference: https://heyapi.dev/openapi-ts/clients/fetch#throwing-errors
*/
export function isExpectedDaemonTransientError(error: unknown): boolean {
if (!(error instanceof ApiError)) return false;
if (error.status === 503) return true;
if (error.status === 502) return true;
if (error.status === 401) return true;
if (
error.status === 400 &&
error.message.includes("Organization-Id header")
) {
return true;
if (error instanceof ApiError) {
if (error.status === 503) return true;
if (error.status === 502) return true;
if (error.status === 401) return true;
if (
error.status === 400 &&
error.message.includes("Organization-Id header")
) {
return true;
}
return false;
}

// HeyAPI throwOnError throws the raw JSON response body — typically
// {detail: "message"} for Django REST framework errors — without the
// HTTP status code. Match on known daemon transient detail strings.
if (typeof error === "object" && error !== null && "detail" in error) {
const detail = (error as Record<string, unknown>).detail;
if (typeof detail === "string") {
const lower = detail.toLowerCase();
return DAEMON_TRANSIENT_DETAILS.some((pattern) => lower.includes(pattern));
}
}

return false;
}

Expand Down