diff --git a/web/src/lib/fetcher.ts b/web/src/lib/fetcher.ts index 537ccd661b..d44ff3461a 100644 --- a/web/src/lib/fetcher.ts +++ b/web/src/lib/fetcher.ts @@ -33,6 +33,13 @@ const DEFAULT_AUTH_ERROR_MSG = const DEFAULT_ERROR_MSG = "An error occurred while fetching the data."; +export function isAuthStatusError(error: unknown): boolean { + return ( + error instanceof FetchError && + (error.status === 401 || error.status === 402 || error.status === 403) + ); +} + /** * SWR `onErrorRetry` callback that suppresses automatic retries for * auth or tier-gated errors (401/402/403). Pass this to any SWR hook whose @@ -42,11 +49,7 @@ const DEFAULT_ERROR_MSG = "An error occurred while fetching the data."; export const skipRetryOnAuthError: NonNullable< import("swr").SWRConfiguration["onErrorRetry"] > = (error, _key, _config, revalidate, { retryCount }) => { - if ( - error instanceof FetchError && - (error.status === 401 || error.status === 402 || error.status === 403) - ) - return; + if (isAuthStatusError(error)) return; // For non-auth errors, retry with exponential backoff if ( _config.errorRetryCount !== undefined && diff --git a/web/src/providers/UserProvider.test.tsx b/web/src/providers/UserProvider.test.tsx new file mode 100644 index 0000000000..513241de87 --- /dev/null +++ b/web/src/providers/UserProvider.test.tsx @@ -0,0 +1,172 @@ +// Guards the /api/me contract: unresolved reads as loading and failed auth fetches self-heal. +import { act, render, screen } from "@tests/setup/test-utils"; +// Relative import: jest's moduleNameMapper swaps the @/ path for the global stub. +import { UserProvider, useUser } from "./UserProvider"; +import { FetchError } from "@/lib/fetcher"; +import { useCurrentUser } from "@/lib/users/hooks"; +import { User } from "@/lib/types"; + +jest.mock("posthog-js/react", () => ({ usePostHog: () => undefined })); +jest.mock("@/lib/settings/hooks", () => ({ useSettings: () => ({}) })); +jest.mock("@/lib/users/hooks", () => ({ useCurrentUser: jest.fn() })); +jest.mock("@/lib/auth/hooks", () => ({ + useAuthTypeMetadata: () => ({ + authTypeMetadata: undefined, + isLoading: false, + }), + useTokenRefresh: jest.fn(), +})); +jest.mock("@/lib/users/svc", () => ({ + updateUserPersonalization: jest.fn(), + setUserDefaultModel: jest.fn(), +})); +jest.mock("next-themes", () => ({ + useTheme: () => ({ theme: "light", setTheme: jest.fn() }), +})); + +const mockedUseCurrentUser = jest.mocked(useCurrentUser); + +function setCurrentUser(overrides: Partial>) { + mockedUseCurrentUser.mockReturnValue({ + user: undefined, + isLoading: true, + mutateUser: jest.fn(), + userError: undefined, + ...overrides, + } as ReturnType); +} + +function Probe() { + const { user, userResolution } = useUser(); + if (userResolution !== "resolved") { + return {userResolution}; + } + return {user ? user.email : "signed-out"}; +} + +// Factory: an identical element reference would let React skip the re-render. +function probeTree() { + return ( + + + + ); +} + +function renderProbe() { + return render(probeTree()); +} + +describe("UserProvider user resolution", () => { + it("reports loading while /api/me is unresolved", () => { + setCurrentUser({ user: undefined }); + renderProbe(); + expect(screen.getByText("loading")).toBeInTheDocument(); + }); + + it("reports loading when /api/me failed instead of signed-out", () => { + setCurrentUser({ + user: undefined, + isLoading: false, + userError: new Error("transient"), + }); + renderProbe(); + expect(screen.getByText("loading")).toBeInTheDocument(); + }); + + it("reports signed-out only for a resolved null user", () => { + setCurrentUser({ user: null, isLoading: false }); + renderProbe(); + expect(screen.getByText("signed-out")).toBeInTheDocument(); + }); + + it("exposes the user once resolved", () => { + setCurrentUser({ + user: { id: "u1", email: "john@example.com" } as User, + isLoading: false, + }); + renderProbe(); + expect(screen.getByText("john@example.com")).toBeInTheDocument(); + }); +}); + +function authError(label: string): FetchError { + return new FetchError(label, 403, null); +} + +describe("UserProvider /api/me retry", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("retries an auth failure on a backoff schedule, then reports unavailable", () => { + const mutateUser = jest.fn(); + + setCurrentUser({ userError: authError("fail 1"), mutateUser }); + const { rerender } = renderProbe(); + act(() => jest.advanceTimersByTime(2_000)); + expect(mutateUser).toHaveBeenCalledTimes(1); + + setCurrentUser({ userError: authError("fail 2"), mutateUser }); + rerender(probeTree()); + act(() => jest.advanceTimersByTime(5_000)); + expect(mutateUser).toHaveBeenCalledTimes(2); + + setCurrentUser({ userError: authError("fail 3"), mutateUser }); + rerender(probeTree()); + act(() => jest.advanceTimersByTime(15_000)); + expect(mutateUser).toHaveBeenCalledTimes(3); + + setCurrentUser({ userError: authError("fail 4"), mutateUser }); + rerender(probeTree()); + act(() => jest.advanceTimersByTime(120_000)); + expect(mutateUser).toHaveBeenCalledTimes(3); + // Budget spent: unavailable, never a false signed-out claim. + expect(screen.getByText("unavailable")).toBeInTheDocument(); + }); + + it("does not schedule retries for non-auth failures (SWR owns those)", () => { + const mutateUser = jest.fn(); + + setCurrentUser({ + userError: new FetchError("boom", 500, null), + mutateUser, + }); + renderProbe(); + act(() => jest.advanceTimersByTime(29_000)); + expect(mutateUser).not.toHaveBeenCalled(); + expect(screen.getByText("loading")).toBeInTheDocument(); + }); + + it("reports unavailable once a non-auth failure outlives the deadline", () => { + setCurrentUser({ userError: new FetchError("boom", 500, null) }); + renderProbe(); + act(() => jest.advanceTimersByTime(31_000)); + expect(screen.getByText("unavailable")).toBeInTheDocument(); + }); + + it("resets the retry budget after a successful fetch", () => { + const mutateUser = jest.fn(); + + setCurrentUser({ userError: authError("fail"), mutateUser }); + const { rerender } = renderProbe(); + act(() => jest.advanceTimersByTime(2_000)); + expect(mutateUser).toHaveBeenCalledTimes(1); + + setCurrentUser({ + user: { id: "u1", email: "john@example.com" } as User, + isLoading: false, + mutateUser, + }); + rerender(probeTree()); + + setCurrentUser({ userError: authError("fail again"), mutateUser }); + rerender(probeTree()); + act(() => jest.advanceTimersByTime(2_000)); + expect(mutateUser).toHaveBeenCalledTimes(2); + }); +}); diff --git a/web/src/providers/UserProvider.tsx b/web/src/providers/UserProvider.tsx index 2950afe454..dd5094480a 100644 --- a/web/src/providers/UserProvider.tsx +++ b/web/src/providers/UserProvider.tsx @@ -16,6 +16,7 @@ import { ThemePreference, } from "@/lib/types"; import { usePostHog } from "posthog-js/react"; +import { isAuthStatusError } from "@/lib/fetcher"; import { useSettings } from "@/lib/settings/hooks"; import { useCurrentUser } from "@/lib/users/hooks"; import { useAuthTypeMetadata, useTokenRefresh } from "@/lib/auth/hooks"; @@ -26,8 +27,18 @@ import { } from "@/lib/users/svc"; import { useTheme } from "next-themes"; +// Auth failures skip SWR's retry but are usually transient refresh races. SWR's backoff owns the rest. +const ME_RETRY_DELAYS_MS = [2_000, 5_000, 15_000]; + +// Ceiling on reporting loading for a failing /api/me, so the account menu always comes back. +const ME_LOADING_DEADLINE_MS = 30_000; + +/** Only "resolved" lets a null user mean signed out. "unavailable" means /api/me keeps failing for a possibly valid session. */ +export type UserResolution = "loading" | "unavailable" | "resolved"; + interface UserContextType { user: User | null; + userResolution: UserResolution; isAdmin: boolean; isCurator: boolean; refreshUser: () => Promise; @@ -61,7 +72,7 @@ interface UserContextType { const UserContext = createContext(undefined); export function UserProvider({ children }: { children: React.ReactNode }) { - const { user: fetchedUser, mutateUser } = useCurrentUser(); + const { user: fetchedUser, mutateUser, userError } = useCurrentUser(); const { authTypeMetadata, isLoading: authTypeMetadataLoading } = useAuthTypeMetadata(); const updatedSettingsData = useSettings(); @@ -99,6 +110,55 @@ export function UserProvider({ children }: { children: React.ReactNode }) { setUpToDateUser(mergeUserPreferences(fetchedUser ?? null)); }, [fetchedUser, mergeUserPreferences]); + const [meRetriesExhausted, setMeRetriesExhausted] = useState(false); + + const meRetryCountRef = useRef(0); + const meFirstErrorAtRef = useRef(null); + useEffect(() => { + if (!userError) { + meRetryCountRef.current = 0; + meFirstErrorAtRef.current = null; + setMeRetriesExhausted(false); + return; + } + meFirstErrorAtRef.current ??= Date.now(); + if (!isAuthStatusError(userError)) { + // SWR's backoff owns non-auth retries. Bound only how long we report loading. + const remaining = + ME_LOADING_DEADLINE_MS - (Date.now() - meFirstErrorAtRef.current); + if (remaining <= 0) { + setMeRetriesExhausted(true); + return; + } + const deadlineId = setTimeout( + () => setMeRetriesExhausted(true), + remaining + ); + return () => clearTimeout(deadlineId); + } + // Fresh error identity per failure advances the schedule. The count moves in the timer, so StrictMode is safe. + const attempt = meRetryCountRef.current; + const delay = ME_RETRY_DELAYS_MS[attempt]; + if (delay === undefined) { + setMeRetriesExhausted(true); + return; + } + const timeoutId = setTimeout(() => { + meRetryCountRef.current = attempt + 1; + void mutateUser(); + }, delay); + return () => clearTimeout(timeoutId); + }, [userError, mutateUser]); + + const awaitingMe = fetchedUser === undefined && !meRetriesExhausted; + const mergePending = fetchedUser != null && upToDateUser === null; + const userResolution: UserResolution = + awaitingMe || mergePending + ? "loading" + : fetchedUser === undefined + ? "unavailable" + : "resolved"; + useEffect(() => { if (!posthog) return; @@ -539,6 +599,7 @@ export function UserProvider({ children }: { children: React.ReactNode }) {
diff --git a/web/src/sections/sidebar/AccountPopover.test.tsx b/web/src/sections/sidebar/AccountPopover.test.tsx new file mode 100644 index 0000000000..caec42688a --- /dev/null +++ b/web/src/sections/sidebar/AccountPopover.test.tsx @@ -0,0 +1,76 @@ +// The chip shows a skeleton while unresolved and "Anonymous" only when resolved signed-out. +import { render, screen } from "@tests/setup/test-utils"; +import AccountPopover from "@/sections/sidebar/AccountPopover"; +import { useUser } from "@/providers/UserProvider"; +import { User } from "@/lib/types"; + +// Factory mock: the global stub pins userResolution to "resolved". +jest.mock("@/providers/UserProvider", () => ({ useUser: jest.fn() })); +jest.mock("next/navigation", () => ({ + useRouter: () => ({ push: jest.fn() }), + usePathname: () => "/app", + useSearchParams: () => new URLSearchParams(), +})); +jest.mock("@/sections/sidebar/NotificationsPopover", () => ({ + __esModule: true, + default: () => null, +})); +jest.mock("@/hooks/useAppFocus", () => ({ + __esModule: true, + default: () => ({ isUserSettings: () => false }), +})); +jest.mock("@/hooks/useScreenSize", () => ({ + __esModule: true, + default: () => ({ isMobile: false }), +})); +jest.mock("@/lib/settings/hooks", () => ({ + useSettings: () => ({ vectorDbEnabled: false }), +})); +jest.mock("@/hooks/useNotifications", () => ({ + useNotificationSummary: () => ({ undismissedCount: 0, refresh: jest.fn() }), +})); + +const mockedUseUser = jest.mocked(useUser); + +function setUser( + user: User | null, + userResolution: "loading" | "unavailable" | "resolved" +) { + mockedUseUser.mockReturnValue({ + user, + userResolution, + } as ReturnType); +} + +it("shows a skeleton instead of Anonymous while the user is unresolved", () => { + setUser(null, "loading"); + render(); + expect(screen.queryByText("Anonymous")).not.toBeInTheDocument(); + expect(screen.queryByRole("button")).not.toBeInTheDocument(); +}); + +it("shows a neutral label, not Anonymous, when the user is unavailable", () => { + setUser(null, "unavailable"); + render(); + expect(screen.getByText("Account")).toBeInTheDocument(); + expect(screen.queryByText("Anonymous")).not.toBeInTheDocument(); +}); + +it("shows Anonymous for a resolved signed-out user", () => { + setUser(null, "resolved"); + render(); + expect(screen.getByText("Anonymous")).toBeInTheDocument(); +}); + +it("shows the user's name once resolved", () => { + setUser( + { + id: "u1", + email: "john@example.com", + personalization: { name: "John" }, + } as unknown as User, + "resolved" + ); + render(); + expect(screen.getByText("John")).toBeInTheDocument(); +}); diff --git a/web/src/sections/sidebar/AccountPopover.tsx b/web/src/sections/sidebar/AccountPopover.tsx index a486aeaebd..58fcde4186 100644 --- a/web/src/sections/sidebar/AccountPopover.tsx +++ b/web/src/sections/sidebar/AccountPopover.tsx @@ -30,6 +30,7 @@ import useAppFocus from "@/hooks/useAppFocus"; import useScreenSize from "@/hooks/useScreenSize"; import { useSettings } from "@/lib/settings/hooks"; import UserAvatar from "@/refresh-components/avatars/UserAvatar"; +import SidebarTabSkeleton from "@/refresh-components/skeletons/SidebarTabSkeleton"; import { useNotificationSummary } from "@/hooks/useNotifications"; import { SvgOnyxLogo } from "@opal/logos"; import { markdown } from "@opal/utils"; @@ -45,7 +46,7 @@ function SettingsPopover({ onOpenNotifications, undismissedCount, }: SettingsPopoverProps) { - const { user } = useUser(); + const { user, userResolution } = useUser(); const settings = useSettings(); const enterpriseSettings = settings.enterprise; const router = useRouter(); @@ -90,7 +91,14 @@ function SettingsPopover({ {[
- +
, null,
@@ -198,13 +206,14 @@ export default function AccountPopover({ const [popupState, setPopupState] = useState< "Settings" | "Notifications" | undefined >(undefined); - const { user } = useUser(); + const { user, userResolution } = useUser(); const appFocus = useAppFocus(); const { isMobile } = useScreenSize(); const { vectorDbEnabled } = useSettings(); const { undismissedCount, refresh: refreshNotificationSummary } = useNotificationSummary(); - const userDisplayName = getUserDisplayName(user); + const userDisplayName = + userResolution === "unavailable" ? "Account" : getUserDisplayName(user); const handlePopoverOpen = (state: boolean) => { if (state) { @@ -221,6 +230,9 @@ export default function AccountPopover({ setPopupState(undefined); } }; + if (userResolution === "loading") { + return ; + } return ( diff --git a/web/tests/setup/mocks/components/UserProvider.tsx b/web/tests/setup/mocks/components/UserProvider.tsx index ccf8d652cb..e4916f20c7 100644 --- a/web/tests/setup/mocks/components/UserProvider.tsx +++ b/web/tests/setup/mocks/components/UserProvider.tsx @@ -19,6 +19,7 @@ import React, { createContext, useContext } from "react"; interface UserContextType { user: any; + userResolution: "loading" | "unavailable" | "resolved"; isAdmin: boolean; isCurator: boolean; refreshUser: () => Promise; @@ -36,6 +37,7 @@ interface UserContextType { const mockUserContext: UserContextType = { user: null, + userResolution: "resolved", isAdmin: false, isCurator: false, refreshUser: async () => {},