Skip to content

Commit a012502

Browse files
committed
feat(core): Use Promise.try where available
We use this instead of our own `SyncPromise` implementation where available. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/try for details.
1 parent c084bd6 commit a012502

File tree

8 files changed

+80
-46
lines changed

8 files changed

+80
-46
lines changed

packages/core/src/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
10791079
* @param event The event to send to Sentry.
10801080
* @param hint May contain additional information about the original exception.
10811081
* @param currentScope A scope containing event metadata.
1082-
* @returns A SyncPromise that resolves with the event or rejects in case event was/will not be send.
1082+
* @returns A PromiseLike that resolves with the event or rejects in case event was/will not be send.
10831083
*/
10841084
protected _processEvent(
10851085
event: Event,

packages/core/src/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,13 @@ export {
242242
supportsReferrerPolicy,
243243
supportsReportingObserver,
244244
} from './utils/supports';
245-
export { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils/syncpromise';
245+
export {
246+
// eslint-disable-next-line deprecation/deprecation
247+
SyncPromise,
248+
rejectedSyncPromise,
249+
resolvedSyncPromise,
250+
makeSyncPromise,
251+
} from './utils/syncpromise';
246252
export { browserPerformanceTimeOrigin, dateTimestampInSeconds, timestampInSeconds } from './utils/time';
247253
export {
248254
TRACEPARENT_REGEXP,

packages/core/src/utils/promisebuffer.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { rejectedSyncPromise, resolvedSyncPromise, SyncPromise } from './syncpromise';
1+
import { makeSyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './syncpromise';
22

33
export interface PromiseBuffer<T> {
44
// exposes the internal array so tests can assert on the state of it.
@@ -74,28 +74,30 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
7474
* `false` otherwise
7575
*/
7676
function drain(timeout?: number): PromiseLike<boolean> {
77-
return new SyncPromise<boolean>((resolve, reject) => {
77+
return makeSyncPromise<boolean>(() => {
7878
let counter = buffer.length;
7979

8080
if (!counter) {
81-
return resolve(true);
81+
return true;
8282
}
8383

84-
// wait for `timeout` ms and then resolve to `false` (if not cancelled first)
85-
const capturedSetTimeout = setTimeout(() => {
86-
if (timeout && timeout > 0) {
87-
resolve(false);
88-
}
89-
}, timeout);
90-
91-
// if all promises resolve in time, cancel the timer and resolve to `true`
92-
buffer.forEach(item => {
93-
void resolvedSyncPromise(item).then(() => {
94-
if (!--counter) {
95-
clearTimeout(capturedSetTimeout);
96-
resolve(true);
84+
return new Promise<boolean>((resolve, reject) => {
85+
// wait for `timeout` ms and then resolve to `false` (if not cancelled first)
86+
const capturedSetTimeout = setTimeout(() => {
87+
if (timeout && timeout > 0) {
88+
resolve(false);
9789
}
98-
}, reject);
90+
}, timeout);
91+
92+
// if all promises resolve in time, cancel the timer and resolve to `true`
93+
buffer.forEach(item => {
94+
void resolvedSyncPromise(item).then(() => {
95+
if (!--counter) {
96+
clearTimeout(capturedSetTimeout);
97+
resolve(true);
98+
}
99+
}, reject);
100+
});
99101
});
100102
});
101103
}

packages/core/src/utils/syncpromise.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,28 @@ const STATE_RESOLVED = 1;
77
const STATE_REJECTED = 2;
88

99
type State = typeof STATE_PENDING | typeof STATE_RESOLVED | typeof STATE_REJECTED;
10+
type Executor<T> = (resolve: (value?: T | PromiseLike<T> | null) => void, reject: (reason?: any) => void) => void;
11+
type PromiseTry = <T>(executor: Executor<T>) => PromiseLike<T>;
12+
type PromiseWithTry = PromiseConstructor & { try: PromiseTry };
13+
14+
/**
15+
* Takes an executor and returns a promise that is executed synchronously (if the executor is synchronous) or else asynchronously.
16+
* It always returns a promise.
17+
*
18+
* This uses the native Promise.try, if it exists, else our SyncPromise implementation.
19+
*/
20+
export function makeSyncPromise<T>(executor: Executor<T>): PromiseLike<T> {
21+
if (hasPromiseTry(Promise)) {
22+
return Promise.try(executor);
23+
}
24+
25+
// eslint-disable-next-line deprecation/deprecation
26+
return new SyncPromise(executor);
27+
}
28+
29+
function hasPromiseTry(Promise: typeof globalThis.Promise): Promise is PromiseWithTry {
30+
return 'try' in Promise && typeof Promise.try === 'function';
31+
}
1032

1133
// Overloads so we can call resolvedSyncPromise without arguments and generic argument
1234
export function resolvedSyncPromise(): PromiseLike<void>;
@@ -19,9 +41,7 @@ export function resolvedSyncPromise<T>(value: T | PromiseLike<T>): PromiseLike<T
1941
* @returns the resolved sync promise
2042
*/
2143
export function resolvedSyncPromise<T>(value?: T | PromiseLike<T>): PromiseLike<T> {
22-
return new SyncPromise(resolve => {
23-
resolve(value);
24-
});
44+
return makeSyncPromise(() => value);
2545
}
2646

2747
/**
@@ -31,16 +51,16 @@ export function resolvedSyncPromise<T>(value?: T | PromiseLike<T>): PromiseLike<
3151
* @returns the rejected sync promise
3252
*/
3353
export function rejectedSyncPromise<T = never>(reason?: any): PromiseLike<T> {
34-
return new SyncPromise((_, reject) => {
35-
reject(reason);
54+
return makeSyncPromise(() => {
55+
throw reason;
3656
});
3757
}
3858

39-
type Executor<T> = (resolve: (value?: T | PromiseLike<T> | null) => void, reject: (reason?: any) => void) => void;
40-
4159
/**
4260
* Thenable class that behaves like a Promise and follows it's interface
4361
* but is not async internally
62+
*
63+
* @deprecated Use makeSyncPromise instead.
4464
*/
4565
export class SyncPromise<T> implements PromiseLike<T> {
4666
private _state: State;
@@ -59,6 +79,7 @@ export class SyncPromise<T> implements PromiseLike<T> {
5979
onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | null,
6080
onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null,
6181
): PromiseLike<TResult1 | TResult2> {
82+
// eslint-disable-next-line deprecation/deprecation
6283
return new SyncPromise((resolve, reject) => {
6384
this._handlers.push([
6485
false,
@@ -100,6 +121,7 @@ export class SyncPromise<T> implements PromiseLike<T> {
100121

101122
/** @inheritdoc */
102123
public finally<TResult>(onfinally?: (() => void) | null): PromiseLike<TResult> {
124+
// eslint-disable-next-line deprecation/deprecation
103125
return new SyncPromise<TResult>((resolve, reject) => {
104126
let val: TResult | any;
105127
let isRejected: boolean;

packages/core/test/lib/client.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
makeSession,
99
Scope,
1010
setCurrentClient,
11-
SyncPromise,
1211
withMonitor,
1312
} from '../../src';
1413
import * as integrationModule from '../../src/integration';
@@ -2111,7 +2110,7 @@ describe('Client', () => {
21112110
const spy = vi.spyOn(TestClient.instance!, 'eventFromMessage');
21122111
spy.mockImplementationOnce(
21132112
(message, level) =>
2114-
new SyncPromise(resolve => {
2113+
new Promise(resolve => {
21152114
setTimeout(() => resolve({ message, level }), 150);
21162115
}),
21172116
);

packages/core/test/lib/utils/promisebuffer.test.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { describe, expect, test, vi } from 'vitest';
22
import { makePromiseBuffer } from '../../../src/utils/promisebuffer';
3-
import { SyncPromise } from '../../../src/utils/syncpromise';
3+
import { makeSyncPromise } from '../../../src/utils/syncpromise';
44

55
describe('PromiseBuffer', () => {
66
describe('add()', () => {
77
test('no limit', () => {
88
const buffer = makePromiseBuffer();
9-
const p = vi.fn(() => new SyncPromise(resolve => setTimeout(resolve)));
9+
const p = vi.fn(() => makeSyncPromise(() => {}));
1010
void buffer.add(p);
1111
expect(buffer.$.length).toEqual(1);
1212
});
@@ -15,10 +15,10 @@ describe('PromiseBuffer', () => {
1515
const buffer = makePromiseBuffer(1);
1616
let task1;
1717
const producer1 = vi.fn(() => {
18-
task1 = new SyncPromise(resolve => setTimeout(resolve));
18+
task1 = makeSyncPromise(() => {});
1919
return task1;
2020
});
21-
const producer2 = vi.fn(() => new SyncPromise(resolve => setTimeout(resolve)));
21+
const producer2 = vi.fn(() => makeSyncPromise(() => {}));
2222
expect(buffer.add(producer1)).toEqual(task1);
2323
void expect(buffer.add(producer2)).rejects.toThrowError();
2424
expect(buffer.$.length).toEqual(1);
@@ -31,7 +31,7 @@ describe('PromiseBuffer', () => {
3131
test('without timeout', async () => {
3232
const buffer = makePromiseBuffer();
3333
for (let i = 0; i < 5; i++) {
34-
void buffer.add(() => new SyncPromise(resolve => setTimeout(resolve)));
34+
void buffer.add(() => makeSyncPromise(() => {}));
3535
}
3636
expect(buffer.$.length).toEqual(5);
3737
const result = await buffer.drain();
@@ -42,7 +42,7 @@ describe('PromiseBuffer', () => {
4242
test('with timeout', async () => {
4343
const buffer = makePromiseBuffer();
4444
for (let i = 0; i < 5; i++) {
45-
void buffer.add(() => new SyncPromise(resolve => setTimeout(resolve, 100)));
45+
void buffer.add(() => new Promise(resolve => setTimeout(resolve, 100)));
4646
}
4747
expect(buffer.$.length).toEqual(5);
4848
const result = await buffer.drain(50);
@@ -60,7 +60,7 @@ describe('PromiseBuffer', () => {
6060

6161
test('resolved promises should not show up in buffer length', async () => {
6262
const buffer = makePromiseBuffer();
63-
const producer = () => new SyncPromise(resolve => setTimeout(resolve));
63+
const producer = () => makeSyncPromise(() => {});
6464
const task = buffer.add(producer);
6565
expect(buffer.$.length).toEqual(1);
6666
await task;
@@ -69,7 +69,10 @@ describe('PromiseBuffer', () => {
6969

7070
test('rejected promises should not show up in buffer length', async () => {
7171
const buffer = makePromiseBuffer();
72-
const producer = () => new SyncPromise((_, reject) => setTimeout(reject));
72+
const producer = () =>
73+
makeSyncPromise(() => {
74+
throw new Error('whoops');
75+
});
7376
const task = buffer.add(producer);
7477
expect(buffer.$.length).toEqual(1);
7578
try {
@@ -82,7 +85,7 @@ describe('PromiseBuffer', () => {
8285

8386
test('resolved task should give an access to the return value', async () => {
8487
const buffer = makePromiseBuffer<string>();
85-
const producer = () => new SyncPromise<string>(resolve => setTimeout(() => resolve('test')));
88+
const producer = () => makeSyncPromise<string>(() => 'test');
8689
const task = buffer.add(producer);
8790
const result = await task;
8891
expect(result).toEqual('test');
@@ -91,7 +94,10 @@ describe('PromiseBuffer', () => {
9194
test('rejected task should give an access to the return value', async () => {
9295
expect.assertions(1);
9396
const buffer = makePromiseBuffer<string>();
94-
const producer = () => new SyncPromise<string>((_, reject) => setTimeout(() => reject(new Error('whoops'))));
97+
const producer = () =>
98+
makeSyncPromise<string>(() => {
99+
throw new Error('whoops');
100+
});
95101
const task = buffer.add(producer);
96102
try {
97103
await task;

packages/core/test/lib/utils/syncpromise.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable deprecation/deprecation */
12
import { describe, expect, test, vi } from 'vitest';
23
import { rejectedSyncPromise, resolvedSyncPromise, SyncPromise } from '../../../src/utils/syncpromise';
34

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import { createTransport } from '../../src/transports/base';
22
import type { Transport } from '../../src/types-hoist/transport';
3-
import { SyncPromise } from '../../src/utils/syncpromise';
43

54
async function sleep(delay: number): Promise<void> {
6-
return new SyncPromise(resolve => setTimeout(resolve, delay));
5+
return new Promise(resolve => setTimeout(resolve, delay));
76
}
87

98
export function makeFakeTransport(delay: number = 2000): {
@@ -15,13 +14,12 @@ export function makeFakeTransport(delay: number = 2000): {
1514
let sendCalled = 0;
1615
let sentCount = 0;
1716
const makeTransport = () =>
18-
createTransport({ recordDroppedEvent: () => undefined }, () => {
17+
createTransport({ recordDroppedEvent: () => undefined }, async () => {
1918
sendCalled++;
20-
return new SyncPromise(async res => {
21-
await sleep(delay);
22-
sentCount++;
23-
res({});
24-
});
19+
await sleep(delay);
20+
sentCount++;
21+
return {};
2522
});
23+
2624
return { makeTransport, getSendCalled: () => sendCalled, getSentCount: () => sentCount, delay };
2725
}

0 commit comments

Comments
 (0)