fix: Copy properties onto Sentry-chained promises

Copy properties from the original promise onto the chained tracker, so
that we can return something that can inspect the error, does not
obscure `unhandledrejection`, *and* supports jQuery's extended
PromiseLike objects.

We only do this if the original PromiseLike object or its chained
counterpart are not `instanceof Promise`. So far, we have not
encountered such decoration on "normal" Promise objects, so this is a
fast way to ensure that we're only providing this affordance where it's
needed. This does introduce a limitation that if someone decorates a
standard Promise object, and its decorations do not extend to chained
results of `then()`, then we will lose them. If and when that happens,
we can consider extending this check to something more thorough, even if
it's slightly less performant.

A guard is added to prevent cases where a chained/copied promise is
passed through this function again, so that we know we still have to do
the operation even though it's a "normal" Promise.

Author: isaacs

.size-limit.js

@@ -148,7 +148,7 @@ module.exports = [
     import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
     ignore: ['react/jsx-runtime'],
     gzip: true,
-    limit: '45 KB',
+    limit: '45.1 KB',
   },
   // Vue SDK (ESM)
   {

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,20 +53,11 @@ export class AsyncContextStack {
     }
 
     if (isThenable(maybePromiseResult)) {
-      // Attach handlers but still return original promise
-      maybePromiseResult.then(
-        res => {
-          this._popScope();
-          return res;
-        },
-        _e => {
-          this._popScope();
-          // We don't re-throw the error here because the caller already receives the original rejection, and it's being handled by the caller of withScope.
-          // Re-throwing it here would cause unhandled promise rejections.
-        },
-      );
-
-      return maybePromiseResult;
+      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,55 @@
+const isActualPromise = (p: unknown) =>
+  p instanceof Promise && !(p as unknown as ChainedPromiseLike<unknown>)[kChainedCopy];
+
+type ChainedPromiseLike<T> = PromiseLike<T> & {
+  [kChainedCopy]: true;
+};
+const kChainedCopy = Symbol('chained PromiseLike');
+
+/**
+ * Copy the properties from a decorated promiselike object onto its chained
+ * actual promise.
+ */
+export const chainAndCopyPromiseLike = <V, T extends PromiseLike<V>>(
+  original: T,
+  onSuccess: (value: V) => void,
+  onError: (e: unknown) => void,
+): T => {
+  const chained = original.then(
+    value => {
+      onSuccess(value);
+      return value;
+    },
+    err => {
+      onError(err);
+      throw err;
+    },
+  ) as T;
+
+  // if we're just dealing with "normal" Promise objects, return the chain
+  return isActualPromise(chained) && isActualPromise(original) ? chained : copyProps(original, chained);
+};
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const copyProps = <T extends Record<string, any>>(original: T, chained: T): T => {
+  let mutated = false;
+  //oxlint-disable-next-line guard-for-in
+  for (const key in original) {
+    if (key in chained) continue;
+    mutated = true;
+    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;
+    }
+  }
+
+  if (mutated) Object.assign(chained, { [kChainedCopy]: true });
+  return chained;
+};

packages/core/src/utils/handleCallbackErrors.ts

@@ -1,3 +1,4 @@
+import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
 import { isThenable } from '../utils/is';
 
 /* eslint-disable */
@@ -76,21 +77,18 @@ function maybeHandlePromiseRejection<MaybePromise>(
   onSuccess: (result: MaybePromise | AwaitedPromise<MaybePromise>) => void,
 ): MaybePromise {
   if (isThenable(value)) {
-    // Attach handlers but still return original promise
-    value.then(
-      res => {
+    return chainAndCopyPromiseLike(
+      value as MaybePromise & PromiseLike<Awaited<typeof value>> & Record<string, unknown>,
+      result => {
         onFinally();
-        onSuccess(res);
+        onSuccess(result as Awaited<MaybePromise>);
       },
       err => {
         onError(err);
         onFinally();
       },
-    );
-
-    return value;
+    ) as MaybePromise;
   }
-
   // Non-thenable value - call callbacks immediately and return as-is
   onFinally();
   onSuccess(value);

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

@@ -211,25 +211,13 @@ 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(jqXHR instanceof Promise).toBe(false);
+      const result = startSpan({ name: 'test' }, () => jqXHR);
       expect(typeof result.abort).toBe('function');
       result.abort();
       expect(jqXHR.abort).toHaveBeenCalled();

packages/core/test/lib/utils/chain-and-copy-promiselike.test.ts

@@ -0,0 +1,56 @@
+import { describe, it, expect } from 'vitest';
+import { chainAndCopyPromiseLike } from '../../../src/utils/chain-and-copy-promiselike';
+
+describe('chain and copy promiselike objects', () => {
+  it('does no copying for normal promises', async () => {
+    const p = new Promise<number>(res => res(1));
+    Object.assign(p, { newProperty: true });
+    let success = false;
+    let error = false;
+    const q = chainAndCopyPromiseLike(
+      p,
+      () => {
+        success = true;
+      },
+      () => {
+        error = true;
+      },
+    );
+    expect(await q).toBe(1);
+    //@ts-expect-error - this is not a normal prop on Promises
+    expect(q.newProperty).toBe(undefined);
+    expect(success).toBe(true);
+    expect(error).toBe(false);
+  });
+
+  it('copies properties of non-Promise then-ables', async () => {
+    class FakePromise<T extends unknown> {
+      value: T;
+      constructor(value: T) {
+        this.value = value;
+      }
+      then(fn: (value: T) => unknown) {
+        const newVal = fn(this.value);
+        return new FakePromise(newVal);
+      }
+    }
+    const p = new FakePromise(1) as PromiseLike<number>;
+    Object.assign(p, { newProperty: true });
+    let success = false;
+    let error = false;
+    const q = chainAndCopyPromiseLike(
+      p,
+      () => {
+        success = true;
+      },
+      () => {
+        error = true;
+      },
+    );
+    expect(await q).toBe(1);
+    //@ts-expect-error - this is not a normal prop on FakePromises
+    expect(q.newProperty).toBe(true);
+    expect(success).toBe(true);
+    expect(error).toBe(false);
+  });
+});