fix: harden async window open placeholder logic (#29393)

web/app/components/app/app-publisher/index.tsx

@@ -21,6 +21,7 @@ import {
 import { useKeyPress } from 'ahooks'
 import Divider from '../../base/divider'
 import Loading from '../../base/loading'
+import Toast from '../../base/toast'
 import Tooltip from '../../base/tooltip'
 import { getKeyboardKeyCodeBySystem, getKeyboardKeyNameBySystem } from '../../workflow/utils'
 import AccessControl from '../app-access-control'
@@ -41,6 +42,7 @@ import type { InputVar, Variable } from '@/app/components/workflow/types'
 import { appDefaultIconBackground } from '@/config'
 import { useGlobalPublicStore } from '@/context/global-public-context'
 import { useFormatTimeFromNow } from '@/hooks/use-format-time-from-now'
+import { useAsyncWindowOpen } from '@/hooks/use-async-window-open'
 import { AccessMode } from '@/models/access-control'
 import { useAppWhiteListSubjects, useGetUserCanAccessApp } from '@/service/access-control'
 import { fetchAppDetailDirect } from '@/service/apps'
@@ -49,7 +51,6 @@ import { AppModeEnum } from '@/types/app'
 import type { PublishWorkflowParams } from '@/types/workflow'
 import { basePath } from '@/utils/var'
 import UpgradeBtn from '@/app/components/billing/upgrade-btn'
-import { useAsyncWindowOpen } from '@/hooks/use-async-window-open'
 
 const ACCESS_MODE_MAP: Record<AccessMode, { label: string, icon: React.ElementType }> = {
   [AccessMode.ORGANIZATION]: {
@@ -153,6 +154,7 @@ const AppPublisher = ({
 
   const { data: userCanAccessApp, isLoading: isGettingUserCanAccessApp, refetch } = useGetUserCanAccessApp({ appId: appDetail?.id, enabled: false })
   const { data: appAccessSubjects, isLoading: isGettingAppWhiteListSubjects } = useAppWhiteListSubjects(appDetail?.id, open && systemFeatures.webapp_auth.enabled && appDetail?.access_mode === AccessMode.SPECIFIC_GROUPS_MEMBERS)
+  const openAsyncWindow = useAsyncWindowOpen()
 
   const noAccessPermission = useMemo(() => systemFeatures.webapp_auth.enabled && appDetail && appDetail.access_mode !== AccessMode.EXTERNAL_MEMBERS && !userCanAccessApp?.result, [systemFeatures, appDetail, userCanAccessApp])
   const disabledFunctionButton = useMemo(() => (!publishedAt || missingStartNode || noAccessPermission), [publishedAt, missingStartNode, noAccessPermission])
@@ -216,23 +218,20 @@ const AppPublisher = ({
       setPublished(false)
   }, [disabled, onToggle, open])
 
-  const { openAsync } = useAsyncWindowOpen()
-
-  const handleOpenInExplore = useCallback(() => {
-    if (!appDetail?.id) return
-
-    openAsync(
-      async () => {
-        const { installed_apps }: { installed_apps?: { id: string }[] } = await fetchInstalledAppList(appDetail.id) || {}
-        if (installed_apps && installed_apps.length > 0)
+  const handleOpenInExplore = useCallback(async () => {
+    await openAsyncWindow(async () => {
+      if (!appDetail?.id)
+        throw new Error('App not found')
+      const { installed_apps }: any = await fetchInstalledAppList(appDetail?.id) || {}
+      if (installed_apps?.length > 0)
         return `${basePath}/explore/installed/${installed_apps[0].id}`
       throw new Error('No app found in Explore')
+    }, {
+      onError: (err) => {
+        Toast.notify({ type: 'error', message: `${err.message || err}` })
       },
-      {
-        errorMessage: 'Failed to open app in Explore',
-      },
-    )
-  }, [appDetail?.id, openAsync])
+    })
+  }, [appDetail?.id, openAsyncWindow])
 
   const handleAccessControlUpdate = useCallback(async () => {
     if (!appDetail)

web/app/components/apps/app-card.tsx

@@ -7,7 +7,7 @@ import { useTranslation } from 'react-i18next'
 import { RiBuildingLine, RiGlobalLine, RiLockLine, RiMoreFill, RiVerifiedBadgeLine } from '@remixicon/react'
 import cn from '@/utils/classnames'
 import { type App, AppModeEnum } from '@/types/app'
-import { ToastContext } from '@/app/components/base/toast'
+import Toast, { ToastContext } from '@/app/components/base/toast'
 import { copyApp, deleteApp, exportAppConfig, updateAppInfo } from '@/service/apps'
 import type { DuplicateAppModalProps } from '@/app/components/app/duplicate-modal'
 import AppIcon from '@/app/components/base/app-icon'
@@ -27,11 +27,11 @@ import { fetchWorkflowDraft } from '@/service/workflow'
 import { fetchInstalledAppList } from '@/service/explore'
 import { AppTypeIcon } from '@/app/components/app/type-selector'
 import Tooltip from '@/app/components/base/tooltip'
+import { useAsyncWindowOpen } from '@/hooks/use-async-window-open'
 import { AccessMode } from '@/models/access-control'
 import { useGlobalPublicStore } from '@/context/global-public-context'
 import { formatTime } from '@/utils/time'
 import { useGetUserCanAccessApp } from '@/service/access-control'
-import { useAsyncWindowOpen } from '@/hooks/use-async-window-open'
 import dynamic from 'next/dynamic'
 
 const EditAppModal = dynamic(() => import('@/app/components/explore/create-app-modal'), {
@@ -65,6 +65,7 @@ const AppCard = ({ app, onRefresh }: AppCardProps) => {
   const { isCurrentWorkspaceEditor } = useAppContext()
   const { onPlanInfoChanged } = useProviderContext()
   const { push } = useRouter()
+  const openAsyncWindow = useAsyncWindowOpen()
 
   const [showEditModal, setShowEditModal] = useState(false)
   const [showDuplicateModal, setShowDuplicateModal] = useState(false)
@@ -243,24 +244,25 @@ const AppCard = ({ app, onRefresh }: AppCardProps) => {
       e.preventDefault()
       setShowAccessControl(true)
     }
-    const { openAsync } = useAsyncWindowOpen()
-
-    const onClickInstalledApp = (e: React.MouseEvent<HTMLButtonElement>) => {
+    const onClickInstalledApp = async (e: React.MouseEvent<HTMLButtonElement>) => {
       e.stopPropagation()
       props.onClick?.()
       e.preventDefault()
-
-      openAsync(
-        async () => {
-          const { installed_apps }: { installed_apps?: { id: string }[] } = await fetchInstalledAppList(app.id) || {}
-          if (installed_apps && installed_apps.length > 0)
+      try {
+        await openAsyncWindow(async () => {
+          const { installed_apps }: any = await fetchInstalledAppList(app.id) || {}
+          if (installed_apps?.length > 0)
             return `${basePath}/explore/installed/${installed_apps[0].id}`
           throw new Error('No app found in Explore')
+        }, {
+          onError: (err) => {
+            Toast.notify({ type: 'error', message: `${err.message || err}` })
           },
-        {
-          errorMessage: 'Failed to open app in Explore',
-        },
-      )
+        })
+      }
+      catch (e: any) {
+        Toast.notify({ type: 'error', message: `${e.message || e}` })
+      }
     }
     return (
       <div className="relative flex w-full flex-col py-1" onMouseLeave={onMouseLeave}>

web/app/components/billing/billing-page/index.tsx

@@ -9,33 +9,28 @@ import PlanComp from '../plan'
 import { useAppContext } from '@/context/app-context'
 import { useProviderContext } from '@/context/provider-context'
 import { useBillingUrl } from '@/service/use-billing'
+import { useAsyncWindowOpen } from '@/hooks/use-async-window-open'
 
 const Billing: FC = () => {
   const { t } = useTranslation()
   const { isCurrentWorkspaceManager } = useAppContext()
   const { enableBilling } = useProviderContext()
   const { data: billingUrl, isFetching, refetch } = useBillingUrl(enableBilling && isCurrentWorkspaceManager)
+  const openAsyncWindow = useAsyncWindowOpen()
 
   const handleOpenBilling = async () => {
-    // Open synchronously to preserve user gesture for popup blockers
-    if (billingUrl) {
-      window.open(billingUrl, '_blank', 'noopener,noreferrer')
-      return
-    }
-
-    const newWindow = window.open('', '_blank', 'noopener,noreferrer')
-    try {
+    await openAsyncWindow(async () => {
       const url = (await refetch()).data
-      if (url && newWindow) {
-        newWindow.location.href = url
-        return
-      }
-    }
-    catch (err) {
+      if (url)
+        return url
+      return null
+    }, {
+      immediateUrl: billingUrl,
+      features: 'noopener,noreferrer',
+      onError: (err) => {
         console.error('Failed to fetch billing url', err)
-    }
-    // Close the placeholder window if we failed to fetch the URL
-    newWindow?.close()
+      },
+    })
   }
 
   return (

web/app/components/billing/pricing/plans/cloud-plan-item/index.tsx

@@ -43,6 +43,7 @@ const CloudPlanItem: FC<CloudPlanItemProps> = ({
   const isCurrentPaidPlan = isCurrent && !isFreePlan
   const isPlanDisabled = isCurrentPaidPlan ? false : planInfo.level <= ALL_PLANS[currentPlan].level
   const { isCurrentWorkspaceManager } = useAppContext()
+  const openAsyncWindow = useAsyncWindowOpen()
 
   const btnText = useMemo(() => {
     if (isCurrent)
@@ -55,8 +56,6 @@ const CloudPlanItem: FC<CloudPlanItemProps> = ({
     })[plan]
   }, [isCurrent, plan, t])
 
-  const { openAsync } = useAsyncWindowOpen()
-
   const handleGetPayUrl = async () => {
     if (loading)
       return
@@ -75,13 +74,16 @@ const CloudPlanItem: FC<CloudPlanItemProps> = ({
     setLoading(true)
     try {
       if (isCurrentPaidPlan) {
-        await openAsync(
-          () => fetchBillingUrl().then(res => res.url),
-          {
-            errorMessage: 'Failed to open billing page',
-            windowFeatures: 'noopener,noreferrer',
+        await openAsyncWindow(async () => {
+          const res = await fetchBillingUrl()
+          if (res.url)
+            return res.url
+          throw new Error('Failed to open billing page')
+        }, {
+          onError: (err) => {
+            Toast.notify({ type: 'error', message: err.message || String(err) })
           },
-        )
+        })
         return
       }
 

web/hooks/use-async-window-open.spec.ts

@@ -0,0 +1,116 @@
+import { act, renderHook } from '@testing-library/react'
+import { useAsyncWindowOpen } from './use-async-window-open'
+
+describe('useAsyncWindowOpen', () => {
+  const originalOpen = window.open
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  afterAll(() => {
+    window.open = originalOpen
+  })
+
+  it('opens immediate url synchronously without calling async getter', async () => {
+    const openSpy = jest.fn()
+    window.open = openSpy
+    const getUrl = jest.fn()
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(getUrl, {
+        immediateUrl: 'https://example.com',
+        target: '_blank',
+        features: 'noopener,noreferrer',
+      })
+    })
+
+    expect(openSpy).toHaveBeenCalledWith('https://example.com', '_blank', 'noopener,noreferrer')
+    expect(getUrl).not.toHaveBeenCalled()
+  })
+
+  it('sets opener to null and redirects when async url resolves', async () => {
+    const close = jest.fn()
+    const mockWindow: any = {
+      location: { href: '' },
+      close,
+      opener: 'should-be-cleared',
+    }
+    const openSpy = jest.fn(() => mockWindow)
+    window.open = openSpy
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(async () => 'https://example.com/path')
+    })
+
+    expect(openSpy).toHaveBeenCalledWith('about:blank', '_blank', undefined)
+    expect(mockWindow.opener).toBeNull()
+    expect(mockWindow.location.href).toBe('https://example.com/path')
+    expect(close).not.toHaveBeenCalled()
+  })
+
+  it('closes placeholder and forwards error when async getter throws', async () => {
+    const close = jest.fn()
+    const mockWindow: any = {
+      location: { href: '' },
+      close,
+      opener: null,
+    }
+    const openSpy = jest.fn(() => mockWindow)
+    window.open = openSpy
+    const onError = jest.fn()
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    const error = new Error('fetch failed')
+    await act(async () => {
+      await result.current(async () => {
+        throw error
+      }, { onError })
+    })
+
+    expect(close).toHaveBeenCalled()
+    expect(onError).toHaveBeenCalledWith(error)
+    expect(mockWindow.location.href).toBe('')
+  })
+
+  it('closes placeholder and reports when no url is returned', async () => {
+    const close = jest.fn()
+    const mockWindow: any = {
+      location: { href: '' },
+      close,
+      opener: null,
+    }
+    const openSpy = jest.fn(() => mockWindow)
+    window.open = openSpy
+    const onError = jest.fn()
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(async () => null, { onError })
+    })
+
+    expect(close).toHaveBeenCalled()
+    expect(onError).toHaveBeenCalled()
+    const errArg = onError.mock.calls[0][0] as Error
+    expect(errArg.message).toBe('No url resolved for new window')
+  })
+
+  it('reports failure when window.open returns null', async () => {
+    const openSpy = jest.fn(() => null)
+    window.open = openSpy
+    const getUrl = jest.fn()
+    const onError = jest.fn()
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(getUrl, { onError })
+    })
+
+    expect(onError).toHaveBeenCalled()
+    const errArg = onError.mock.calls[0][0] as Error
+    expect(errArg.message).toBe('Failed to open new window')
+    expect(getUrl).not.toHaveBeenCalled()
+  })
+})

web/hooks/use-async-window-open.ts

@@ -1,72 +1,49 @@
 import { useCallback } from 'react'
-import Toast from '@/app/components/base/toast'
 
-export type AsyncWindowOpenOptions = {
-  successMessage?: string
-  errorMessage?: string
-  windowFeatures?: string
-  onError?: (error: any) => void
-  onSuccess?: (url: string) => void
+type GetUrl = () => Promise<string | null | undefined>
+
+type AsyncWindowOpenOptions = {
+  immediateUrl?: string | null
+  target?: string
+  features?: string
+  onError?: (error: Error) => void
 }
 
-export const useAsyncWindowOpen = () => {
-  const openAsync = useCallback(async (
-    fetchUrl: () => Promise<string>,
-    options: AsyncWindowOpenOptions = {},
-  ) => {
+export const useAsyncWindowOpen = () => useCallback(async (getUrl: GetUrl, options?: AsyncWindowOpenOptions) => {
   const {
-      successMessage,
-      errorMessage = 'Failed to open page',
-      windowFeatures = 'noopener,noreferrer',
+    immediateUrl,
+    target = '_blank',
+    features,
     onError,
-      onSuccess,
-    } = options
+  } = options ?? {}
 
-    const newWindow = window.open('', '_blank', windowFeatures)
+  if (immediateUrl) {
+    window.open(immediateUrl, target, features)
+    return
+  }
 
+  const newWindow = window.open('about:blank', target, features)
   if (!newWindow) {
-      const error = new Error('Popup blocked by browser')
-      onError?.(error)
-      Toast.notify({
-        type: 'error',
-        message: 'Popup blocked. Please allow popups for this site.',
-      })
+    onError?.(new Error('Failed to open new window'))
     return
   }
 
   try {
-      const url = await fetchUrl()
+    newWindow.opener = null
+  }
+  catch { /* noop */ }
 
+  try {
+    const url = await getUrl()
     if (url) {
       newWindow.location.href = url
-        onSuccess?.(url)
-
-        if (successMessage) {
-          Toast.notify({
-            type: 'success',
-            message: successMessage,
-          })
+      return
     }
-      }
-      else {
     newWindow.close()
-        const error = new Error('Invalid URL received')
-        onError?.(error)
-        Toast.notify({
-          type: 'error',
-          message: errorMessage,
-        })
-      }
+    onError?.(new Error('No url resolved for new window'))
   }
   catch (error) {
     newWindow.close()
-      onError?.(error)
-      Toast.notify({
-        type: 'error',
-        message: errorMessage,
-      })
+    onError?.(error instanceof Error ? error : new Error(String(error)))
   }
 }, [])
-
-  return { openAsync }
-}