fix(core): Return same value from startSpan as callback returns

dev-packages/browser-integration-tests/suites/public-api/startSpan/thenable-with-extra-methods/subject.js

@@ -0,0 +1,79 @@
+/**
+ * Test that verifies thenable objects with extra methods (like jQuery's jqXHR)
+ * preserve those methods when returned from Sentry.startSpan().
+ *
+ * Example case:
+ *   const jqXHR = Sentry.startSpan({ name: "test" }, () => $.ajax(...));
+ *   jqXHR.abort(); // Should work and not throw an error because of missing abort() method
+ */
+
+// Load jQuery from CDN
+const script = document.createElement('script');
+script.src = 'https://code.jquery.com/jquery-3.7.1.min.js';
+script.integrity = 'sha256-/JqT3SQfawRcv/BIHPThkBvs0OEvtFFmqPF/lYI/Cxo=';
+script.crossOrigin = 'anonymous';
+
+script.onload = function () {
+  runTest();
+};
+
+script.onerror = function () {
+  window.jqXHRTestError = 'Failed to load jQuery';
+  window.jqXHRMethodsPreserved = false;
+};
+
+document.head.appendChild(script);
+
+async function runTest() {
+  window.jqXHRAbortCalled = false;
+  window.jqXHRAbortResult = null;
+  window.jqXHRTestError = null;
+
+  try {
+    if (!window.jQuery) {
+      throw new Error('jQuery not loaded');
+    }
+
+    const result = Sentry.startSpan({ name: 'test-jqxhr', op: 'http.client' }, () => {
+      // Make a real AJAX request with jQuery
+      return window.jQuery.ajax({
+        url: 'https://httpbin.org/status/200',
+        method: 'GET',
+        timeout: 5000,
+      });
+    });
+
+    const hasAbort = typeof result.abort === 'function';
+    const hasReadyState = 'readyState' in result;
+
+    if (hasAbort && hasReadyState) {
+      try {
+        result.abort();
+        window.jqXHRAbortCalled = true;
+        window.jqXHRAbortResult = 'abort-successful';
+        window.jqXHRMethodsPreserved = true;
+      } catch (e) {
+        console.log('abort() threw an error:', e);
+        window.jqXHRTestError = `abort() failed: ${e.message}`;
+        window.jqXHRMethodsPreserved = false;
+      }
+    } else {
+      window.jqXHRMethodsPreserved = false;
+      window.jqXHRTestError = 'jqXHR methods not preserved';
+    }
+
+    // Since we aborted the request, it should be rejected
+    try {
+      await result;
+      window.jqXHRPromiseResolved = true; // Unexpected
+    } catch (err) {
+      // Expected: aborted request rejects
+      window.jqXHRPromiseResolved = false;
+      window.jqXHRPromiseRejected = true;
+    }
+  } catch (error) {
+    console.error('Test error:', error);
+    window.jqXHRTestError = error.message;
+    window.jqXHRMethodsPreserved = false;
+  }
+}

dev-packages/browser-integration-tests/suites/public-api/startSpan/thenable-with-extra-methods/test.ts

@@ -0,0 +1,51 @@
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
+
+sentryTest('preserves extra methods on real jQuery jqXHR objects', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+  const transactionPromise = waitForTransactionRequest(page);
+
+  await page.goto(url);
+
+  // Wait for jQuery to load
+  await page.waitForTimeout(1000);
+
+  const methodsPreserved = await page.evaluate(() => (window as any).jqXHRMethodsPreserved);
+  expect(methodsPreserved).toBe(true);
+
+  const abortCalled = await page.evaluate(() => (window as any).jqXHRAbortCalled);
+  expect(abortCalled).toBe(true);
+
+  const abortReturnValue = await page.evaluate(() => (window as any).jqXHRAbortResult);
+  expect(abortReturnValue).toBe('abort-successful');
+
+  const testError = await page.evaluate(() => (window as any).jqXHRTestError);
+  expect(testError).toBeNull();
+
+  const transaction = envelopeRequestParser(await transactionPromise);
+  expect(transaction.transaction).toBe('test-jqxhr');
+  expect(transaction.spans).toBeDefined();
+});
+
+sentryTest('aborted request rejects promise correctly', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+  await page.goto(url);
+
+  // Wait for jQuery to load
+  await page.waitForTimeout(1000);
+
+  const promiseRejected = await page.evaluate(() => (window as any).jqXHRPromiseRejected);
+  expect(promiseRejected).toBe(true);
+
+  const promiseResolved = await page.evaluate(() => (window as any).jqXHRPromiseResolved);
+  expect(promiseResolved).toBe(false);
+});

packages/core/src/asyncContext/stackStrategy.ts

@@ -1,6 +1,7 @@
 import type { Client } from '../client';
 import { getDefaultCurrentScope, getDefaultIsolationScope } from '../defaultScopes';
 import { Scope } from '../scope';
+import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
 import { isThenable } from '../utils/is';
 import { getMainCarrier, getSentryCarrier } from './../carrier';
 import type { AsyncContextStrategy } from './types';
