From a6c0d80fe1b4cc9598f3b436614f5bc96d6506ec Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 23 Feb 2026 17:15:54 +0000 Subject: [PATCH] Fix: Logout on 401 error in useGitUser; downgrade provider error to warning (#12935) Co-authored-by: openhands --- .../__tests__/hooks/use-git-user.test.tsx | 113 ++++++++++++++++++ frontend/src/hooks/query/use-git-user.ts | 10 ++ openhands/integrations/provider.py | 2 +- 3 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 frontend/__tests__/hooks/use-git-user.test.tsx diff --git a/frontend/__tests__/hooks/use-git-user.test.tsx b/frontend/__tests__/hooks/use-git-user.test.tsx new file mode 100644 index 0000000000..950e476c22 --- /dev/null +++ b/frontend/__tests__/hooks/use-git-user.test.tsx @@ -0,0 +1,113 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import React from "react"; +import { useGitUser } from "#/hooks/query/use-git-user"; +import { useLogout } from "#/hooks/mutation/use-logout"; +import UserService from "#/api/user-service/user-service.api"; +import * as useShouldShowUserFeaturesModule from "#/hooks/use-should-show-user-features"; +import * as useConfigModule from "#/hooks/query/use-config"; +import { AxiosError } from "axios"; + +vi.mock("#/hooks/use-should-show-user-features"); +vi.mock("#/hooks/query/use-config"); +vi.mock("#/hooks/mutation/use-logout"); +vi.mock("#/api/user-service/user-service.api"); +vi.mock("posthog-js/react", () => ({ + usePostHog: vi.fn(() => ({ + identify: vi.fn(), + })), +})); + +describe("useGitUser", () => { + let mockLogout: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + + mockLogout = { + mutate: vi.fn(), + mutateAsync: vi.fn(), + data: undefined, + error: null, + isPending: false, + isSuccess: false, + isError: false, + isIdle: true, + reset: vi.fn(), + status: "idle", + } as unknown as ReturnType; + + vi.mocked(useShouldShowUserFeaturesModule.useShouldShowUserFeatures).mockReturnValue(true); + vi.mocked(useConfigModule.useConfig).mockReturnValue({ + data: { app_mode: "saas" }, + isLoading: false, + error: null, + } as any); + vi.mocked(useLogout).mockReturnValue(mockLogout); + }); + + const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + + return ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + }; + + it("should call logout when receiving a 401 error", async () => { + // Mock the user service to throw a 401 error + const mockError = new AxiosError("Unauthorized", "401", undefined, undefined, { + status: 401, + data: { message: "Unauthorized" }, + } as any); + + vi.mocked(UserService.getUser).mockRejectedValue(mockError); + + const { result } = renderHook(() => useGitUser(), { + wrapper: createWrapper(), + }); + + // Wait for the query to fail (status becomes 'error') + await waitFor(() => { + expect(result.current.status).toBe("error"); + }); + + // Wait for the useEffect to trigger logout + await waitFor(() => { + expect(mockLogout.mutate).toHaveBeenCalled(); + }); + }); + + it("should not call logout for non-401 errors", async () => { + // Mock the user service to throw a 500 error + const mockError = new AxiosError("Server Error", "500", undefined, undefined, { + status: 500, + data: { message: "Internal Server Error" }, + } as any); + + vi.mocked(UserService.getUser).mockRejectedValue(mockError); + + const { result } = renderHook(() => useGitUser(), { + wrapper: createWrapper(), + }); + + // Wait for the query to fail (status becomes 'error') + await waitFor(() => { + expect(result.current.status).toBe("error"); + }); + + // Wait a bit to ensure logout is not called + await waitFor(() => { + expect(mockLogout.mutate).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/frontend/src/hooks/query/use-git-user.ts b/frontend/src/hooks/query/use-git-user.ts index bc18e086b8..971999f25c 100644 --- a/frontend/src/hooks/query/use-git-user.ts +++ b/frontend/src/hooks/query/use-git-user.ts @@ -4,10 +4,12 @@ import { usePostHog } from "posthog-js/react"; import { useConfig } from "./use-config"; import UserService from "#/api/user-service/user-service.api"; import { useShouldShowUserFeatures } from "#/hooks/use-should-show-user-features"; +import { useLogout } from "../mutation/use-logout"; export const useGitUser = () => { const posthog = usePostHog(); const { data: config } = useConfig(); + const logout = useLogout(); // Use the shared hook to determine if we should fetch user data const shouldFetchUser = useShouldShowUserFeatures(); @@ -33,5 +35,13 @@ export const useGitUser = () => { } }, [user.data]); + // If we get a 401 here, it means that the integration tokens need to be + // refreshed. Since this happens at login, we log out. + React.useEffect(() => { + if (user?.error?.response?.status === 401) { + logout.mutate(); + } + }, [user.status]); + return user; }; diff --git a/openhands/integrations/provider.py b/openhands/integrations/provider.py index 446e8c0a5e..9ddd963c81 100644 --- a/openhands/integrations/provider.py +++ b/openhands/integrations/provider.py @@ -172,7 +172,7 @@ class ProviderHandler: exceptions.append((provider, e)) continue for provider, exc in exceptions: - logger.error( + logger.warning( f'Failed to get user from provider {provider}: {exc}', exc_info=(type(exc), exc, exc.__traceback__), )