fix: app-router prefetch crash when an invalid URL is passed to Link (#…

…66755)

Closes [#66650](#66650)
Closes NEXT-3520

- Make Link not throw during prefetch if it received an invalid `href`
(see [#66650](#66650))
- Throw in dev mode if an invalid link was passed to `router.prefetch`
-- this matches current prod behavior
- (previously, we'd immediately exit out of `router.prefetch`, so the
user would see no indication that this'd fail in prod)

If an invalid URL was passed to `<Link>`, the whole app would crash and
show "A client-side exception occurred". A failed prefetch should not
bring down the whole app.

Note that This preserves the current behavior of explicit
`router.prefetch(url)` throwing an error if `url` is invalid. We may
want to adjust this in the future, but that could be considered a
breaking change, so I'm leaving it out for now. This only affects
`Link`, which was intended to catch any errors thrown from
`router.prefetch`, but that bit was accidentally broken.

packages/next/src/client/components/app-router.tsx

@@ -356,14 +356,24 @@ function Router({
       forward: () => window.history.forward(),
       prefetch: (href, options) => {
         // Don't prefetch for bots as they don't navigate.
+        if (isBot(window.navigator.userAgent)) {
+          return
+        }
+
+        let url: URL
+        try {
+          url = new URL(addBasePath(href), window.location.href)
+        } catch (_) {
+          throw new Error(
+            `Cannot prefetch '${href}' because it cannot be converted to a URL.`
+          )
+        }
+
         // Don't prefetch during development (improves compilation performance)
-        if (
-          isBot(window.navigator.userAgent) ||
-          process.env.NODE_ENV === 'development'
-        ) {
+        if (process.env.NODE_ENV === 'development') {
           return
         }
-        const url = new URL(addBasePath(href), window.location.href)
+
         // External urls can't be prefetched in the same way.
         if (isExternalURL(url)) {
           return

packages/next/src/client/link.tsx

@@ -161,15 +161,21 @@ function prefetch(
     prefetched.add(prefetchedKey)
   }
 
-  const prefetchPromise = isAppRouter
-    ? (router as AppRouterInstance).prefetch(href, appOptions)
-    : (router as NextRouter).prefetch(href, as, options)
+  const doPrefetch = async () => {
+    if (isAppRouter) {
+      // note that `appRouter.prefetch()` is currently sync,
+      // so we have to wrap this call in an async function to be able to catch() errors below.
+      return (router as AppRouterInstance).prefetch(href, appOptions)
+    } else {
+      return (router as NextRouter).prefetch(href, as, options)
+    }
+  }
 
   // Prefetch the JSON page if asked (only in the client)
   // We need to handle a prefetch error here since we may be
   // loading with priority which can reject but we don't
   // want to force navigation since this is only a prefetch
-  Promise.resolve(prefetchPromise).catch((err) => {
+  doPrefetch().catch((err) => {
     if (process.env.NODE_ENV !== 'production') {
       // rethrow to show invalid URL errors
       throw err

test/e2e/app-dir/app-prefetch/app/invalid-url/delay.js

@@ -0,0 +1,15 @@
+'use client'
+
+import { useEffect } from 'react'
+import { useState } from 'react'
+
+export function Delay({ duration = 500, children }) {
+  const [isVisible, setIsVisible] = useState(false)
+  useEffect(() => {
+    const timeout = setTimeout(() => setIsVisible(true), duration)
+    return () => clearTimeout(timeout)
+  }, [duration])
+
+  if (!isVisible) return null
+  return <>{children}</>
+}

test/e2e/app-dir/app-prefetch/app/invalid-url/error.js

@@ -0,0 +1,4 @@
+'use client'
+export default function Error() {
+  return <h1>A prefetch threw an error</h1>
+}

test/e2e/app-dir/app-prefetch/app/invalid-url/from-link/page.js

@@ -0,0 +1,16 @@
+import Link from 'next/link'
+import { INVALID_URL } from '../invalid-url'
+import { Delay } from '../delay'
+
+export const dynamic = 'force-dynamic'
+
+export default function Page() {
+  return (
+    <>
+      <Link href={INVALID_URL}>invalid link</Link>
+      <Delay>
+        <h1>Hello, world!</h1>
+      </Delay>
+    </>
+  )
+}

test/e2e/app-dir/app-prefetch/app/invalid-url/from-router-prefetch/page.js

@@ -0,0 +1,20 @@
+'use client'
+import { useEffect } from 'react'
+import { INVALID_URL } from '../invalid-url'
+import { Delay } from '../delay'
+import { useRouter } from 'next/navigation'
+
+export const dynamic = 'force-dynamic'
+
+export default function Page() {
+  const router = useRouter()
+  useEffect(() => {
+    router.prefetch(INVALID_URL)
+  }, [router])
+
+  return (
+    <Delay>
+      <h1>Hello, world!</h1>
+    </Delay>
+  )
+}

test/e2e/app-dir/app-prefetch/app/invalid-url/invalid-url.js

@@ -0,0 +1,8 @@
+// We need a URL that reliably fails `new URL(url, window.location)
+// This only fails if `window.location` starts with `https://`:
+//
+//   const invalidUrl = 'http:'
+//
+// because `new URL('http:', 'http://localhost:3000')` works fine.
+// So better to pick something that's always invalid
+export const INVALID_URL = '///'

test/e2e/app-dir/app-prefetch/prefetching.test.ts

@@ -394,5 +394,23 @@ createNextDescribe(
         expect(await browser.elementById('count').text()).toBe('4')
       })
     })
+
+    describe('invalid URLs', () => {
+      it('should not throw when an invalid URL is passed to Link', async () => {
+        const browser = await next.browser('/invalid-url/from-link')
+
+        await check(() => browser.hasElementByCssSelector('h1'), true)
+        expect(await browser.elementByCss('h1').text()).toEqual('Hello, world!')
+      })
+
+      it('should throw when an invalid URL is passed to router.prefetch', async () => {
+        const browser = await next.browser('/invalid-url/from-router-prefetch')
+
+        await check(() => browser.hasElementByCssSelector('h1'), true)
+        expect(await browser.elementByCss('h1').text()).toEqual(
+          'A prefetch threw an error'
+        )
+      })
+    })
   }
 )