diff --git a/apps/studio/pages/cli/login.tsx b/apps/studio/pages/cli/login.tsx index 280ef6793b2..5c9e88f0bc7 100644 --- a/apps/studio/pages/cli/login.tsx +++ b/apps/studio/pages/cli/login.tsx @@ -3,7 +3,7 @@ import { Terminal } from 'lucide-react' import Head from 'next/head' import Link from 'next/link' import { useRouter } from 'next/router' -import { useEffect, useState, type ReactNode } from 'react' +import { useEffect, useRef, useState, type ReactNode } from 'react' import { Button, Card, CardContent } from 'ui' import { Admonition, ShimmeringLoader } from 'ui-patterns' @@ -48,6 +48,19 @@ const CliLoginInterstitial = ({ ) +function getErrorMessage(error: unknown): string { + if (error instanceof Error) return error.message + if ( + typeof error === 'object' && + error !== null && + 'message' in error && + typeof (error as { message: unknown }).message === 'string' + ) { + return (error as { message: string }).message + } + return 'Unknown error' +} + const CliLoginPage: NextPageWithLayout = () => { const router = useRouter() const { session_id, public_key, token_name, device_code } = useParams() @@ -98,6 +111,7 @@ export const CliLoginScreen = ({ }) => { const { profile } = useProfile() const [status, setStatus] = useState({ _tag: 'loading' }) + const startedForSessionIdRef = useRef(undefined) const displayName = profile?.primary_email ?? profile?.username useEffect(() => { @@ -117,6 +131,13 @@ export const CliLoginScreen = ({ return } + // Guard against re-render loops triggered by unstable deps (e.g. a new + // `navigate` reference on each parent render) firing the POST more than + // once per session_id. Without this, the dashboard creates several + // identical access tokens before navigating to the device_code view. + if (startedForSessionIdRef.current === sessionId) return + startedForSessionIdRef.current = sessionId + let isActive = true setStatus({ _tag: 'loading' }) @@ -133,10 +154,7 @@ export const CliLoginScreen = ({ } } catch (error: unknown) { if (!isActive) return - setStatus({ - _tag: 'error', - message: error instanceof Error ? error.message : 'Unknown error', - }) + setStatus({ _tag: 'error', message: getErrorMessage(error) }) } } diff --git a/apps/studio/tests/pages/cli-login.test.tsx b/apps/studio/tests/pages/cli-login.test.tsx index afecbf3c642..d66b2a453d4 100644 --- a/apps/studio/tests/pages/cli-login.test.tsx +++ b/apps/studio/tests/pages/cli-login.test.tsx @@ -119,4 +119,43 @@ describe('CliLoginScreen', () => { expect(await screen.findByText('Unable to create CLI sign-in')).toBeInTheDocument() expect(screen.getByText(/Session expired/)).toBeInTheDocument() }) + + test('surfaces error messages from non-Error rejection shapes (openapi-fetch)', async () => { + createCliLoginSessionMock.mockRejectedValue({ + message: + 'User can have up to 20 personal access tokens. Please remove the excess tokens to create new ones.', + statusCode: 403, + }) + renderScreen() + + expect(await screen.findByText('Unable to create CLI sign-in')).toBeInTheDocument() + expect(screen.getByText(/User can have up to 20 personal access tokens/)).toBeInTheDocument() + expect(screen.queryByText(/Unknown error/)).not.toBeInTheDocument() + }) + + test('POSTs createCliLoginSession exactly once even when parent re-renders', async () => { + createCliLoginSessionMock.mockResolvedValue({ nonce: 'ABCDEFGH12345678' }) + const { rerender } = renderScreen() + + await waitFor(() => { + expect(createCliLoginSessionMock).toHaveBeenCalledTimes(1) + }) + + // Re-render with a brand-new `navigate` prop (mirrors the production + // bug where the parent recreated the closure on every render). + for (let i = 0; i < 5; i++) { + rerender( + + ) + } + + expect(createCliLoginSessionMock).toHaveBeenCalledTimes(1) + }) })