mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-19 01:50:51 -05:00
fix(web): stop rendering "Anonymous" while the current user is unresolved (#13852)
This commit is contained in:
1 parent
74a6878deb
commit
0d8ca41972
7 files changed
+340
-12
No files matched your search
@@ -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 &&
|
||||
|
||||
@@ -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<ReturnType<typeof useCurrentUser>>) {
|
||||
mockedUseCurrentUser.mockReturnValue({
|
||||
user: undefined,
|
||||
isLoading: true,
|
||||
mutateUser: jest.fn(),
|
||||
userError: undefined,
|
||||
...overrides,
|
||||
} as ReturnType<typeof useCurrentUser>);
|
||||
}
|
||||
|
||||
function Probe() {
|
||||
const { user, userResolution } = useUser();
|
||||
if (userResolution !== "resolved") {
|
||||
return <span>{userResolution}</span>;
|
||||
}
|
||||
return <span>{user ? user.email : "signed-out"}</span>;
|
||||
}
|
||||
|
||||
// Factory: an identical element reference would let React skip the re-render.
|
||||
function probeTree() {
|
||||
return (
|
||||
<UserProvider>
|
||||
<Probe />
|
||||
</UserProvider>
|
||||
);
|
||||
}
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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<void>;
|
||||
@@ -61,7 +72,7 @@ interface UserContextType {
|
||||
const UserContext = createContext<UserContextType | undefined>(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<number | null>(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 }) {
|
||||
<UserContext.Provider
|
||||
value={{
|
||||
user: upToDateUser,
|
||||
userResolution,
|
||||
refreshUser,
|
||||
authTypeMetadata,
|
||||
updateUserAutoScroll,
|
||||
|
||||
@@ -2,18 +2,20 @@ import { cn } from "@opal/utils";
|
||||
|
||||
interface SidebarTabSkeletonProps {
|
||||
textWidth?: string;
|
||||
folded?: boolean;
|
||||
}
|
||||
|
||||
export default function SidebarTabSkeleton({
|
||||
textWidth = "w-2/3",
|
||||
folded,
|
||||
}: SidebarTabSkeletonProps) {
|
||||
return (
|
||||
<div className="w-full rounded-08 p-1.5">
|
||||
<div className="h-6 flex flex-row items-center px-1 py-0.5">
|
||||
<div
|
||||
className={cn(
|
||||
"h-3 rounded-sm bg-background-tint-04 animate-pulse",
|
||||
textWidth
|
||||
"bg-background-tint-04 animate-pulse",
|
||||
folded ? "h-4 w-4 rounded-full" : cn("h-3 rounded-sm", textWidth)
|
||||
)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -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<typeof useUser>);
|
||||
}
|
||||
|
||||
it("shows a skeleton instead of Anonymous while the user is unresolved", () => {
|
||||
setUser(null, "loading");
|
||||
render(<AccountPopover />);
|
||||
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(<AccountPopover />);
|
||||
expect(screen.getByText("Account")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Anonymous")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows Anonymous for a resolved signed-out user", () => {
|
||||
setUser(null, "resolved");
|
||||
render(<AccountPopover />);
|
||||
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(<AccountPopover />);
|
||||
expect(screen.getByText("John")).toBeInTheDocument();
|
||||
});
|
||||
@@ -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({
|
||||
<PopoverMenu>
|
||||
{[
|
||||
<div key="user-email" className="p-2">
|
||||
<Content sizePreset="main-ui" title={getUserEmail(user)} />
|
||||
<Content
|
||||
sizePreset="main-ui"
|
||||
title={
|
||||
userResolution === "unavailable"
|
||||
? "Profile unavailable"
|
||||
: getUserEmail(user)
|
||||
}
|
||||
/>
|
||||
</div>,
|
||||
null,
|
||||
<div key="user-settings" data-testid="Settings/user-settings">
|
||||
@@ -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 <SidebarTabSkeleton folded={folded} />;
|
||||
}
|
||||
|
||||
return (
|
||||
<Popover open={!!popupState} onOpenChange={handlePopoverOpen}>
|
||||
|
||||
@@ -19,6 +19,7 @@ import React, { createContext, useContext } from "react";
|
||||
|
||||
interface UserContextType {
|
||||
user: any;
|
||||
userResolution: "loading" | "unavailable" | "resolved";
|
||||
isAdmin: boolean;
|
||||
isCurator: boolean;
|
||||
refreshUser: () => Promise<void>;
|
||||
@@ -36,6 +37,7 @@ interface UserContextType {
|
||||
|
||||
const mockUserContext: UserContextType = {
|
||||
user: null,
|
||||
userResolution: "resolved",
|
||||
isAdmin: false,
|
||||
isCurator: false,
|
||||
refreshUser: async () => {},
|
||||
|
||||
Reference in new issue
Block a user