wip: Copy properties onto chained promises

This might be an acceptable compromise, so long as it doesn't blow up
our bundle size too much.

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.

Author: isaacs

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,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,3 +1,4 @@
+import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
 import { isThenable } from '../utils/is';
 
 /* eslint-disable */
@@ -76,23 +77,21 @@ 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;
+  } else {
+    // Non-thenable value - call callbacks immediately and return as-is
+    onFinally();
+    onSuccess(value);
   }
-
-  // Non-thenable value - call callbacks immediately and return as-is
-  onFinally();
-  onSuccess(value);
   return value;
 }

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

@@ -212,24 +212,36 @@ 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
+      const original = Object.assign(Promise.resolve(42), {
+        value: 123,
+        getValue() {
+          return this.value;
+        },
+      });
+      const result = startSpan({ name: 'test' }, () => original);
+      // preserve add-on methods on handled promises
+      expect(result.getValue()).toBe(original.getValue());
     });
 
     it('returns same instance on multiple calls', () => {
-      const p = Promise.resolve(1);
-      const result1 = startSpan({}, () => p);
-      const result2 = startSpan({}, () => p);
-      expect(result1).toBe(result2);
+      const p = Object.assign(Promise.resolve(1), {
+        value: 42,
+        getValue() {
+          return this.value;
+        },
+      });
+      const result1 = startSpan({ name: 'test' }, () => p);
+      const result2 = startSpan({ name: 'test' }, () => p);
+      expect(result1.getValue()).toBe(42);
+      expect(result2.getValue()).toBe(42);
     });
 
     it('preserves custom thenable methods', async () => {
       const jqXHR = {
         then: Promise.resolve(1).then.bind(Promise.resolve(1)),
         abort: vi.fn(),
       };
-      const result = startSpan({}, () => jqXHR);
+      const result = startSpan({ name: 'test' }, () => jqXHR);
       expect(typeof result.abort).toBe('function');
       result.abort();
       expect(jqXHR.abort).toHaveBeenCalled();