Merge branch 'develop' into sig/nitro-dir-option

Author: s1gr1d

dev-packages/e2e-tests/test-applications/nextjs-pages-dir/pages/_error.tsx

@@ -0,0 +1,36 @@
+import type { NextPageContext } from 'next';
+import * as Sentry from '@sentry/nextjs';
+
+interface ErrorProps {
+  statusCode?: number;
+  eventId?: string;
+  lastEventId?: string;
+}
+
+function ErrorPage({ statusCode, eventId, lastEventId }: ErrorProps) {
+  return (
+    <div>
+      <h1>Error Page</h1>
+      <p data-testid="status-code">Status Code: {statusCode}</p>
+      <p data-testid="event-id">Event ID from return: {eventId || 'No event ID'}</p>
+      <p data-testid="last-event-id">Event ID from lastEventId(): {lastEventId || 'No event ID'}</p>
+    </div>
+  );
+}
+
+ErrorPage.getInitialProps = async (context: NextPageContext) => {
+  const { res, err } = context;
+
+  const statusCode = res?.statusCode || err?.statusCode || 404;
+
+  // Capture the error using captureUnderscoreErrorException
+  // This should return the already-captured event ID from the data fetcher
+  const eventId = await Sentry.captureUnderscoreErrorException(context);
+
+  // Also get the last event ID from lastEventId()
+  const lastEventId = Sentry.lastEventId();
+
+  return { statusCode, eventId, lastEventId };
+};
+
+export default ErrorPage;

dev-packages/e2e-tests/test-applications/nextjs-pages-dir/pages/underscore-error/test-error-page.tsx

@@ -0,0 +1,7 @@
+export default function TestErrorPage() {
+  return <div>This page should never render</div>;
+}
+
+export function getServerSideProps() {
+  throw new Error('Test error to trigger _error.tsx page');
+}

dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/error-page-lasteventid.test.ts

@@ -0,0 +1,38 @@
+import { expect, test } from '@playwright/test';
+import { waitForError } from '@sentry-internal/test-utils';
+import { isDevMode } from './isDevMode';
+import { isNext13 } from './nextjsVersion';
+
+test('lastEventId() should return the event ID after captureUnderscoreErrorException', async ({ page }) => {
+  test.skip(isDevMode, 'should be skipped for non-dev mode');
+  test.skip(isNext13, 'should be skipped for Next.js 13');
+
+  const errorEventPromise = waitForError('nextjs-pages-dir', errorEvent => {
+    return errorEvent?.exception?.values?.[0]?.value === 'Test error to trigger _error.tsx page';
+  });
+
+  await page.goto('/underscore-error/test-error-page');
+  const errorEvent = await errorEventPromise;
+
+  // Since the error is already captured by withErrorInstrumentation in getServerSideProps,
+  // the mechanism should be 'auto.function.nextjs.wrapped', not 'auto.function.nextjs.underscore_error'
+  expect(errorEvent.exception?.values?.[0]?.mechanism?.type).toBe('auto.function.nextjs.wrapped');
+  // The function name might be e.g. 'getServerSideProps$1'
+  expect(errorEvent.exception?.values?.[0]?.mechanism?.data?.function).toContain('getServerSideProps');
+  expect(errorEvent.exception?.values?.[0]?.mechanism?.handled).toBe(false);
+
+  const eventIdFromReturn = await page.locator('[data-testid="event-id"]').textContent();
+  const returnedEventId = eventIdFromReturn?.replace('Event ID from return: ', '');
+
+  const lastEventIdFromFunction = await page.locator('[data-testid="last-event-id"]').textContent();
+  const lastEventId = lastEventIdFromFunction?.replace('Event ID from lastEventId(): ', '');
+
+  expect(returnedEventId).toBeDefined();
+  expect(returnedEventId).not.toBe('No event ID');
+  expect(lastEventId).toBeDefined();
+  expect(lastEventId).not.toBe('No event ID');
+
+  expect(lastEventId).toBe(returnedEventId);
+  expect(errorEvent.event_id).toBe(returnedEventId);
+  expect(errorEvent.event_id).toBe(lastEventId);
+});

dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/nextjsVersion.ts

@@ -0,0 +1,5 @@
+const packageJson = require('../package.json');
+const nextjsVersion = packageJson.dependencies.next;
+const nextjsMajor = Number(nextjsVersion.split('.')[0]);
+
+export const isNext13 = !isNaN(nextjsMajor) && nextjsMajor === 13;

packages/browser/src/tracing/request.ts

@@ -332,24 +332,28 @@ function xhrCallback(
 
   const shouldCreateSpanResult = hasSpansEnabled() && shouldCreateSpan(url);
 
-  // check first if the request has finished and is tracked by an existing span which should now end
-  if (handlerData.endTimestamp && shouldCreateSpanResult) {
+  // Handle XHR completion - clean up spans from the record
+  if (handlerData.endTimestamp) {
     const spanId = xhr.__sentry_xhr_span_id__;
     if (!spanId) return;
 
     const span = spans[spanId];
-    if (span && sentryXhrData.status_code !== undefined) {
-      setHttpStatus(span, sentryXhrData.status_code);
-      span.end();
 
-      onRequestSpanEnd?.(span, {
-        headers: createHeadersSafely(parseXhrResponseHeaders(xhr as XMLHttpRequest & SentryWrappedXMLHttpRequest)),
-        error: handlerData.error,
-      });
+    if (span) {
+      if (shouldCreateSpanResult && sentryXhrData.status_code !== undefined) {
+        setHttpStatus(span, sentryXhrData.status_code);
+        span.end();
+
+        onRequestSpanEnd?.(span, {
+          headers: createHeadersSafely(parseXhrResponseHeaders(xhr as XMLHttpRequest & SentryWrappedXMLHttpRequest)),
+          error: handlerData.error,
+        });
+      }
 
       // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
       delete spans[spanId];
     }
+
     return undefined;
   }
 

packages/core/src/fetch.ts

@@ -81,19 +81,23 @@ export function instrumentFetchRequest(
 
   const shouldCreateSpanResult = hasSpansEnabled() && shouldCreateSpan(url);
 
-  if (handlerData.endTimestamp && shouldCreateSpanResult) {
+  if (handlerData.endTimestamp) {
     const spanId = handlerData.fetchData.__span;
     if (!spanId) return;
 
     const span = spans[spanId];
-    if (span) {
-      endSpan(span, handlerData);
 
-      _callOnRequestSpanEnd(span, handlerData, spanOriginOrOptions);
+    if (span) {
+      // Only end the span and call hooks if we're actually recording
+      if (shouldCreateSpanResult) {
+        endSpan(span, handlerData);
+        _callOnRequestSpanEnd(span, handlerData, spanOriginOrOptions);
+      }
 
       // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
       delete spans[spanId];
     }
+
     return undefined;
   }
 

packages/core/src/index.ts

@@ -221,6 +221,7 @@ export {
   addExceptionMechanism,
   addExceptionTypeValue,
   checkOrSetAlreadyCaught,
+  isAlreadyCaptured,
   getEventDescription,
   parseSemver,
   uuid4,

packages/core/src/utils/misc.ts

@@ -227,7 +227,13 @@ export function checkOrSetAlreadyCaught(exception: unknown): boolean {
   return false;
 }
 
-function isAlreadyCaptured(exception: unknown): boolean | void {
+/**
+ * Checks whether we've already captured the given exception (note: not an identical exception - the very object).
+ * It is considered already captured if it has the `__sentry_captured__` property set to `true`.
+ *
+ * @internal Only considered for internal usage
+ */
+export function isAlreadyCaptured(exception: unknown): boolean | void {
   try {
     return (exception as { __sentry_captured__?: boolean }).__sentry_captured__;
   } catch {} // eslint-disable-line no-empty

packages/core/test/lib/fetch.test.ts

@@ -1,10 +1,12 @@
-import { describe, expect, it, vi } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type { HandlerDataFetch } from '../../src';
 import { _addTracingHeadersToFetchRequest, instrumentFetchRequest } from '../../src/fetch';
 import type { Span } from '../../src/types-hoist/span';
 
-const { DEFAULT_SENTRY_TRACE, DEFAULT_BAGGAGE } = vi.hoisted(() => ({
+const { DEFAULT_SENTRY_TRACE, DEFAULT_BAGGAGE, hasSpansEnabled } = vi.hoisted(() => ({
   DEFAULT_SENTRY_TRACE: 'defaultTraceId-defaultSpanId-1',
   DEFAULT_BAGGAGE: 'sentry-trace_id=defaultTraceId,sentry-sampled=true,sentry-sample_rate=0.5,sentry-sample_rand=0.232',
+  hasSpansEnabled: vi.fn(),
 }));
 
 const CUSTOM_SENTRY_TRACE = '123-abc-1';
@@ -23,7 +25,18 @@ vi.mock('../../src/utils/traceData', () => {
   };
 });
 
+vi.mock('../../src/utils/hasSpansEnabled', () => {
+  return {
+    hasSpansEnabled,
+  };
+});
+
 describe('_addTracingHeadersToFetchRequest', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    hasSpansEnabled.mockReturnValue(false);
+  });
+
   describe('when request is a string', () => {
     describe('and no request headers are set', () => {
       it.each([
@@ -412,6 +425,53 @@ describe('_addTracingHeadersToFetchRequest', () => {
 });
 
 describe('instrumentFetchRequest', () => {
+  describe('span cleanup', () => {
+    it.each([
+      { name: 'non-recording', hasTracingEnabled: false },
+      { name: 'recording', hasTracingEnabled: true },
+    ])('cleans up $name spans from the spans record when fetch completes', ({ hasTracingEnabled }) => {
+      hasSpansEnabled.mockReturnValue(hasTracingEnabled);
+
+      const spans: Record<string, Span> = {};
+
+      const handlerData = {
+        fetchData: { url: '/api/test', method: 'GET' },
+        args: ['/api/test'] as unknown[],
+        startTimestamp: Date.now(),
+      };
+
+      instrumentFetchRequest(
+        handlerData,
+        () => true,
+        () => false,
+        spans,
+        { spanOrigin: 'auto.http.fetch' },
+      );
+
+      // @ts-expect-error -- we know it exists
+      const spanId = handlerData.fetchData.__span;
+
+      expect(spanId).toBeDefined();
+      expect(spans[spanId]).toBeDefined();
+
+      const completedHandlerData: HandlerDataFetch = {
+        ...handlerData,
+        endTimestamp: Date.now() + 100,
+        response: { status: 200, headers: new Headers() } as Response,
+      };
+
+      instrumentFetchRequest(
+        completedHandlerData,
+        () => true,
+        () => false,
+        spans,
+        { spanOrigin: 'auto.http.fetch' },
+      );
+
+      expect(spans[spanId]).toBeUndefined();
+    });
+  });
+
   describe('options object mutation', () => {
     it('does not mutate the original options object', () => {
       const originalOptions = { method: 'POST', body: JSON.stringify({ data: 'test' }) };

packages/nextjs/src/common/pages-router-instrumentation/_error.ts

@@ -1,4 +1,10 @@
-import { captureException, httpRequestToRequestData, withScope } from '@sentry/core';
+import {
+  captureException,
+  getIsolationScope,
+  httpRequestToRequestData,
+  isAlreadyCaptured,
+  withScope,
+} from '@sentry/core';
 import type { NextPageContext } from 'next';
 import { flushSafelyWithTimeout, waitUntil } from '../utils/responseEnd';
 
@@ -38,6 +44,13 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP
     return;
   }
 
+  // If the error was already captured (e.g., by wrapped functions in data fetchers),
+  // return the existing event ID instead of capturing it again (needed for lastEventId() to work)
+  if (err && isAlreadyCaptured(err)) {
+    waitUntil(flushSafelyWithTimeout());
+    return getIsolationScope().lastEventId();
+  }
+
   const eventId = withScope(scope => {
     if (req) {
       const normalizedRequest = httpRequestToRequestData(req);