fix(vue): Remove deprecated next callback from router instrumentation

packages/vue/src/router.ts

@@ -30,6 +30,9 @@ export type Route = {
 interface VueRouter {
   onError: (fn: (err: Error) => void) => void;
   beforeEach: (fn: (to: Route, from: Route, next?: () => void) => void) => void;
+  // Vue Router 3 exposes a `mode` property ('hash' | 'history' | 'abstract').
+  // Vue Router 4+ replaced it with `options.history`. Used for version detection.
+  mode?: string;
 }
 
 /**
@@ -52,9 +55,16 @@ export function instrumentVueRouter(
 ): void {
   let hasHandledFirstPageLoad = false;
 
+  // Detect Vue Router 3 by checking for the `mode` property which only exists in VR3.
+  // Vue Router 4+ uses `options.history` instead and does not expose `mode`.
+  const isLegacyRouter = 'mode' in router;
+
   router.onError(error => captureException(error, { mechanism: { handled: false } }));
 
-  router.beforeEach((to, _from, next) => {
+  // Use rest params to capture `next` without declaring it as a named parameter.
+  // This keeps Function.length === 2, which tells Vue Router 4+/5+ to use the
+  // modern return-based resolution (no deprecation warning in Vue Router 5.0.3+).
+  router.beforeEach((to: Route, _from: Route, ...rest: [(() => void)?]) => {
     // We avoid trying to re-fetch the page load span when we know we already handled it the first time
     const activePageLoadSpan = !hasHandledFirstPageLoad ? getActivePageLoadSpan() : undefined;
 
@@ -116,11 +126,16 @@ export function instrumentVueRouter(
       });
     }
 
-    // Vue Router 4 no longer exposes the `next` function, so we need to
-    // check if it's available before calling it.
-    // `next` needs to be called in Vue Router 3 so that the hook is resolved.
-    if (next) {
-      next();
+    // Vue Router 3 requires `next()` to be called to resolve the navigation guard.
+    // Vue Router 4+ auto-resolves guards with Function.length < 3 via `guardToPromiseFn`.
+    // In Vue Router 5.0.3+, the `next` callback passed to guards is wrapped with
+    // `withDeprecationWarning()`, so calling it emits a console warning. We avoid
+    // calling it on modern routers where it is both unnecessary and noisy.
+    if (isLegacyRouter) {
+      const next = rest[0];
+      if (typeof next === 'function') {
+        next();
+      }
     }
   });
 }

packages/vue/test/router.test.ts

@@ -23,11 +23,9 @@ vi.mock('@sentry/core', async () => {
 
 const mockVueRouter = {
   onError: vi.fn<[(error: Error) => void]>(),
-  beforeEach: vi.fn<[(from: Route, to: Route, next?: () => void) => void]>(),
+  beforeEach: vi.fn<[(from: Route, to: Route) => void]>(),
 };
 
-const mockNext = vi.fn();
-
 const testRoutes: Record<string, Route> = {
   initialPageloadRoute: { matched: [], params: {}, path: '', query: {} },
   normalRoute1: {
@@ -118,8 +116,8 @@ describe('instrumentVueRouter()', () => {
 
       const from = testRoutes[fromKey]!;
       const to = testRoutes[toKey]!;
-      beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
-      beforeEachCallback(to, from, mockNext);
+      beforeEachCallback(to, testRoutes['initialPageloadRoute']!); // fake initial pageload
+      beforeEachCallback(to, from);
 
       expect(mockStartSpan).toHaveBeenCalledTimes(2);
       expect(mockStartSpan).toHaveBeenLastCalledWith({
@@ -131,8 +129,6 @@ describe('instrumentVueRouter()', () => {
         },
         op: 'navigation',
       });
-
-      expect(mockNext).toHaveBeenCalledTimes(2);
     },
   );
 
@@ -171,7 +167,7 @@ describe('instrumentVueRouter()', () => {
       const from = testRoutes[fromKey]!;
       const to = testRoutes[toKey]!;
 
-      beforeEachCallback(to, from, mockNext);
+      beforeEachCallback(to, from);
       expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
 
       expect(mockRootSpan.updateName).toHaveBeenCalledWith(transactionName);
@@ -180,8 +176,6 @@ describe('instrumentVueRouter()', () => {
         [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue',
         ...getAttributesForRoute(to),
       });
-
-      expect(mockNext).toHaveBeenCalledTimes(1);
     },
   );
 
@@ -198,8 +192,8 @@ describe('instrumentVueRouter()', () => {
 
     const from = testRoutes.normalRoute1!;
     const to = testRoutes.namedRoute!;
-    beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
-    beforeEachCallback(to, from, mockNext);
+    beforeEachCallback(to, testRoutes['initialPageloadRoute']!); // fake initial pageload
+    beforeEachCallback(to, from);
 
     // first startTx call happens when the instrumentation is initialized (for pageloads)
     expect(mockStartSpan).toHaveBeenLastCalledWith({
@@ -226,8 +220,8 @@ describe('instrumentVueRouter()', () => {
 
     const from = testRoutes.normalRoute1!;
     const to = testRoutes.namedRoute!;
-    beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
-    beforeEachCallback(to, from, mockNext);
+    beforeEachCallback(to, testRoutes['initialPageloadRoute']!); // fake initial pageload
+    beforeEachCallback(to, from);
 
     // first startTx call happens when the instrumentation is initialized (for pageloads)
     expect(mockStartSpan).toHaveBeenLastCalledWith({
@@ -284,7 +278,7 @@ describe('instrumentVueRouter()', () => {
     const to = testRoutes['normalRoute1']!;
     const from = testRoutes['initialPageloadRoute']!;
 
-    beforeEachCallback(to, from, mockNext);
+    beforeEachCallback(to, from);
 
     expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
 
@@ -318,7 +312,7 @@ describe('instrumentVueRouter()', () => {
     const from = testRoutes['initialPageloadRoute']!;
     const to = testRoutes['normalRoute1']!;
 
-    beforeEachCallback(to, from, mockNext);
+    beforeEachCallback(to, from);
 
     expect(scopeSetTransactionNameSpy).toHaveBeenCalledTimes(1);
     expect(scopeSetTransactionNameSpy).toHaveBeenCalledWith('/books/:bookId/chapter/:chapterId');
@@ -357,7 +351,7 @@ describe('instrumentVueRouter()', () => {
       expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
 
       const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!;
-      beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext);
+      beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!);
 
       expect(mockRootSpan.updateName).toHaveBeenCalledTimes(expectedCallsAmount);
       expect(mockStartSpan).not.toHaveBeenCalled();
@@ -381,14 +375,14 @@ describe('instrumentVueRouter()', () => {
       expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
 
       const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!;
-      beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
-      beforeEachCallback(testRoutes['normalRoute2']!, testRoutes['normalRoute1']!, mockNext);
+      beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!); // fake initial pageload
+      beforeEachCallback(testRoutes['normalRoute2']!, testRoutes['normalRoute1']!);
 
       expect(mockStartSpan).toHaveBeenCalledTimes(expectedCallsAmount);
     },
   );
 
-  it("doesn't throw when `next` is not available in the beforeEach callback (Vue Router 4)", () => {
+  it('does not declare a third parameter to avoid Vue Router next() deprecation warning', () => {
     const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
     instrumentVueRouter(
       mockVueRouter,
@@ -398,21 +392,45 @@ describe('instrumentVueRouter()', () => {
 
     const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!;
 
-    const from = testRoutes.normalRoute1!;
-    const to = testRoutes.namedRoute!;
-    beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload
-    beforeEachCallback(to, from, undefined);
+    // Vue Router uses Function.length to detect whether the guard uses the legacy
+    // `next` callback. Guards with < 3 params use the modern return-based pattern.
+    expect(beforeEachCallback.length).toBeLessThan(3);
+  });
 
-    // first startTx call happens when the instrumentation is initialized (for pageloads)
-    expect(mockStartSpan).toHaveBeenLastCalledWith({
-      name: '/login',
-      attributes: {
-        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue',
-        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
-        ...getAttributesForRoute(to),
-      },
-      op: 'navigation',
-    });
+  it('calls next() for Vue Router 3 (legacy router with mode property)', () => {
+    const mockNext = vi.fn();
+    const mockLegacyRouter = {
+      onError: vi.fn<[(error: Error) => void]>(),
+      beforeEach: vi.fn<[(from: Route, to: Route, next?: () => void) => void]>(),
+      mode: 'history',
+    };
+
+    const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
+    instrumentVueRouter(
+      mockLegacyRouter,
+      { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true },
+      mockStartSpan,
+    );
+
+    const beforeEachCallback = mockLegacyRouter.beforeEach.mock.calls[0]![0]!;
+    beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext);
+
+    expect(mockNext).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not call next() for Vue Router 4+ (modern router without mode property)', () => {
+    const mockNext = vi.fn();
+    const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
+    instrumentVueRouter(
+      mockVueRouter,
+      { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true },
+      mockStartSpan,
+    );
+
+    const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!;
+    beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext);
+
+    expect(mockNext).not.toHaveBeenCalled();
   });
 });