fix(loaders): avoids Uncaught PromiseRejection if the signal is thrown

Fix #768

Author: posva

src/data-loaders/navigation-guard.ts

@@ -152,6 +152,7 @@ export function setupLoaderGuard({
     }
 
     const loaders: UseDataLoader[] = Array.from(to.meta[LOADER_SET_KEY]!)
+    const { signal } = to.meta[ABORT_CONTROLLER_KEY]!
 
     // unset the context so all loaders are executed as root loaders
     setCurrentContext([])
@@ -217,11 +218,16 @@ export function setupLoaderGuard({
       })
       .catch((error) =>
         error instanceof NavigationResult
-          ? error.value
-          : // let the error propagate to router.onError()
-            // we use never because the rejection means we never resolve a value and using anything else
-            // will not be valid from the navigation guard's perspective
-            Promise.reject<never>(error)
+          ? // TODO: why? add comment explaining
+            error.value
+          : // we don't want to propagate an error if it was our own abort signal
+            // this includes cancelled navigations + signal.throwIfAborted() calls
+            signal.aborted && error === signal.reason
+            ? false
+            : // let the error propagate to router.onError()
+              // we use never because the rejection means we never resolve a value and using anything else
+              // will not be valid from the navigation guard's perspective
+              Promise.reject<never>(error)
       )
       .finally(() => {
         // unset the context so mounting happens without an active context

tests/data-loaders/tester.ts

@@ -39,6 +39,7 @@ import { dataOneSpy, dataTwoSpy } from '../data-loaders/loaders'
 import {
   createMemoryHistory,
   createRouter,
+  isNavigationFailure,
   NavigationFailureType,
   type RouteLocationNormalizedLoaded,
   type Router,
@@ -634,6 +635,78 @@ export function testDefineLoader<Context = void>(
       })
     }
 
+    describe('thrown errors in a aborted loader', () => {
+      it(`navigation does not reject if the loader throws the passed signal, commit: ${commit}`, async () => {
+        const loader = mockedLoader({
+          key: 'id',
+          commit,
+          lazy: false,
+        })
+
+        loader.spy.mockImplementation(
+          async (
+            _to: RouteLocationNormalizedLoaded,
+            { signal }: { signal?: AbortSignal }
+          ) => {
+            await delay(10)
+            signal?.throwIfAborted()
+            return 'ko'
+          }
+        )
+
+        const { router } = singleLoaderOneRoute(loader.loader)
+        const onError = vi.fn()
+        router.onError(onError)
+
+        const navigationPromise = router.push('/fetch')
+        // let the loaders start
+        await vi.advanceTimersByTimeAsync(5)
+
+        await expect(router.push('/?other')).resolves.toBeUndefined()
+        await vi.advanceTimersByTimeAsync(5)
+
+        const failure = await navigationPromise
+        expect(failure).toBeDefined()
+        expect(
+          isNavigationFailure(failure, NavigationFailureType.aborted)
+        ).toBe(true)
+        expect(router.currentRoute.value.fullPath).toBe('/?other')
+        // the error was not propagated
+        expect(onError).not.toHaveBeenCalled()
+      })
+
+      it(`navigation rejects if the loader throws an error, commit: ${commit}`, async () => {
+        const loader = mockedLoader({
+          key: 'id',
+          commit,
+          lazy: false,
+        })
+
+        loader.spy.mockImplementation(async () => {
+          await delay(10)
+          throw new Error('ko')
+        })
+
+        const { router } = singleLoaderOneRoute(loader.loader)
+        const onError = vi.fn()
+        router.onError(onError)
+
+        const navigationPromise = expect(router.push('/fetch')).rejects.toThrow(
+          'ko'
+        )
+        // let the loaders start
+        await vi.advanceTimersByTimeAsync(5)
+
+        await expect(router.push('/?other')).resolves.toBeUndefined()
+        await vi.advanceTimersByTimeAsync(5)
+
+        await navigationPromise
+        expect(router.currentRoute.value.fullPath).toBe('/?other')
+        // the error was not propagated
+        expect(onError).toHaveBeenCalledTimes(1)
+      })
+    })
+
     // https://github.com/posva/unplugin-vue-router/issues/584
     it(`skips child loaders if parent returns a NavigationResult, commit: ${commit}`, async () => {
       // Parent loader that redirects