From b6934a5ba6984e787694fc203b49ce654cc5d0f6 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 22:50:18 -0500 Subject: [PATCH 1/6] ref(utils): Extract SyncPromise static methods into func --- packages/utils/src/syncpromise.ts | 56 ++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts index 4e5e798e94e4..9a0fe73561f3 100644 --- a/packages/utils/src/syncpromise.ts +++ b/packages/utils/src/syncpromise.ts @@ -18,7 +18,7 @@ const enum States { * Thenable class that behaves like a Promise and follows it's interface * but is not async internally */ -class SyncPromise implements PromiseLike { +export class SyncPromise implements PromiseLike { private _state: States = States.PENDING; private _handlers: Array<[boolean, (value: T) => void, (reason: any) => any]> = []; private _value: any; @@ -35,16 +35,12 @@ class SyncPromise implements PromiseLike { /** JSDoc */ public static resolve(value: T | PromiseLike): PromiseLike { - return new SyncPromise(resolve => { - resolve(value); - }); + return syncPromiseResolve(value); } /** JSDoc */ public static reject(reason?: any): PromiseLike { - return new SyncPromise((_, reject) => { - reject(reason); - }); + return syncPromiseReject(reason); } /** JSDoc */ @@ -178,4 +174,48 @@ class SyncPromise implements PromiseLike { }; } -export { SyncPromise }; +/** JSDoc */ +export function syncPromiseResolve(value: T | PromiseLike): PromiseLike { + return new SyncPromise(resolve => { + resolve(value); + }); +} + +/** JSDoc */ +export function syncPromiseReject(reason?: any): PromiseLike { + return new SyncPromise((_, reject) => { + reject(reason); + }); +} + +/** JSDoc */ +export function syncPromiseAll(collection: Array>): PromiseLike { + return new SyncPromise((resolve, reject) => { + if (!Array.isArray(collection)) { + reject(new TypeError(`Promise.all requires an array as input.`)); + return; + } + + if (collection.length === 0) { + resolve([]); + return; + } + + let counter = collection.length; + const resolvedCollection: U[] = []; + + collection.forEach((item, index) => { + void syncPromiseResolve(item) + .then(value => { + resolvedCollection[index] = value; + counter -= 1; + + if (counter !== 0) { + return; + } + resolve(resolvedCollection); + }) + .then(null, reject); + }); + }); +} From 6970ee1b0275321d6d8ca2001e3596e7485c5ef7 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 22:54:33 -0500 Subject: [PATCH 2/6] convert SyncPromise.resolve --- packages/browser/src/eventbuilder.ts | 6 +++--- packages/browser/src/sdk.ts | 6 +++--- packages/browser/test/unit/mocks/simpletransport.ts | 4 ++-- packages/core/src/baseclient.ts | 3 ++- packages/core/src/transports/noop.ts | 8 ++++---- packages/core/test/mocks/backend.ts | 6 +++--- packages/node/src/integrations/linkederrors.ts | 6 +++--- packages/node/src/parsers.ts | 8 ++++---- 8 files changed, 24 insertions(+), 23 deletions(-) diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index afc7b51c50d1..c9307749a0d8 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -8,7 +8,7 @@ import { isErrorEvent, isEvent, isPlainObject, - SyncPromise, + syncPromiseResolve, } from '@sentry/utils'; import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers'; @@ -28,7 +28,7 @@ export function eventFromException(options: Options, exception: unknown, hint?: if (hint && hint.event_id) { event.event_id = hint.event_id; } - return SyncPromise.resolve(event); + return syncPromiseResolve(event); } /** @@ -49,7 +49,7 @@ export function eventFromMessage( if (hint && hint.event_id) { event.event_id = hint.event_id; } - return SyncPromise.resolve(event); + return syncPromiseResolve(event); } /** diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 518052ac71ad..334332335cb0 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -1,5 +1,5 @@ import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core'; -import { addInstrumentationHandler, getGlobalObject, logger, SyncPromise } from '@sentry/utils'; +import { addInstrumentationHandler, getGlobalObject, logger, syncPromiseResolve } from '@sentry/utils'; import { BrowserOptions } from './backend'; import { BrowserClient } from './client'; @@ -162,7 +162,7 @@ export function flush(timeout?: number): PromiseLike { return client.flush(timeout); } logger.warn('Cannot flush events. No client defined.'); - return SyncPromise.resolve(false); + return syncPromiseResolve(false); } /** @@ -179,7 +179,7 @@ export function close(timeout?: number): PromiseLike { return client.close(timeout); } logger.warn('Cannot flush events and disable SDK. No client defined.'); - return SyncPromise.resolve(false); + return syncPromiseResolve(false); } /** diff --git a/packages/browser/test/unit/mocks/simpletransport.ts b/packages/browser/test/unit/mocks/simpletransport.ts index 0066f9c8c6cc..95f5c7f26f36 100644 --- a/packages/browser/test/unit/mocks/simpletransport.ts +++ b/packages/browser/test/unit/mocks/simpletransport.ts @@ -1,4 +1,4 @@ -import { eventStatusFromHttpCode, SyncPromise } from '@sentry/utils'; +import { eventStatusFromHttpCode, syncPromiseResolve } from '@sentry/utils'; import { Event, Response } from '../../../src'; import { BaseTransport } from '../../../src/transports'; @@ -6,7 +6,7 @@ import { BaseTransport } from '../../../src/transports'; export class SimpleTransport extends BaseTransport { public sendEvent(_: Event): PromiseLike { return this._buffer.add(() => - SyncPromise.resolve({ + syncPromiseResolve({ status: eventStatusFromHttpCode(200), }), ); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index e9c4f1ca48bf..c951b348aadf 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -21,6 +21,7 @@ import { normalize, SentryError, SyncPromise, + syncPromiseResolve, truncate, uuid4, } from '@sentry/utils'; @@ -356,7 +357,7 @@ export abstract class BaseClient implement } // We prepare the result here with a resolved Event. - let result = SyncPromise.resolve(prepared); + let result = syncPromiseResolve(prepared); // This should be the last thing called, since we want that // {@link Hub.addEventProcessor} gets the finished prepared event. diff --git a/packages/core/src/transports/noop.ts b/packages/core/src/transports/noop.ts index bf1a1603b53a..08aac9b3a4af 100644 --- a/packages/core/src/transports/noop.ts +++ b/packages/core/src/transports/noop.ts @@ -1,5 +1,5 @@ import { Event, Response, Transport } from '@sentry/types'; -import { SyncPromise } from '@sentry/utils'; +import { syncPromiseResolve } from '@sentry/utils'; /** Noop transport */ export class NoopTransport implements Transport { @@ -7,8 +7,8 @@ export class NoopTransport implements Transport { * @inheritDoc */ public sendEvent(_: Event): PromiseLike { - return SyncPromise.resolve({ - reason: `NoopTransport: Event has been skipped because no Dsn is configured.`, + return syncPromiseResolve({ + reason: 'NoopTransport: Event has been skipped because no Dsn is configured.', status: 'skipped', }); } @@ -17,6 +17,6 @@ export class NoopTransport implements Transport { * @inheritDoc */ public close(_?: number): PromiseLike { - return SyncPromise.resolve(true); + return syncPromiseResolve(true); } } diff --git a/packages/core/test/mocks/backend.ts b/packages/core/test/mocks/backend.ts index f98e5adf4f69..9230fd8d7584 100644 --- a/packages/core/test/mocks/backend.ts +++ b/packages/core/test/mocks/backend.ts @@ -1,6 +1,6 @@ import { Session } from '@sentry/hub'; import { Event, Options, SeverityLevel, Transport } from '@sentry/types'; -import { SyncPromise } from '@sentry/utils'; +import { syncPromiseResolve } from '@sentry/utils'; import { BaseBackend } from '../../src/basebackend'; @@ -24,7 +24,7 @@ export class TestBackend extends BaseBackend { // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types public eventFromException(exception: any): PromiseLike { - return SyncPromise.resolve({ + return syncPromiseResolve({ exception: { values: [ { @@ -39,7 +39,7 @@ export class TestBackend extends BaseBackend { } public eventFromMessage(message: string, level: SeverityLevel = 'info'): PromiseLike { - return SyncPromise.resolve({ message, level }); + return syncPromiseResolve({ message, level }); } public sendEvent(event: Event): void { diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index 6428ee7f0a46..3e1baa664f1c 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/packages/node/src/integrations/linkederrors.ts @@ -1,6 +1,6 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import { Event, EventHint, Exception, ExtendedError, Integration } from '@sentry/types'; -import { isInstanceOf, SyncPromise } from '@sentry/utils'; +import { isInstanceOf, SyncPromise, syncPromiseResolve } from '@sentry/utils'; import { getExceptionFromError } from '../parsers'; @@ -56,7 +56,7 @@ export class LinkedErrors implements Integration { */ private _handler(event: Event, hint?: EventHint): PromiseLike { if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) { - return SyncPromise.resolve(event); + return syncPromiseResolve(event); } return new SyncPromise(resolve => { @@ -78,7 +78,7 @@ export class LinkedErrors implements Integration { */ private _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): PromiseLike { if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) { - return SyncPromise.resolve(stack); + return syncPromiseResolve(stack); } return new SyncPromise((resolve, reject) => { void getExceptionFromError(error[key]) diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index e9dbfb9ebb39..6e9c50c82fbc 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,5 +1,5 @@ import { Event, Exception, ExtendedError, StackFrame } from '@sentry/types'; -import { addContextToFrame, basename, dirname, SyncPromise } from '@sentry/utils'; +import { addContextToFrame, basename, dirname, SyncPromise, syncPromiseResolve } from '@sentry/utils'; import { readFile } from 'fs'; import { LRUMap } from 'lru_map'; @@ -71,7 +71,7 @@ function getModule(filename: string, base?: string): string { function readSourceFiles(filenames: string[]): PromiseLike<{ [key: string]: string | null }> { // we're relying on filenames being de-duped already if (filenames.length === 0) { - return SyncPromise.resolve({}); + return syncPromiseResolve({}); } return new SyncPromise<{ @@ -176,7 +176,7 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions // We do an early return if we do not want to fetch context liens if (linesOfContext <= 0) { - return SyncPromise.resolve(frames); + return syncPromiseResolve(frames); } try { @@ -184,7 +184,7 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions } catch (_) { // This happens in electron for example where we are not able to read files from asar. // So it's fine, we recover be just returning all frames without pre/post context. - return SyncPromise.resolve(frames); + return syncPromiseResolve(frames); } } From 29794af353716df49bcb507e8dd010ad23ba3a91 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 22:55:14 -0500 Subject: [PATCH 3/6] convert SyncPromise.reject --- packages/core/src/baseclient.ts | 5 +++-- packages/utils/src/promisebuffer.ts | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index c951b348aadf..66a295898f0d 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -21,6 +21,7 @@ import { normalize, SentryError, SyncPromise, + syncPromiseReject, syncPromiseResolve, truncate, uuid4, @@ -532,7 +533,7 @@ export abstract class BaseClient implement } if (!this._isEnabled()) { - return SyncPromise.reject(new SentryError('SDK not enabled, will not capture event.')); + return syncPromiseReject(new SentryError('SDK not enabled, will not capture event.')); } const isTransaction = event.type === 'transaction'; @@ -541,7 +542,7 @@ export abstract class BaseClient implement // Sampling for transaction happens somewhere else if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) { recordLostEvent('sample_rate', 'event'); - return SyncPromise.reject( + return syncPromiseReject( new SentryError( `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`, ), diff --git a/packages/utils/src/promisebuffer.ts b/packages/utils/src/promisebuffer.ts index 94c4a8f48d00..0d6bac82ef57 100644 --- a/packages/utils/src/promisebuffer.ts +++ b/packages/utils/src/promisebuffer.ts @@ -1,5 +1,5 @@ import { SentryError } from './error'; -import { SyncPromise } from './syncpromise'; +import { SyncPromise, syncPromiseReject } from './syncpromise'; function allPromises(collection: Array>): PromiseLike { return new SyncPromise((resolve, reject) => { @@ -48,7 +48,7 @@ export class PromiseBuffer { */ public add(taskProducer: () => PromiseLike): PromiseLike { if (!this.isReady()) { - return SyncPromise.reject(new SentryError('Not adding Promise due to buffer limit reached.')); + return syncPromiseReject(new SentryError('Not adding Promise due to buffer limit reached.')); } // start the task and add its promise to the queue From 257fe08f26d392b38b39d95d53f831f35e7b6e53 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 22:55:51 -0500 Subject: [PATCH 4/6] convert SyncPromise.all --- packages/utils/src/syncpromise.ts | 32 ------------------------------- 1 file changed, 32 deletions(-) diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts index 9a0fe73561f3..3fb5e1391833 100644 --- a/packages/utils/src/syncpromise.ts +++ b/packages/utils/src/syncpromise.ts @@ -187,35 +187,3 @@ export function syncPromiseReject(reason?: any): PromiseLike { reject(reason); }); } - -/** JSDoc */ -export function syncPromiseAll(collection: Array>): PromiseLike { - return new SyncPromise((resolve, reject) => { - if (!Array.isArray(collection)) { - reject(new TypeError(`Promise.all requires an array as input.`)); - return; - } - - if (collection.length === 0) { - resolve([]); - return; - } - - let counter = collection.length; - const resolvedCollection: U[] = []; - - collection.forEach((item, index) => { - void syncPromiseResolve(item) - .then(value => { - resolvedCollection[index] = value; - counter -= 1; - - if (counter !== 0) { - return; - } - resolve(resolvedCollection); - }) - .then(null, reject); - }); - }); -} From d5895db568630a5cfdea696851e05f7bd5dda89d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 22:58:18 -0500 Subject: [PATCH 5/6] remove static methods --- packages/utils/src/syncpromise.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts index 3fb5e1391833..4c85c06da9c7 100644 --- a/packages/utils/src/syncpromise.ts +++ b/packages/utils/src/syncpromise.ts @@ -33,16 +33,6 @@ export class SyncPromise implements PromiseLike { } } - /** JSDoc */ - public static resolve(value: T | PromiseLike): PromiseLike { - return syncPromiseResolve(value); - } - - /** JSDoc */ - public static reject(reason?: any): PromiseLike { - return syncPromiseReject(reason); - } - /** JSDoc */ public then( onfulfilled?: ((value: T) => TResult1 | PromiseLike) | null, From 29774ca38d866240b319587b37cd000c103eaf2e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 22:58:24 -0500 Subject: [PATCH 6/6] update tests --- packages/utils/test/is.test.ts | 4 ++-- packages/utils/test/syncpromise.test.ts | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/utils/test/is.test.ts b/packages/utils/test/is.test.ts index cd936edacee0..3cf2bdc9b0ba 100644 --- a/packages/utils/test/is.test.ts +++ b/packages/utils/test/is.test.ts @@ -1,6 +1,6 @@ import { isDOMError, isDOMException, isError, isErrorEvent, isInstanceOf, isPrimitive, isThenable } from '../src/is'; import { supportsDOMError, supportsDOMException, supportsErrorEvent } from '../src/supports'; -import { SyncPromise } from '../src/syncpromise'; +import { SyncPromise, syncPromiseResolve } from '../src/syncpromise'; class SentryError extends Error { public name: string; @@ -75,7 +75,7 @@ describe('isPrimitive()', () => { describe('isThenable()', () => { test('should work as advertised', () => { expect(isThenable(Promise.resolve(true))).toEqual(true); - expect(isThenable(SyncPromise.resolve(true))).toEqual(true); + expect(isThenable(syncPromiseResolve(true))).toEqual(true); expect(isThenable(undefined)).toEqual(false); expect(isThenable(null)).toEqual(false); diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts index 54256acfa4ad..390e588ad8c5 100644 --- a/packages/utils/test/syncpromise.test.ts +++ b/packages/utils/test/syncpromise.test.ts @@ -1,4 +1,4 @@ -import { SyncPromise } from '../src/syncpromise'; +import { SyncPromise, syncPromiseResolve, syncPromiseReject } from '../src/syncpromise'; describe('SyncPromise', () => { test('simple', () => { @@ -17,9 +17,9 @@ describe('SyncPromise', () => { return new SyncPromise(resolve => { resolve(42); }) - .then(_ => SyncPromise.resolve('a')) - .then(_ => SyncPromise.resolve(0.1)) - .then(_ => SyncPromise.resolve(false)) + .then(_ => syncPromiseResolve('a')) + .then(_ => syncPromiseResolve(0.1)) + .then(_ => syncPromiseResolve(false)) .then(val => { expect(val).toBe(false); }); @@ -84,7 +84,7 @@ describe('SyncPromise', () => { return ( c // @ts-ignore Argument of type 'PromiseLike' is not assignable to parameter of type 'SyncPromise' - .then(val => f(SyncPromise.resolve('x'), val)) + .then(val => f(syncPromiseResolve('x'), val)) .then(val => f(b, val)) // @ts-ignore Argument of type 'SyncPromise' is not assignable to parameter of type 'string' .then(val => f(a, val)) @@ -97,7 +97,7 @@ describe('SyncPromise', () => { test('simple static', () => { expect.assertions(1); - const p = SyncPromise.resolve(10); + const p = syncPromiseResolve(10); return p.then(val => { expect(val).toBe(10); }); @@ -260,7 +260,7 @@ describe('SyncPromise', () => { }) .then(value => { expect(value).toEqual(2); - return SyncPromise.reject('wat'); + return syncPromiseReject('wat'); }) .then(null, reason => { expect(reason).toBe('wat');