@@ -52,17 +53,11 @@ export class AsyncContextStack {
     }
 
     if (isThenable(maybePromiseResult)) {
-      // @ts-expect-error - isThenable returns the wrong type
-      return maybePromiseResult.then(
-        res => {
-          this._popScope();
-          return res;
-        },
-        e => {
-          this._popScope();
-          throw e;
-        },
-      );
+      return chainAndCopyPromiseLike(
+        maybePromiseResult as PromiseLike<Awaited<typeof maybePromiseResult>> & Record<string, unknown>,
+        () => this._popScope(),
+        () => this._popScope(),
+      ) as T;
     }
 
     this._popScope();

packages/core/src/utils/chain-and-copy-promiselike.ts

@@ -0,0 +1,43 @@
+/**
+ * Copy the properties from a decorated promiselike object onto its chained
+ * actual promise.
+ */
+export const chainAndCopyPromiseLike = <V, T extends PromiseLike<V> & Record<string, unknown>>(
+  original: T,
+  onSuccess: (value: V) => void,
+  onError: (e: unknown) => void,
+): T => {
+  return copyProps(
+    original,
+    original.then(
+      value => {
+        onSuccess(value);
+        return value;
+      },
+      err => {
+        onError(err);
+        throw err;
+      },
+    ),
+  ) as T;
+};
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const copyProps = <T extends Record<string, any>>(original: T, chained: T): T => {
+  for (const key in original) {
+    if (key in chained) continue;
+    const value = original[key];
+    if (typeof value === 'function') {
+      Object.defineProperty(chained, key, {
+        value: (...args: unknown[]) => value.apply(original, args),
+        enumerable: true,
+        configurable: true,
+        writable: true,
+      });
+    } else {
+      (chained as Record<string, unknown>)[key] = value;
+    }
+  }
+
+  return chained;
+};

packages/core/src/utils/handleCallbackErrors.ts

@@ -1,4 +1,5 @@
 import { isThenable } from '../utils/is';
+import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
 
 /* eslint-disable */
 // Vendor "Awaited" in to be TS 3.8 compatible
@@ -62,7 +63,12 @@ export function handleCallbackErrors<
  * Maybe handle a promise rejection.
  * This expects to be given a value that _may_ be a promise, or any other value.
  * If it is a promise, and it rejects, it will call the `onError` callback.
- * Other than this, it will generally return the given value as-is.
+ *
+ * For thenable objects with extra methods (like jQuery's jqXHR),
+ * this function preserves those methods by wrapping the original thenable in a Proxy
+ * that intercepts .then() calls to apply error handling while forwarding all other
+ * properties to the original object.
+ * This allows code like `startSpan(() => $.ajax(...)).abort()` to work correctly.
  */
 function maybeHandlePromiseRejection<MaybePromise>(
   value: MaybePromise,
@@ -71,22 +77,21 @@ function maybeHandlePromiseRejection<MaybePromise>(
   onSuccess: (result: MaybePromise | AwaitedPromise<MaybePromise>) => void,
 ): MaybePromise {
   if (isThenable(value)) {
-    // @ts-expect-error - the isThenable check returns the "wrong" type here
-    return value.then(
-      res => {
+    return chainAndCopyPromiseLike(
+      value as MaybePromise & PromiseLike<Awaited<typeof value>> & Record<string, unknown>,
+      (result) => {
         onFinally();
-        onSuccess(res);
-        return res;
+        onSuccess(result as Awaited<MaybePromise>);
       },
-      e => {
-        onError(e);
-        onFinally();
-        throw e;
+      (err) => {
+        onError(err)
+        onFinally()
       },
-    );
+    ) as MaybePromise;
+  } else {
+    // Non-thenable value - call callbacks immediately and return as-is
+    onFinally();
+    onSuccess(value);
   }
-
-  onFinally();
-  onSuccess(value);
   return value;
 }

packages/core/test/lib/tracing/trace.test.ts

@@ -210,6 +210,32 @@ describe('startSpan', () => {
     });
   });
 
+  describe('AsyncContext withScope promise integrity behavior', () => {
+    it('returns the original promise instance', async () => {
+      const original = Promise.resolve(42);
+      const result = startSpan({}, () => original);
+      expect(result).toBe(original); // New behavior
+    });
+
+    it('returns same instance on multiple calls', () => {
+      const p = Promise.resolve(1);
+      const result1 = startSpan({}, () => p);
+      const result2 = startSpan({}, () => p);
+      expect(result1).toBe(result2);
+    });
+
+    it('preserves custom thenable methods', async () => {
+      const jqXHR = {
+        then: Promise.resolve(1).then.bind(Promise.resolve(1)),
+        abort: vi.fn(),
+      };
+      const result = startSpan({}, () => jqXHR);
+      expect(typeof result.abort).toBe('function');
+      result.abort();
+      expect(jqXHR.abort).toHaveBeenCalled();
+    });
+  });
+
   it('returns a non recording span if tracing is disabled', () => {
     const options = getDefaultTestClientOptions({});
     client = new TestClient(options);