Skip to content

Commit c497238

Browse files
mitsuhikomitsuhiko
authored
feat(utils): Kill SyncPromise.reject and SyncPromise.resolve (#4329)
1 parent b43b496 commit c497238

File tree

12 files changed

+76
-73
lines changed

12 files changed

+76
-73
lines changed

‎packages/browser/src/eventbuilder.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
isErrorEvent,
99
isEvent,
1010
isPlainObject,
11-
SyncPromise,
11+
resolvedSyncPromise,
1212
} from '@sentry/utils';
1313

1414
import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
@@ -28,7 +28,7 @@ export function eventFromException(options: Options, exception: unknown, hint?:
2828
if (hint && hint.event_id) {
2929
event.event_id = hint.event_id;
3030
}
31-
return SyncPromise.resolve(event);
31+
return resolvedSyncPromise(event);
3232
}
3333

3434
/**
@@ -49,7 +49,7 @@ export function eventFromMessage(
4949
if (hint && hint.event_id) {
5050
event.event_id = hint.event_id;
5151
}
52-
return SyncPromise.resolve(event);
52+
return resolvedSyncPromise(event);
5353
}
5454

5555
/**

‎packages/browser/src/sdk.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
2-
import { addInstrumentationHandler, getGlobalObject, logger, SyncPromise } from '@sentry/utils';
2+
import { addInstrumentationHandler, getGlobalObject, logger, resolvedSyncPromise } from '@sentry/utils';
33

44
import { BrowserOptions } from './backend';
55
import { BrowserClient } from './client';
@@ -162,7 +162,7 @@ export function flush(timeout?: number): PromiseLike<boolean> {
162162
return client.flush(timeout);
163163
}
164164
logger.warn('Cannot flush events. No client defined.');
165-
return SyncPromise.resolve(false);
165+
return resolvedSyncPromise(false);
166166
}
167167

168168
/**
@@ -179,7 +179,7 @@ export function close(timeout?: number): PromiseLike<boolean> {
179179
return client.close(timeout);
180180
}
181181
logger.warn('Cannot flush events and disable SDK. No client defined.');
182-
return SyncPromise.resolve(false);
182+
return resolvedSyncPromise(false);
183183
}
184184

185185
/**

‎packages/browser/test/unit/mocks/simpletransport.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { eventStatusFromHttpCode, SyncPromise } from '@sentry/utils';
1+
import { eventStatusFromHttpCode, resolvedSyncPromise } from '@sentry/utils';
22

33
import { Event, Response } from '../../../src';
44
import { BaseTransport } from '../../../src/transports';
55

66
export class SimpleTransport extends BaseTransport {
77
public sendEvent(_: Event): PromiseLike<Response> {
88
return this._buffer.add(() =>
9-
SyncPromise.resolve({
9+
resolvedSyncPromise({
1010
status: eventStatusFromHttpCode(200),
1111
}),
1212
);

‎packages/core/src/baseclient.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {
1919
isThenable,
2020
logger,
2121
normalize,
22+
rejectedSyncPromise,
23+
resolvedSyncPromise,
2224
SentryError,
2325
SyncPromise,
2426
truncate,
@@ -356,7 +358,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
356358
}
357359

358360
// We prepare the result here with a resolved Event.
359-
let result = SyncPromise.resolve<Event | null>(prepared);
361+
let result = resolvedSyncPromise<Event | null>(prepared);
360362

361363
// This should be the last thing called, since we want that
362364
// {@link Hub.addEventProcessor} gets the finished prepared event.
@@ -531,7 +533,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
531533
}
532534

533535
if (!this._isEnabled()) {
534-
return SyncPromise.reject(new SentryError('SDK not enabled, will not capture event.'));
536+
return rejectedSyncPromise(new SentryError('SDK not enabled, will not capture event.'));
535537
}
536538

537539
const isTransaction = event.type === 'transaction';
@@ -540,7 +542,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
540542
// Sampling for transaction happens somewhere else
541543
if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) {
542544
recordLostEvent('sample_rate', 'event');
543-
return SyncPromise.reject(
545+
return rejectedSyncPromise(
544546
new SentryError(
545547
`Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
546548
),

‎packages/core/src/transports/noop.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { Event, Response, Transport } from '@sentry/types';
2-
import { SyncPromise } from '@sentry/utils';
2+
import { resolvedSyncPromise } from '@sentry/utils';
33

44
/** Noop transport */
55
export class NoopTransport implements Transport {
66
/**
77
* @inheritDoc
88
*/
99
public sendEvent(_: Event): PromiseLike<Response> {
10-
return SyncPromise.resolve({
10+
return resolvedSyncPromise({
1111
reason: `NoopTransport: Event has been skipped because no Dsn is configured.`,
1212
status: 'skipped',
1313
});
@@ -17,6 +17,6 @@ export class NoopTransport implements Transport {
1717
* @inheritDoc
1818
*/
1919
public close(_?: number): PromiseLike<boolean> {
20-
return SyncPromise.resolve(true);
20+
return resolvedSyncPromise(true);
2121
}
2222
}

‎packages/core/test/mocks/backend.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Session } from '@sentry/hub';
22
import { Event, Options, SeverityLevel, Transport } from '@sentry/types';
3-
import { SyncPromise } from '@sentry/utils';
3+
import { resolvedSyncPromise } from '@sentry/utils';
44

55
import { BaseBackend } from '../../src/basebackend';
66

@@ -24,7 +24,7 @@ export class TestBackend extends BaseBackend<TestOptions> {
2424

2525
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
2626
public eventFromException(exception: any): PromiseLike<Event> {
27-
return SyncPromise.resolve({
27+
return resolvedSyncPromise({
2828
exception: {
2929
values: [
3030
{
@@ -39,7 +39,7 @@ export class TestBackend extends BaseBackend<TestOptions> {
3939
}
4040

4141
public eventFromMessage(message: string, level: SeverityLevel = 'info'): PromiseLike<Event> {
42-
return SyncPromise.resolve({ message, level });
42+
return resolvedSyncPromise({ message, level });
4343
}
4444

4545
public sendEvent(event: Event): void {

‎packages/node/src/integrations/linkederrors.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
22
import { Event, EventHint, Exception, ExtendedError, Integration } from '@sentry/types';
3-
import { isInstanceOf, SyncPromise } from '@sentry/utils';
3+
import { isInstanceOf, resolvedSyncPromise, SyncPromise } from '@sentry/utils';
44

55
import { getExceptionFromError } from '../parsers';
66

@@ -56,7 +56,7 @@ export class LinkedErrors implements Integration {
5656
*/
5757
private _handler(event: Event, hint?: EventHint): PromiseLike<Event> {
5858
if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) {
59-
return SyncPromise.resolve(event);
59+
return resolvedSyncPromise(event);
6060
}
6161

6262
return new SyncPromise<Event>(resolve => {
@@ -78,7 +78,7 @@ export class LinkedErrors implements Integration {
7878
*/
7979
private _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): PromiseLike<Exception[]> {
8080
if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) {
81-
return SyncPromise.resolve(stack);
81+
return resolvedSyncPromise(stack);
8282
}
8383
return new SyncPromise<Exception[]>((resolve, reject) => {
8484
void getExceptionFromError(error[key])

‎packages/node/src/parsers.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Event, Exception, ExtendedError, StackFrame } from '@sentry/types';
2-
import { addContextToFrame, basename, dirname, SyncPromise } from '@sentry/utils';
2+
import { addContextToFrame, basename, dirname, resolvedSyncPromise, SyncPromise } from '@sentry/utils';
33
import { readFile } from 'fs';
44
import { LRUMap } from 'lru_map';
55

@@ -71,7 +71,7 @@ function getModule(filename: string, base?: string): string {
7171
function readSourceFiles(filenames: string[]): PromiseLike<{ [key: string]: string | null }> {
7272
// we're relying on filenames being de-duped already
7373
if (filenames.length === 0) {
74-
return SyncPromise.resolve({});
74+
return resolvedSyncPromise({});
7575
}
7676

7777
return new SyncPromise<{
@@ -176,15 +176,15 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions
176176

177177
// We do an early return if we do not want to fetch context liens
178178
if (linesOfContext <= 0) {
179-
return SyncPromise.resolve(frames);
179+
return resolvedSyncPromise(frames);
180180
}
181181

182182
try {
183183
return addPrePostContext(filesToRead, frames, linesOfContext);
184184
} catch (_) {
185185
// This happens in electron for example where we are not able to read files from asar.
186186
// So it's fine, we recover be just returning all frames without pre/post context.
187-
return SyncPromise.resolve(frames);
187+
return resolvedSyncPromise(frames);
188188
}
189189
}
190190

‎packages/utils/src/promisebuffer.ts

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,5 @@
11
import { SentryError } from './error';
2-
import { SyncPromise } from './syncpromise';
3-
4-
function allPromises<U = unknown>(collection: Array<U | PromiseLike<U>>): PromiseLike<U[]> {
5-
return new SyncPromise<U[]>((resolve, reject) => {
6-
if (collection.length === 0) {
7-
resolve(null);
8-
return;
9-
}
10-
11-
let counter = collection.length;
12-
collection.forEach(item => {
13-
void SyncPromise.resolve(item)
14-
.then(() => {
15-
// eslint-disable-next-line no-plusplus
16-
if (--counter === 0) {
17-
resolve(null);
18-
}
19-
})
20-
.then(null, reject);
21-
});
22-
});
23-
}
2+
import { rejectedSyncPromise, resolvedSyncPromise, SyncPromise } from './syncpromise';
243

254
export interface PromiseBuffer<T> {
265
// exposes the internal array so tests can assert on the state of it.
@@ -63,7 +42,7 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
6342
*/
6443
function add(taskProducer: () => PromiseLike<T>): PromiseLike<T> {
6544
if (!isReady()) {
66-
return SyncPromise.reject(new SentryError('Not adding Promise due to buffer limit reached.'));
45+
return rejectedSyncPromise(new SentryError('Not adding Promise due to buffer limit reached.'));
6746
}
6847

6948
// start the task and add its promise to the queue
@@ -94,7 +73,13 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
9473
* `false` otherwise
9574
*/
9675
function drain(timeout?: number): PromiseLike<boolean> {
97-
return new SyncPromise<boolean>(resolve => {
76+
return new SyncPromise<boolean>((resolve, reject) => {
77+
let counter = buffer.length;
78+
79+
if (!counter) {
80+
return resolve(true);
81+
}
82+
9883
// wait for `timeout` ms and then resolve to `false` (if not cancelled first)
9984
const capturedSetTimeout = setTimeout(() => {
10085
if (timeout && timeout > 0) {
@@ -103,9 +88,14 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
10388
}, timeout);
10489

10590
// if all promises resolve in time, cancel the timer and resolve to `true`
106-
void allPromises(buffer).then(() => {
107-
clearTimeout(capturedSetTimeout);
108-
resolve(true);
91+
buffer.forEach(item => {
92+
void resolvedSyncPromise(item).then(() => {
93+
// eslint-disable-next-line no-plusplus
94+
if (!--counter) {
95+
clearTimeout(capturedSetTimeout);
96+
resolve(true);
97+
}
98+
}, reject);
10999
});
110100
});
111101
}

‎packages/utils/src/syncpromise.ts

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,30 @@ const enum States {
1414
REJECTED = 2,
1515
}
1616

17+
/**
18+
* Creates a resolved sync promise.
19+
*
20+
* @param value the value to resolve the promise with
21+
* @returns the resolved sync promise
22+
*/
23+
export function resolvedSyncPromise<T>(value: T | PromiseLike<T>): PromiseLike<T> {
24+
return new SyncPromise(resolve => {
25+
resolve(value);
26+
});
27+
}
28+
29+
/**
30+
* Creates a rejected sync promise.
31+
*
32+
* @param value the value to reject the promise with
33+
* @returns the rejected sync promise
34+
*/
35+
export function rejectedSyncPromise<T = never>(reason?: any): PromiseLike<T> {
36+
return new SyncPromise((_, reject) => {
37+
reject(reason);
38+
});
39+
}
40+
1741
/**
1842
* Thenable class that behaves like a Promise and follows it's interface
1943
* but is not async internally
@@ -33,20 +57,6 @@ class SyncPromise<T> implements PromiseLike<T> {
3357
}
3458
}
3559

36-
/** JSDoc */
37-
public static resolve<T>(value: T | PromiseLike<T>): PromiseLike<T> {
38-
return new SyncPromise(resolve => {
39-
resolve(value);
40-
});
41-
}
42-
43-
/** JSDoc */
44-
public static reject<T = never>(reason?: any): PromiseLike<T> {
45-
return new SyncPromise((_, reject) => {
46-
reject(reason);
47-
});
48-
}
49-
5060
/** JSDoc */
5161
public then<TResult1 = T, TResult2 = never>(
5262
onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | null,

‎packages/utils/test/is.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
import { resolvedSyncPromise } from '../dist';
12
import { isDOMError, isDOMException, isError, isErrorEvent, isInstanceOf, isPrimitive, isThenable } from '../src/is';
23
import { supportsDOMError, supportsDOMException, supportsErrorEvent } from '../src/supports';
3-
import { SyncPromise } from '../src/syncpromise';
44

55
class SentryError extends Error {
66
public name: string;
@@ -75,7 +75,7 @@ describe('isPrimitive()', () => {
7575
describe('isThenable()', () => {
7676
test('should work as advertised', () => {
7777
expect(isThenable(Promise.resolve(true))).toEqual(true);
78-
expect(isThenable(SyncPromise.resolve(true))).toEqual(true);
78+
expect(isThenable(resolvedSyncPromise(true))).toEqual(true);
7979

8080
expect(isThenable(undefined)).toEqual(false);
8181
expect(isThenable(null)).toEqual(false);

‎packages/utils/test/syncpromise.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { rejectedSyncPromise, resolvedSyncPromise } from '../dist';
12
import { SyncPromise } from '../src/syncpromise';
23

34
describe('SyncPromise', () => {
@@ -17,9 +18,9 @@ describe('SyncPromise', () => {
1718
return new SyncPromise<number>(resolve => {
1819
resolve(42);
1920
})
20-
.then(_ => SyncPromise.resolve('a'))
21-
.then(_ => SyncPromise.resolve(0.1))
22-
.then(_ => SyncPromise.resolve(false))
21+
.then(_ => resolvedSyncPromise('a'))
22+
.then(_ => resolvedSyncPromise(0.1))
23+
.then(_ => resolvedSyncPromise(false))
2324
.then(val => {
2425
expect(val).toBe(false);
2526
});
@@ -84,7 +85,7 @@ describe('SyncPromise', () => {
8485
return (
8586
c
8687
// @ts-ignore Argument of type 'PromiseLike<string>' is not assignable to parameter of type 'SyncPromise<string>'
87-
.then(val => f(SyncPromise.resolve('x'), val))
88+
.then(val => f(resolvedSyncPromise('x'), val))
8889
.then(val => f(b, val))
8990
// @ts-ignore Argument of type 'SyncPromise<string>' is not assignable to parameter of type 'string'
9091
.then(val => f(a, val))
@@ -97,7 +98,7 @@ describe('SyncPromise', () => {
9798
test('simple static', () => {
9899
expect.assertions(1);
99100

100-
const p = SyncPromise.resolve(10);
101+
const p = resolvedSyncPromise(10);
101102
return p.then(val => {
102103
expect(val).toBe(10);
103104
});
@@ -260,7 +261,7 @@ describe('SyncPromise', () => {
260261
})
261262
.then(value => {
262263
expect(value).toEqual(2);
263-
return SyncPromise.reject('wat');
264+
return rejectedSyncPromise('wat');
264265
})
265266
.then(null, reason => {
266267
expect(reason).toBe('wat');

0 commit comments

Comments
 (0)