fix: Make sure that SyncPromise handler is called only once (getsentry#2511)

kamilogorek · web-flow · commit 1ed8a18d34f2 · 2020-03-25T14:16:40.000+01:00
* fix: Make sure that SyncPromise handler is called only once

* happy linter is happy

* linters plz

* Use SyncPromise inside Buffer tests
diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts
@@ -106,7 +106,8 @@ export function supportsNativeFetch(): boolean {
   // so create a "pure" iframe to see if that has native fetch
   let result = false;
   const doc = global.document;
-  if (doc && typeof doc.createElement === 'function') {
+  // tslint:disable-next-line:no-unbound-method deprecation
+  if (doc && typeof (doc.createElement as unknown) === `function`) {
     try {
       const sandbox = doc.createElement('iframe');
       sandbox.hidden = true;
diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts
@@ -17,6 +17,7 @@ enum States {
 class SyncPromise<T> implements PromiseLike<T> {
   private _state: States = States.PENDING;
   private _handlers: Array<{
+    done: boolean;
     onfulfilled?: ((value: T) => T | PromiseLike<T>) | null;
     onrejected?: ((reason: any) => any) | null;
   }> = [];
@@ -90,6 +91,7 @@ class SyncPromise<T> implements PromiseLike<T> {
   ): PromiseLike<TResult1 | TResult2> {
     return new SyncPromise((resolve, reject) => {
       this._attachHandler({
+        done: false,
         onfulfilled: result => {
           if (!onfulfilled) {
             // TODO: ¯\_(ツ)_/¯
@@ -156,8 +158,7 @@ class SyncPromise<T> implements PromiseLike<T> {
           return;
         }
 
-        // tslint:disable-next-line:no-unsafe-any
-        resolve(val);
+        resolve((val as unknown) as any);
       });
     });
   }
@@ -192,6 +193,8 @@ class SyncPromise<T> implements PromiseLike<T> {
   // TODO: FIXME
   /** JSDoc */
   private readonly _attachHandler = (handler: {
+    /** JSDoc */
+    done: boolean;
     /** JSDoc */
     onfulfilled?(value: T): any;
     /** JSDoc */
@@ -207,22 +210,28 @@ class SyncPromise<T> implements PromiseLike<T> {
       return;
     }
 
-    if (this._state === States.REJECTED) {
-      this._handlers.forEach(handler => {
+    const cachedHandlers = this._handlers.slice();
+    this._handlers = [];
+
+    cachedHandlers.forEach(handler => {
+      if (handler.done) {
+        return;
+      }
+
+      if (this._state === States.RESOLVED) {
+        if (handler.onfulfilled) {
+          handler.onfulfilled((this._value as unknown) as any);
+        }
+      }
+
+      if (this._state === States.REJECTED) {
         if (handler.onrejected) {
           handler.onrejected(this._value);
         }
-      });
-    } else {
-      this._handlers.forEach(handler => {
-        if (handler.onfulfilled) {
-          // tslint:disable-next-line:no-unsafe-any
-          handler.onfulfilled(this._value);
-        }
-      });
-    }
+      }
 
-    this._handlers = [];
+      handler.done = true;
+    });
   };
 }
 
diff --git a/packages/utils/test/promisebuffer.test.ts b/packages/utils/test/promisebuffer.test.ts
@@ -1,4 +1,5 @@
 import { PromiseBuffer } from '../src/promisebuffer';
+import { SyncPromise } from '../src/syncpromise';
 
 // tslint:disable:no-floating-promises
 
@@ -10,23 +11,23 @@ describe('PromiseBuffer', () => {
   describe('add()', () => {
     test('no limit', () => {
       const q = new PromiseBuffer<void>();
-      const p = new Promise<void>(resolve => setTimeout(resolve, 1));
+      const p = new SyncPromise<void>(resolve => setTimeout(resolve, 1));
       q.add(p);
       expect(q.length()).toBe(1);
     });
     test('with limit', () => {
       const q = new PromiseBuffer<void>(1);
-      const p = new Promise<void>(resolve => setTimeout(resolve, 1));
+      const p = new SyncPromise<void>(resolve => setTimeout(resolve, 1));
       expect(q.add(p)).toEqual(p);
-      expect(q.add(new Promise<void>(resolve => setTimeout(resolve, 1)))).rejects.toThrowError();
+      expect(q.add(new SyncPromise<void>(resolve => setTimeout(resolve, 1)))).rejects.toThrowError();
       expect(q.length()).toBe(1);
     });
   });
 
   test('resolved promises should not show up in buffer length', async () => {
     expect.assertions(2);
     const q = new PromiseBuffer<void>();
-    const p = new Promise<void>(resolve => setTimeout(resolve, 1));
+    const p = new SyncPromise<void>(resolve => setTimeout(resolve, 1));
     q.add(p).then(() => {
       expect(q.length()).toBe(0);
     });
@@ -37,7 +38,7 @@ describe('PromiseBuffer', () => {
   test('receive promise result outside and from buffer', async () => {
     expect.assertions(4);
     const q = new PromiseBuffer<string>();
-    const p = new Promise<string>(resolve =>
+    const p = new SyncPromise<string>(resolve =>
       setTimeout(() => {
         resolve('test');
       }, 1),
@@ -57,7 +58,7 @@ describe('PromiseBuffer', () => {
     expect.assertions(3);
     const q = new PromiseBuffer<void>();
     for (let i = 0; i < 5; i++) {
-      const p = new Promise<void>(resolve => setTimeout(resolve, 1));
+      const p = new SyncPromise<void>(resolve => setTimeout(resolve, 1));
       q.add(p);
     }
     expect(q.length()).toBe(5);
@@ -72,7 +73,7 @@ describe('PromiseBuffer', () => {
     expect.assertions(2);
     const q = new PromiseBuffer<void>();
     for (let i = 0; i < 5; i++) {
-      const p = new Promise<void>(resolve => setTimeout(resolve, 100));
+      const p = new SyncPromise<void>(resolve => setTimeout(resolve, 100));
       q.add(p);
     }
     expect(q.length()).toBe(5);
@@ -96,7 +97,7 @@ describe('PromiseBuffer', () => {
   test('rejecting', async () => {
     expect.assertions(1);
     const q = new PromiseBuffer<void>();
-    const p = new Promise<void>((_, reject) => setTimeout(reject, 1));
+    const p = new SyncPromise<void>((_, reject) => setTimeout(reject, 1));
     jest.runAllTimers();
     return q.add(p).then(null, () => {
       expect(true).toBe(true);
