mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
Fix: Logout on 401 error in useGitUser; downgrade provider error to warning (#12935)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
113
frontend/__tests__/hooks/use-git-user.test.tsx
Normal file
113
frontend/__tests__/hooks/use-git-user.test.tsx
Normal file
@@ -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<typeof useLogout>;
|
||||
|
||||
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<typeof useLogout>;
|
||||
|
||||
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 }) => (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
{children}
|
||||
</QueryClientProvider>
|
||||
);
|
||||
};
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
@@ -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__),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user