Harden profile creation logic from dashboard (#51483)

## Context

When signing up a new account on the dashboard, the dashboard's the one
making the request to create a new profile for the user (ideally not,
but the proper fix on the BE is currently blocked). We do so by making
an initial request to GET `/profile`, and if it returns a 404, it's
implied that the account doesn't exist and we fire `createProfile`

Because of this, a race condition could exist (due to replica lag for
example):
- GET `/profile` returns 404, treat user as new
- POST `/profile` returns 200
- Profile query is still in "error" state (`isError` is `true), and post
sign-in tries to redirect to the new org page which triggers the
`useEfect`
[here](https://github.com/supabase/supabase/blob/master/apps/studio/lib/profile.tsx#L82-L96)
- Which prompts another POST `/profile`, and it returns a 409 because
the account's already created
- This then keeps going in a loop which keeps calling `createProfile`
and we didn't have a way to handle this, leading to users in a stuck
loading state until the browser's refreshed

## Changes involved
- Opt to invalidate the profile, permissions, and organizations if the
user runs into a 409
- This should refresh the profile and we'll no longer be stuck in a
create loop since `isError` should no longer be `true` then
- Tighten the dependency array for where `createProfile` is triggered
- Add exponential backoff up to 3 times for profile creation for
transient failures (5xx, network, 429)
- Never retry profile creation for 409, since there's no point doing so

apps/studio/data/profile/profile-create-mutation.ts

@@ -28,6 +28,17 @@ export const useProfileCreateMutation = ({
 
   return useMutation<ProfileCreateData, ResponseError, void>({
     mutationFn: () => createProfile(),
+    // Retry transient failures (5xx, network, 429), but never 4xx - a 409 means the profile
+    // already exists, so retrying the POST can't help
+    retry: (failureCount, error) => {
+      const isClientError = error.code !== undefined && error.code >= 400 && error.code < 500
+      if (isClientError && error.code !== 429) return false
+      return failureCount < 3
+    },
+    retryDelay: (failureCount, error) => {
+      if (error.code === 429 && error.retryAfter) return error.retryAfter * 1000
+      return Math.min(1000 * 2 ** failureCount, 8000)
+    },
     async onSuccess(data, variables, context) {
       await Promise.all([
         queryClient.invalidateQueries({ queryKey: profileKeys.profile() }),

apps/studio/lib/profile.test.tsx

@@ -0,0 +1,142 @@
+import { screen, waitFor } from '@testing-library/react'
+import { HttpResponse } from 'msw'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { ProfileProvider, useProfile } from './profile'
+import type { ProfileResponse } from '@/data/profile/profile-create-mutation'
+import { customRender } from '@/tests/lib/custom-render'
+import { addAPIMock, type APIErrorBody } from '@/tests/lib/msw'
+import { createMockProfile } from '@/tests/lib/profile-helpers'
+
+vi.mock('common', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('common')>()
+  return {
+    ...actual,
+    useParams: () => ({ ref: 'default' }),
+    useUser: () => ({ id: 'user-1', email: 'test@example.com' }),
+    useIsLoggedIn: () => true,
+  }
+})
+
+vi.mock('@sentry/nextjs', () => ({ captureMessage: vi.fn(), captureException: vi.fn() }))
+vi.mock('@/lib/telemetry/track', () => ({ useTrack: () => vi.fn() }))
+
+const profileResponse = (username?: string) =>
+  HttpResponse.json<ProfileResponse>(createMockProfile({ username }))
+const errorResponse = (message: string, status: number) =>
+  HttpResponse.json<APIErrorBody>({ message }, { status })
+
+const ProfileStatus = () => {
+  const { profile, isSuccess } = useProfile()
+  return <div>{isSuccess ? `profile:${profile?.username}` : 'no-profile'}</div>
+}
+
+const renderProvider = () =>
+  customRender(
+    <ProfileProvider>
+      <ProfileStatus />
+    </ProfileProvider>
+  )
+
+// Serves each GET response in order, repeating the last one
+const mockProfileGet = (responses: Array<'not-found' | 'found'>) => {
+  const getSpy = vi.fn()
+  addAPIMock({
+    method: 'get',
+    path: '/platform/profile',
+    response: () => {
+      const next = responses[Math.min(getSpy.mock.calls.length, responses.length - 1)]
+      getSpy()
+      return next === 'found'
+        ? profileResponse('new-user')
+        : errorResponse("User's profile not found", 404)
+    },
+  })
+  return getSpy
+}
+
+describe('ProfileProvider profile creation', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('creates the profile once when it does not exist', async () => {
+    const getSpy = mockProfileGet(['not-found', 'found'])
+    const postSpy = vi.fn()
+    addAPIMock({
+      method: 'post',
+      path: '/platform/profile',
+      response: () => {
+        postSpy()
+        return profileResponse()
+      },
+    })
+
+    renderProvider()
+
+    await waitFor(() => expect(screen.getByText('profile:new-user')).toBeInTheDocument())
+    expect(postSpy).toHaveBeenCalledTimes(1)
+    expect(getSpy).toHaveBeenCalledTimes(2)
+  })
+
+  it('refetches the profile when creation returns 409', async () => {
+    mockProfileGet(['not-found', 'found'])
+    const postSpy = vi.fn()
+    addAPIMock({
+      method: 'post',
+      path: '/platform/profile',
+      response: () => {
+        postSpy()
+        return errorResponse('Profile already exists', 409)
+      },
+    })
+
+    renderProvider()
+
+    await waitFor(() => expect(screen.getByText('profile:new-user')).toBeInTheDocument())
+    expect(postSpy).toHaveBeenCalledTimes(1)
+  })
+
+  it('does not loop POSTing when the profile is still not found after a 409', async () => {
+    const getSpy = mockProfileGet(['not-found'])
+    const postSpy = vi.fn()
+    addAPIMock({
+      method: 'post',
+      path: '/platform/profile',
+      response: () => {
+        postSpy()
+        return errorResponse('Profile already exists', 409)
+      },
+    })
+
+    renderProvider()
+
+    // Initial GET, then the refetch triggered by the 409
+    await waitFor(() => expect(getSpy).toHaveBeenCalledTimes(2))
+    await new Promise((resolve) => setTimeout(resolve, 300))
+    expect(postSpy).toHaveBeenCalledTimes(1)
+    expect(getSpy).toHaveBeenCalledTimes(2)
+  })
+
+  it('retries creation on a server error', async () => {
+    mockProfileGet(['not-found', 'found'])
+    const postSpy = vi.fn()
+    addAPIMock({
+      method: 'post',
+      path: '/platform/profile',
+      response: () => {
+        postSpy()
+        return postSpy.mock.calls.length === 1
+          ? errorResponse('Internal error', 500)
+          : profileResponse()
+      },
+    })
+
+    renderProvider()
+
+    await waitFor(() => expect(screen.getByText('profile:new-user')).toBeInTheDocument(), {
+      timeout: 5000,
+    })
+    expect(postSpy).toHaveBeenCalledTimes(2)
+  })
+})

apps/studio/lib/profile.tsx

@@ -1,4 +1,5 @@
 import * as Sentry from '@sentry/nextjs'
+import { useQueryClient } from '@tanstack/react-query'
 import { useIsLoggedIn, useUser } from 'common'
 import { useRouter } from 'next/router'
 import { createContext, PropsWithChildren, useContext, useEffect, useMemo } from 'react'
@@ -6,7 +7,10 @@ import { toast } from 'sonner'
 
 import { useSignOut } from './auth'
 import { getGitHubProfileImgUrl } from './github'
+import { organizationKeys } from '@/data/organizations/keys'
+import { permissionKeys } from '@/data/permissions/keys'
 import { usePermissionsQuery } from '@/data/permissions/permissions-query'
+import { profileKeys } from '@/data/profile/keys'
 import { useProfileCreateMutation } from '@/data/profile/profile-create-mutation'
 import { useProfileIdentitiesQuery } from '@/data/profile/profile-identities-query'
 import { useProfileQuery } from '@/data/profile/profile-query'
@@ -32,6 +36,7 @@ export const ProfileContext = createContext<ProfileContextType>({
 
 export const ProfileProvider = ({ children }: PropsWithChildren<{}>) => {
   const user = useUser()
+  const queryClient = useQueryClient()
   const isLoggedIn = useIsLoggedIn()
   const router = useRouter()
   const signOut = useSignOut()
@@ -53,14 +58,13 @@ export const ProfileProvider = ({ children }: PropsWithChildren<{}>) => {
     },
     onError: (error) => {
       if (error.code === 409) {
-        // [Joshen] There's currently an assumption that createProfile is getting triggered
-        // multiple times unnecessarily, although the tracing the code i can't see why this might
-        // be happening unless GET profile is somehow returning `User's profile not found` incorrectly
-        // Adding a Sentry capture + toast in hopes to catch this while developing on local / staging
+        // The profile already exists (e.g. GET hit a lagging replica, or it was created
+        // elsewhere), so the earlier "not found" is stale. Refetch rather than leaving the
+        // user stuck on the cached error. Sentry capture is kept to monitor how often this occurs.
         Sentry.captureMessage('Profile already exists: ' + error.message)
-        if (process.env.NEXT_PUBLIC_ENVIRONMENT !== 'prod') {
-          toast.error('[DEV] createProfile called despite profile already exists: ' + error.message)
-        }
+        queryClient.invalidateQueries({ queryKey: profileKeys.profile() })
+        queryClient.invalidateQueries({ queryKey: organizationKeys.list() })
+        queryClient.invalidateQueries({ queryKey: permissionKeys.list() })
       } else {
         Sentry.captureMessage('Failed to create users profile: ' + error.message)
         toast.error('Failed to create your profile. Please refresh to try again.')
@@ -79,21 +83,21 @@ export const ProfileProvider = ({ children }: PropsWithChildren<{}>) => {
     enabled: isLoggedIn,
   })
 
+  const isProfileNotFound = isError && error?.message === "User's profile not found"
   useEffect(() => {
-    if (!isError) return
-    // if the user does not yet exist, create a profile for them
-    if (error?.message === "User's profile not found") {
-      createProfile()
-    }
+    if (!isProfileNotFound || !user?.id) return
+    createProfile()
+  }, [isProfileNotFound, user?.id, createProfile])
 
-    // [Alaister] If the user has a bad auth token, auth-js won't know about it
-    // and will think the user is authenticated. Since fetching the profile happens
-    // on every page load, we can check for a 401 here and sign the user out if
-    // they have a bad token.
-    if (error?.code === 401) {
-      signOut().then(() => router.push('/sign-in'))
-    }
-  }, [error, signOut, router, createProfile, isError])
+  // [Alaister] If the user has a bad auth token, auth-js won't know about it
+  // and will think the user is authenticated. Since fetching the profile happens
+  // on every page load, we can check for a 401 here and sign the user out if
+  // they have a bad token.
+  const isUnauthorized = isError && error?.code === 401
+  useEffect(() => {
+    if (!isUnauthorized) return
+    signOut().then(() => router.push('/sign-in'))
+  }, [isUnauthorized, signOut, router])
 
   const { isInitialLoading: isLoadingPermissions } = usePermissionsQuery({ enabled: isLoggedIn })