Skip to content

Commit 58c695d

Browse files
committed
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.
1 parent b64405a commit 58c695d

File tree

3 files changed

+62
-28
lines changed

3 files changed

+62
-28
lines changed

packages/core/src/asyncContext/stackStrategy.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Client } from '../client';
22
import { getDefaultCurrentScope, getDefaultIsolationScope } from '../defaultScopes';
33
import { Scope } from '../scope';
4+
import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
45
import { isThenable } from '../utils/is';
56
import { getMainCarrier, getSentryCarrier } from './../carrier';
67
import type { AsyncContextStrategy } from './types';
@@ -52,20 +53,11 @@ export class AsyncContextStack {
5253
}
5354

5455
if (isThenable(maybePromiseResult)) {
55-
// Attach handlers but still return original promise
56-
maybePromiseResult.then(
57-
res => {
58-
this._popScope();
59-
return res;
60-
},
61-
_e => {
62-
this._popScope();
63-
// 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.
64-
// Re-throwing it here would cause unhandled promise rejections.
65-
},
66-
);
67-
68-
return maybePromiseResult;
56+
return chainAndCopyPromiseLike(
57+
maybePromiseResult as PromiseLike<Awaited<typeof maybePromiseResult>> & Record<string, unknown>,
58+
() => this._popScope(),
59+
() => this._popScope(),
60+
) as T;
6961
}
7062

7163
this._popScope();
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* Copy the properties from a decorated promiselike object onto its chained
3+
* actual promise.
4+
*/
5+
export const chainAndCopyPromiseLike = <V, T extends PromiseLike<V> & Record<string, unknown>>(
6+
original: T,
7+
onSuccess: (value: V) => void,
8+
onError: (e: unknown) => void,
9+
): T => {
10+
return copyProps(
11+
original,
12+
original.then(
13+
value => {
14+
onSuccess(value);
15+
return value;
16+
},
17+
err => {
18+
onError(err);
19+
throw err;
20+
},
21+
),
22+
) as T;
23+
};
24+
25+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
26+
const copyProps = <T extends Record<string, any>>(original: T, chained: T): T => {
27+
for (const key in original) {
28+
if (key in chained) continue;
29+
const value = original[key];
30+
if (typeof value === 'function') {
31+
Object.defineProperty(chained, key, {
32+
value: (...args: unknown[]) => value.apply(original, args),
33+
enumerable: true,
34+
configurable: true,
35+
writable: true,
36+
});
37+
} else {
38+
(chained as Record<string, unknown>)[key] = value;
39+
}
40+
}
41+
42+
return chained;
43+
};

packages/core/src/utils/handleCallbackErrors.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { isThenable } from '../utils/is';
2+
import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
23

34
/* eslint-disable */
45
// Vendor "Awaited" in to be TS 3.8 compatible
@@ -76,23 +77,21 @@ function maybeHandlePromiseRejection<MaybePromise>(
7677
onSuccess: (result: MaybePromise | AwaitedPromise<MaybePromise>) => void,
7778
): MaybePromise {
7879
if (isThenable(value)) {
79-
// Attach handlers but still return original promise
80-
value.then(
81-
res => {
80+
return chainAndCopyPromiseLike(
81+
value as MaybePromise & PromiseLike<Awaited<typeof value>> & Record<string, unknown>,
82+
(result) => {
8283
onFinally();
83-
onSuccess(res);
84+
onSuccess(result as Awaited<MaybePromise>);
8485
},
85-
err => {
86-
onError(err);
87-
onFinally();
86+
(err) => {
87+
onError(err)
88+
onFinally()
8889
},
89-
);
90-
91-
return value;
90+
) as MaybePromise;
91+
} else {
92+
// Non-thenable value - call callbacks immediately and return as-is
93+
onFinally();
94+
onSuccess(value);
9295
}
93-
94-
// Non-thenable value - call callbacks immediately and return as-is
95-
onFinally();
96-
onSuccess(value);
9796
return value;
9897
}

0 commit comments

Comments
 (0)