Skip to content
Merged
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
11 changes: 11 additions & 0 deletions apps/web/docs/CONVENTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,17 @@ pass additional indexed tags via `tags` and unindexed metadata via `extra`.
Toast/UI display stays at the call site — `captureError` never shows
user-facing UI.

For **best-effort background fetches** (imperative daemon calls that fire
optimistically and have natural retry surfaces like SSE reconnect or
navigation), pass `bestEffort: true`. This additionally filters expected
daemon transient errors (503 startup, 502 bad-gateway, 401 auth-race,
400 org-header-not-ready) while still reporting unexpected errors (500,
data-integrity, programming errors) to Sentry:

```ts
captureError(err, { context: "useActiveConversation.refreshRow", bestEffort: true });
```

**Do not use bare `Sentry.captureException`.** The only exceptions are
framework-level integration points that need raw Sentry scope
manipulation: `RouteErrorBoundary`, `RouterProvider.onError`, and
Expand Down
23 changes: 23 additions & 0 deletions apps/web/src/domains/chat/hooks/use-active-conversation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type { Conversation } from "@/types/conversation-types";
let foregroundImpl: Conversation[] = [];
let backgroundImpl: Conversation[] = [];
let scheduledImpl: Conversation[] = [];
let isOrgReadyImpl = true;
const refreshConversationRowCalls: Array<{
assistantId: string | null;
conversationId: string;
Expand All @@ -34,6 +35,10 @@ mock.module("@/hooks/conversation-queries", () => ({
useScheduledConversationListQuery: () => ({ conversations: scheduledImpl }),
}));

mock.module("@/hooks/use-is-org-ready", () => ({
useIsOrgReady: () => isOrgReadyImpl,
}));

mock.module("@/utils/conversation-cache-mutations", () => ({
refreshConversationRow: async (
_queryClient: unknown,
Expand Down Expand Up @@ -67,6 +72,7 @@ beforeEach(() => {
foregroundImpl = [];
backgroundImpl = [];
scheduledImpl = [];
isOrgReadyImpl = true;
refreshConversationRowCalls.length = 0;
});

Expand Down Expand Up @@ -154,4 +160,21 @@ describe("useActiveConversation", () => {
await Promise.resolve();
expect(refreshConversationRowCalls).toHaveLength(0);
});

test("does not fetch when org is not ready", async () => {
// GIVEN the org store has not hydrated yet
isOrgReadyImpl = false;
foregroundImpl = [];
backgroundImpl = [];

// WHEN the hook runs with org not ready
renderHook(
() => useActiveConversation("asst-1", "bg-unloaded", true),
{ wrapper },
);

// THEN no fetch is issued (prevents 400 org-header errors)
await Promise.resolve();
expect(refreshConversationRowCalls).toHaveLength(0);
});
});
14 changes: 9 additions & 5 deletions apps/web/src/domains/chat/hooks/use-active-conversation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { useEffect, useMemo, useRef } from "react";
import { captureError } from "@/lib/sentry/capture-error";
import { useQueryClient } from "@tanstack/react-query";

import { useIsOrgReady } from "@/hooks/use-is-org-ready";

import type { Conversation } from "@/types/conversation-types";

import {
Expand All @@ -34,6 +36,7 @@ export function useActiveConversation(
enabled: boolean,
): Conversation | undefined {
const queryClient = useQueryClient();
const isOrgReady = useIsOrgReady();
const { conversations: foreground } = useConversationListQuery(
assistantId,
enabled,
Expand Down Expand Up @@ -63,7 +66,7 @@ export function useActiveConversation(

const fetchedConversationIdRef = useRef<string | null>(null);
useEffect(() => {
if (!enabled || !assistantId || !conversationId) {
if (!enabled || !assistantId || !conversationId || !isOrgReady) {
return;
}
if (activeConversation) {
Expand All @@ -78,12 +81,13 @@ export function useActiveConversation(
assistantId,
conversationId,
).catch((error) => {
// Clear the guard so a later dependency change can retry a row that
// failed to fetch transiently.
fetchedConversationIdRef.current = null;
captureError(error, { context: "useActiveConversation.refreshRow" });
captureError(error, {
context: "useActiveConversation.refreshRow",
bestEffort: true,
});
});
}, [enabled, assistantId, conversationId, activeConversation, queryClient]);
}, [enabled, assistantId, conversationId, activeConversation, queryClient, isOrgReady]);

return activeConversation;
}
2 changes: 1 addition & 1 deletion apps/web/src/hooks/use-conversation-sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function handleConversationSyncTags(
).catch((err: unknown) => {
captureError(err, {
context: "useConversationSync.refreshRow",
level: "warning",
bestEffort: true,
extra: {
assistantId,
conversationId: parsed.conversationId,
Expand Down
91 changes: 88 additions & 3 deletions apps/web/src/lib/sentry/capture-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ mock.module("@sentry/react", () => ({
captureException: captureExceptionMock,
}));

const { captureError, normalizeToError } = await import(
"@/lib/sentry/capture-error"
);
const { captureError, normalizeToError, isExpectedDaemonTransientError } =
await import("@/lib/sentry/capture-error");
const { ApiError } = await import("@/utils/api-errors");

describe("normalizeToError", () => {
test("returns Error instances unchanged", () => {
Expand Down Expand Up @@ -111,4 +111,89 @@ describe("captureError", () => {
captureError(new TypeError("Failed to fetch"), { context: "test-ctx" });
expect(captureExceptionMock).not.toHaveBeenCalled();
});

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

test("silently drops daemon transient errors with bestEffort flag", () => {
captureExceptionMock.mockClear();
captureError(new ApiError(503, "Your assistant is still starting up."), {
context: "test-ctx",
bestEffort: true,
});
expect(captureExceptionMock).not.toHaveBeenCalled();
});

test("reports unexpected ApiError even with bestEffort flag", () => {
captureExceptionMock.mockClear();
captureError(new ApiError(500, "Internal Server Error"), {
context: "test-ctx",
bestEffort: true,
});
expect(captureExceptionMock).toHaveBeenCalledTimes(1);
});
});

describe("isExpectedDaemonTransientError", () => {
test("returns true for 503 daemon starting up", () => {
expect(
isExpectedDaemonTransientError(
new ApiError(503, "Your assistant is still starting up."),
),
).toBe(true);
});

test("returns true for 502 bad gateway", () => {
expect(
isExpectedDaemonTransientError(new ApiError(502, "Bad gateway")),
).toBe(true);
});

test("returns true for 401 auth race", () => {
expect(
isExpectedDaemonTransientError(
new ApiError(401, "Authentication credentials were not provided."),
),
).toBe(true);
});

test("returns true for 400 org-header missing", () => {
expect(
isExpectedDaemonTransientError(
new ApiError(400, "Vellum-Organization-Id header is required."),
),
).toBe(true);
});

test("returns false for 400 without org-header message", () => {
expect(
isExpectedDaemonTransientError(
new ApiError(400, "Invalid request body."),
),
).toBe(false);
});

test("returns false for 500 internal server error", () => {
expect(
isExpectedDaemonTransientError(
new ApiError(500, "Internal Server Error"),
),
).toBe(false);
});

test("returns false for non-ApiError instances", () => {
expect(isExpectedDaemonTransientError(new Error("random error"))).toBe(
false,
);
expect(
isExpectedDaemonTransientError(new TypeError("Failed to fetch")),
).toBe(false);
expect(isExpectedDaemonTransientError("string error")).toBe(false);
expect(isExpectedDaemonTransientError(null)).toBe(false);
});
});
39 changes: 39 additions & 0 deletions apps/web/src/lib/sentry/capture-error.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Sentry from "@sentry/react";

import { ApiError } from "@/utils/api-errors";
import { isTransientNetworkError } from "@/utils/is-transient-network-error";

/**
Expand Down Expand Up @@ -45,6 +46,37 @@ export function normalizeToError(value: unknown): Error {
return new Error(String(value));
}

/**
* Detects expected transient HTTP errors from daemon API calls that
* occur during normal startup sequences and auth-session hydration.
*
* These are valid HTTP responses (not browser-level network failures)
* that indicate the daemon or its infrastructure is not yet ready:
*
* - **503** — Daemon still starting up ("Your assistant is still starting up")
* - **502** — Reverse proxy cannot reach the daemon pod yet
* - **401** — Auth session not yet established (race during login)
* - **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.
*/
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;
}
return false;
}

/**
* Captures a non-transient error to Sentry with structured tags.
*
Expand All @@ -53,6 +85,11 @@ export function normalizeToError(value: unknown): Error {
* and the app handles them gracefully via TanStack Query retries,
* best-effort sync patterns, and error-state UI.
*
* When `bestEffort` is `true`, expected daemon transient HTTP errors
* (503/502/401/400-org-header) are also silently dropped. Use this for
* background fetches that fire optimistically and have natural retry
* surfaces (SSE reconnect, dependency-change re-renders, navigation).
*
* Non-Error values (e.g. HeyAPI response bodies thrown by
* `throwOnError: true`) are normalized into `Error` instances before
* capture so Sentry produces useful titles and grouping.
Expand All @@ -72,9 +109,11 @@ export function captureError(
level?: Sentry.SeverityLevel;
tags?: Record<string, string>;
extra?: Record<string, unknown>;
bestEffort?: boolean;
},
): void {
if (isTransientNetworkError(error)) return;
if (opts.bestEffort && isExpectedDaemonTransientError(error)) return;
console.error(`[${opts.context}]`, error);

const normalized = normalizeToError(error);
Expand Down