feat(utils): Kill SyncPromise.reject and SyncPromise.resolve (#4329)

Author: mitsuhiko

packages/browser/src/eventbuilder.ts

@@ -8,7 +8,7 @@ import {
   isErrorEvent,
   isEvent,
   isPlainObject,
-  SyncPromise,
+  resolvedSyncPromise,
 } 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 resolvedSyncPromise(event);
 }
 
 /**
@@ -49,7 +49,7 @@ export function eventFromMessage(
   if (hint && hint.event_id) {
     event.event_id = hint.event_id;
   }
-  return SyncPromise.resolve(event);
+  return resolvedSyncPromise(event);
 }
 
 /**

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, resolvedSyncPromise } from '@sentry/utils';
 
 import { BrowserOptions } from './backend';
 import { BrowserClient } from './client';
@@ -162,7 +162,7 @@ export function flush(timeout?: number): PromiseLike<boolean> {
     return client.flush(timeout);
   }
   logger.warn('Cannot flush events. No client defined.');
-  return SyncPromise.resolve(false);
+  return resolvedSyncPromise(false);
 }
 
 /**
@@ -179,7 +179,7 @@ export function close(timeout?: number): PromiseLike<boolean> {
     return client.close(timeout);
   }
   logger.warn('Cannot flush events and disable SDK. No client defined.');
-  return SyncPromise.resolve(false);
+  return resolvedSyncPromise(false);
 }
 
 /**

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

@@ -1,12 +1,12 @@
-import { eventStatusFromHttpCode, SyncPromise } from '@sentry/utils';
+import { eventStatusFromHttpCode, resolvedSyncPromise } from '@sentry/utils';
 
 import { Event, Response } from '../../../src';
 import { BaseTransport } from '../../../src/transports';
 
 export class SimpleTransport extends BaseTransport {
   public sendEvent(_: Event): PromiseLike<Response> {
     return this._buffer.add(() =>
-      SyncPromise.resolve({
+      resolvedSyncPromise({
         status: eventStatusFromHttpCode(200),
       }),
     );

packages/core/src/baseclient.ts

@@ -19,6 +19,8 @@ import {
   isThenable,
   logger,
   normalize,
+  rejectedSyncPromise,
+  resolvedSyncPromise,
   SentryError,
   SyncPromise,
   truncate,
@@ -356,7 +358,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     }
 
     // We prepare the result here with a resolved Event.
-    let result = SyncPromise.resolve<Event | null>(prepared);
+    let result = resolvedSyncPromise<Event | null>(prepared);
 
     // This should be the last thing called, since we want that
     // {@link Hub.addEventProcessor} gets the finished prepared event.
@@ -531,7 +533,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     }
 
     if (!this._isEnabled()) {
-      return SyncPromise.reject(new SentryError('SDK not enabled, will not capture event.'));
+      return rejectedSyncPromise(new SentryError('SDK not enabled, will not capture event.'));
     }
 
     const isTransaction = event.type === 'transaction';
@@ -540,7 +542,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     // Sampling for transaction happens somewhere else
     if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) {
       recordLostEvent('sample_rate', 'event');
-      return SyncPromise.reject(
+      return rejectedSyncPromise(
         new SentryError(
           `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
         ),

packages/core/src/transports/noop.ts

@@ -1,13 +1,13 @@
 import { Event, Response, Transport } from '@sentry/types';
-import { SyncPromise } from '@sentry/utils';
+import { resolvedSyncPromise } from '@sentry/utils';
 
 /** Noop transport */
 export class NoopTransport implements Transport {
   /**
    * @inheritDoc
    */
   public sendEvent(_: Event): PromiseLike<Response> {
-    return SyncPromise.resolve({
+    return resolvedSyncPromise({
       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<boolean> {
-    return SyncPromise.resolve(true);
+    return resolvedSyncPromise(true);
   }
 }

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 { resolvedSyncPromise } from '@sentry/utils';
 
 import { BaseBackend } from '../../src/basebackend';
 
@@ -24,7 +24,7 @@ export class TestBackend extends BaseBackend<TestOptions> {
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
   public eventFromException(exception: any): PromiseLike<Event> {
-    return SyncPromise.resolve({
+    return resolvedSyncPromise({
       exception: {
         values: [
           {
@@ -39,7 +39,7 @@ export class TestBackend extends BaseBackend<TestOptions> {
   }
 
   public eventFromMessage(message: string, level: SeverityLevel = 'info'): PromiseLike<Event> {
-    return SyncPromise.resolve({ message, level });
+    return resolvedSyncPromise({ message, level });
   }
 
   public sendEvent(event: Event): void {

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, resolvedSyncPromise, SyncPromise } from '@sentry/utils';
 
 import { getExceptionFromError } from '../parsers';
 
@@ -56,7 +56,7 @@ export class LinkedErrors implements Integration {
    */
   private _handler(event: Event, hint?: EventHint): PromiseLike<Event> {
     if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) {
-      return SyncPromise.resolve(event);
+      return resolvedSyncPromise(event);
     }
 
     return new SyncPromise<Event>(resolve => {
@@ -78,7 +78,7 @@ export class LinkedErrors implements Integration {
    */
   private _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): PromiseLike<Exception[]> {
     if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) {
-      return SyncPromise.resolve(stack);
+      return resolvedSyncPromise(stack);
     }
     return new SyncPromise<Exception[]>((resolve, reject) => {
       void getExceptionFromError(error[key])

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, resolvedSyncPromise, SyncPromise } 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 resolvedSyncPromise({});
   }
 
   return new SyncPromise<{
@@ -176,15 +176,15 @@ 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 resolvedSyncPromise(frames);
   }
 
   try {
     return addPrePostContext(filesToRead, frames, linesOfContext);
   } 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 resolvedSyncPromise(frames);
   }
 }
 

packages/utils/src/promisebuffer.ts

@@ -1,26 +1,5 @@
 import { SentryError } from './error';
-import { SyncPromise } from './syncpromise';
-
-function allPromises<U = unknown>(collection: Array<U | PromiseLike<U>>): PromiseLike<U[]> {
-  return new SyncPromise<U[]>((resolve, reject) => {
-    if (collection.length === 0) {
-      resolve(null);
-      return;
-    }
-
-    let counter = collection.length;
-    collection.forEach(item => {
-      void SyncPromise.resolve(item)
-        .then(() => {
-          // eslint-disable-next-line no-plusplus
-          if (--counter === 0) {
-            resolve(null);
-          }
-        })
-        .then(null, reject);
-    });
-  });
-}
+import { rejectedSyncPromise, resolvedSyncPromise, SyncPromise } from './syncpromise';
 
 export interface PromiseBuffer<T> {
   // 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> {
    */
   function add(taskProducer: () => PromiseLike<T>): PromiseLike<T> {
     if (!isReady()) {
-      return SyncPromise.reject(new SentryError('Not adding Promise due to buffer limit reached.'));
+      return rejectedSyncPromise(new SentryError('Not adding Promise due to buffer limit reached.'));
     }
 
     // start the task and add its promise to the queue
@@ -94,7 +73,13 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
    * `false` otherwise
    */
   function drain(timeout?: number): PromiseLike<boolean> {
-    return new SyncPromise<boolean>(resolve => {
+    return new SyncPromise<boolean>((resolve, reject) => {
+      let counter = buffer.length;
+
+      if (!counter) {
+        return resolve(true);
+      }
+
       // wait for `timeout` ms and then resolve to `false` (if not cancelled first)
       const capturedSetTimeout = setTimeout(() => {
         if (timeout && timeout > 0) {
@@ -103,9 +88,14 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
       }, timeout);
 
       // if all promises resolve in time, cancel the timer and resolve to `true`
-      void allPromises(buffer).then(() => {
-        clearTimeout(capturedSetTimeout);
-        resolve(true);
+      buffer.forEach(item => {
+        void resolvedSyncPromise(item).then(() => {
+          // eslint-disable-next-line no-plusplus
+          if (!--counter) {
+            clearTimeout(capturedSetTimeout);
+            resolve(true);
+          }
+        }, reject);
       });
     });
   }

packages/utils/src/syncpromise.ts

@@ -14,6 +14,30 @@ const enum States {
   REJECTED = 2,
 }
 
+/**
+ * Creates a resolved sync promise.
+ *
+ * @param value the value to resolve the promise with
+ * @returns the resolved sync promise
+ */
+export function resolvedSyncPromise<T>(value: T | PromiseLike<T>): PromiseLike<T> {
+  return new SyncPromise(resolve => {
+    resolve(value);
+  });
+}
+
+/**
+ * Creates a rejected sync promise.
+ *
+ * @param value the value to reject the promise with
+ * @returns the rejected sync promise
+ */
+export function rejectedSyncPromise<T = never>(reason?: any): PromiseLike<T> {
+  return new SyncPromise((_, reject) => {
+    reject(reason);
+  });
+}
+
 /**
  * Thenable class that behaves like a Promise and follows it's interface
  * but is not async internally
@@ -33,20 +57,6 @@ class SyncPromise<T> implements PromiseLike<T> {
     }
   }
 
-  /** JSDoc */
-  public static resolve<T>(value: T | PromiseLike<T>): PromiseLike<T> {
-    return new SyncPromise(resolve => {
-      resolve(value);
-    });
-  }
-
-  /** JSDoc */
-  public static reject<T = never>(reason?: any): PromiseLike<T> {
-    return new SyncPromise((_, reject) => {
-      reject(reason);
-    });
-  }
-
   /** JSDoc */
   public then<TResult1 = T, TResult2 = never>(
     onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | null,