Core & multiple modules: refactor usage of Promise to avoid deferrals…

… when possible (#8626)

* Core & multiple modules: refactor usage of Promise to avoid deferrals when possible

* Unhandled rejection handling

* Improve tests

modules/currency.js

@@ -5,7 +5,7 @@ import CONSTANTS from '../src/constants.json';
 import { ajax } from '../src/ajax.js';
 import { config } from '../src/config.js';
 import { getHook } from '../src/hook.js';
-import {promiseControls} from '../src/utils/promise.js';
+import {defer} from '../src/utils/promise.js';
 
 const DEFAULT_CURRENCY_RATE_URL = 'https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json?date=$$TODAY$$';
 const CURRENCY_RATE_PRECISION = 4;
@@ -24,7 +24,7 @@ var defaultRates;
 export const ready = (() => {
   let ctl;
   function reset() {
-    ctl = promiseControls();
+    ctl = defer();
   }
   reset();
   return {done: () => ctl.resolve(), reset, promise: () => ctl.promise}

modules/userId/index.js

@@ -152,7 +152,7 @@ import {
   isEmpty
 } from '../../src/utils.js';
 import {getPPID as coreGetPPID} from '../../src/adserver.js';
-import {promiseControls} from '../../src/utils/promise.js';
+import {defer, GreedyPromise} from '../../src/utils/promise.js';
 import {hasPurpose1Consent} from '../../src/utils/gpdr.js';
 
 const MODULE_NAME = 'User ID';
@@ -504,24 +504,20 @@ function addIdDataToAdUnitBids(adUnits, submodules) {
   });
 }
 
-function delayFor(ms) {
-  return new Promise((resolve) => setTimeout(resolve, ms));
-}
-
 const INIT_CANCELED = {};
 
-function idSystemInitializer({delay = delayFor} = {}) {
-  const startInit = promiseControls();
-  const startCallbacks = promiseControls();
+function idSystemInitializer({delay = GreedyPromise.timeout} = {}) {
+  const startInit = defer();
+  const startCallbacks = defer();
   let cancel;
   let initialized = false;
 
   function cancelAndTry(promise) {
     if (cancel != null) {
       cancel.reject(INIT_CANCELED);
     }
-    cancel = promiseControls();
-    return Promise.race([promise, cancel.promise]);
+    cancel = defer();
+    return GreedyPromise.race([promise, cancel.promise]);
   }
 
   // grab a reference to global vars so that the promise chains remain isolated;
@@ -540,7 +536,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
   }
 
   let done = cancelAndTry(
-    Promise.all([hooksReady, startInit.promise])
+    GreedyPromise.all([hooksReady, startInit.promise])
       .then(() => gdprDataHandler.promise)
       .then(checkRefs((consentData) => {
         initSubmodules(initModules, allModules, consentData);
@@ -549,7 +545,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
       .then(checkRefs(() => {
         const modWithCb = initModules.filter(item => isFn(item.callback));
         if (modWithCb.length) {
-          return new Promise((resolve) => processSubmoduleCallbacks(modWithCb, resolve));
+          return new GreedyPromise((resolve) => processSubmoduleCallbacks(modWithCb, resolve));
         }
       }))
   );
@@ -573,7 +569,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
         });
       }
     }
-    if (refresh) {
+    if (refresh && initialized) {
       done = cancelAndTry(
         done
           .catch(() => null)
@@ -588,7 +584,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
               return sm.callback != null;
             });
             if (cbModules.length) {
-              return new Promise((resolve) => processSubmoduleCallbacks(cbModules, resolve));
+              return new GreedyPromise((resolve) => processSubmoduleCallbacks(cbModules, resolve));
             }
           }))
       );
@@ -621,8 +617,8 @@ function getPPID() {
  * @param {Object} reqBidsConfigObj required; This is the same param that's used in pbjs.requestBids.
  * @param {function} fn required; The next function in the chain, used by hook.js
  */
-export function requestBidsHook(fn, reqBidsConfigObj, {delay = delayFor} = {}) {
-  Promise.race([
+export function requestBidsHook(fn, reqBidsConfigObj, {delay = GreedyPromise.timeout} = {}) {
+  GreedyPromise.race([
     getUserIdsAsync(),
     delay(auctionDelay)
   ]).then(() => {
@@ -767,7 +763,16 @@ function refreshUserIds({submoduleNames} = {}, callback) {
  */
 
 function getUserIdsAsync() {
-  return initIdSystem().then(() => getUserIds(), (e) => e === INIT_CANCELED ? getUserIdsAsync() : Promise.reject(e));
+  return initIdSystem().then(
+    () => getUserIds(),
+    (e) =>
+      e === INIT_CANCELED
+        // there's a pending refresh - because GreedyPromise runs this synchronously, we are now in the middle
+        // of canceling the previous init, before the refresh logic has had a chance to run.
+        // Use a "normal" Promise to clear the stack and let it complete (or this will just recurse infinitely)
+        ? Promise.resolve().then(getUserIdsAsync)
+        : GreedyPromise.reject(e)
+  );
 }
 
 /**
@@ -914,6 +919,8 @@ function updateSubmodules() {
     return;
   }
   // do this to avoid reprocessing submodules
+  // TODO: the logic does not match the comment - addedSubmodules is always a copy of submoduleRegistry
+  // (if it did it would not be correct - it's not enough to find new modules, as others may have been removed or changed)
   const addedSubmodules = submoduleRegistry.filter(i => !find(submodules, j => j.name === i.name));
 
   submodules.splice(0, submodules.length);
@@ -949,6 +956,17 @@ export function attachIdSystem(submodule) {
   if (!find(submoduleRegistry, i => i.name === submodule.name)) {
     submoduleRegistry.push(submodule);
     updateSubmodules();
+    // TODO: a test case wants this to work even if called after init (the setConfig({userId}))
+    // so we trigger a refresh. But is that even possible outside of tests?
+    initIdSystem({refresh: true, submoduleNames: [submodule.name]});
+  }
+}
+
+function normalizePromise(fn) {
+  // for public methods that return promises, make sure we return a "normal" one - to avoid
+  // exposing confusing stack traces
+  return function() {
+    return Promise.resolve(fn.apply(this, arguments));
   }
 }
 
@@ -957,7 +975,7 @@ export function attachIdSystem(submodule) {
  * so a callback is added to fire after the consentManagement module.
  * @param {{getConfig:function}} config
  */
-export function init(config, {delay = delayFor} = {}) {
+export function init(config, {delay = GreedyPromise.timeout} = {}) {
   ppidSource = undefined;
   submodules = [];
   configRegistry = [];
@@ -1002,10 +1020,10 @@ export function init(config, {delay = delayFor} = {}) {
   // exposing getUserIds function in global-name-space so that userIds stored in Prebid can be used by external codes.
   (getGlobal()).getUserIds = getUserIds;
   (getGlobal()).getUserIdsAsEids = getUserIdsAsEids;
-  (getGlobal()).getEncryptedEidsForSource = getEncryptedEidsForSource;
+  (getGlobal()).getEncryptedEidsForSource = normalizePromise(getEncryptedEidsForSource);
   (getGlobal()).registerSignalSources = registerSignalSources;
-  (getGlobal()).refreshUserIds = refreshUserIds;
-  (getGlobal()).getUserIdsAsync = getUserIdsAsync;
+  (getGlobal()).refreshUserIds = normalizePromise(refreshUserIds);
+  (getGlobal()).getUserIdsAsync = normalizePromise(getUserIdsAsync);
   (getGlobal()).getUserIdsAsEidBySource = getUserIdsAsEidBySource;
 }
 

src/auction.js

@@ -76,6 +76,7 @@ import {bidderSettings} from './bidderSettings.js';
 import * as events from './events.js'
 import adapterManager from './adapterManager.js';
 import CONSTANTS from './constants.json';
+import {GreedyPromise} from './utils/promise.js';
 
 const { syncUsers } = userSync;
 
@@ -384,9 +385,9 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM
 
   function waitFor(requestId, result) {
     if (ready[requestId] == null) {
-      ready[requestId] = Promise.resolve();
+      ready[requestId] = GreedyPromise.resolve();
     }
-    ready[requestId] = ready[requestId].then(() => Promise.resolve(result).catch(() => {}))
+    ready[requestId] = ready[requestId].then(() => GreedyPromise.resolve(result).catch(() => {}))
   }
 
   function guard(bidderRequest, fn) {
@@ -398,9 +399,9 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM
     const wait = ready[bidderRequest.bidderRequestId];
     const orphanWait = ready['']; // also wait for "orphan" responses that are not associated with any request
     if ((wait != null || orphanWait != null) && timeRemaining > 0) {
-      Promise.race([
-        new Promise((resolve) => setTimeout(resolve, timeRemaining)),
-        Promise.resolve(orphanWait).then(() => wait)
+      GreedyPromise.race([
+        GreedyPromise.timeout(timeRemaining),
+        GreedyPromise.resolve(orphanWait).then(() => wait)
       ]).then(fn);
     } else {
       fn();

src/consentHandler.js

@@ -1,28 +1,28 @@
 import {isStr, timestamp} from './utils.js';
+import {defer, GreedyPromise} from './utils/promise.js';
 
 export class ConsentHandler {
   #enabled;
   #data;
-  #promise;
-  #resolve;
+  #defer;
   #ready;
   generatedTime;
 
   constructor() {
     this.reset();
   }
 
+  #resolve(data) {
+    this.#ready = true;
+    this.#data = data;
+    this.#defer.resolve(data);
+  }
+
   /**
    * reset this handler (mainly for tests)
    */
   reset() {
-    this.#promise = new Promise((resolve) => {
-      this.#resolve = (data) => {
-        this.#ready = true;
-        this.#data = data;
-        resolve(data);
-      };
-    });
+    this.#defer = defer();
     this.#enabled = false;
     this.#data = null;
     this.#ready = false;
@@ -56,12 +56,12 @@ export class ConsentHandler {
    */
   get promise() {
     if (this.#ready) {
-      return Promise.resolve(this.#data);
+      return GreedyPromise.resolve(this.#data);
     }
     if (!this.#enabled) {
       this.#resolve(null);
     }
-    return this.#promise;
+    return this.#defer.promise;
   }
 
   setConsentData(data, time = timestamp()) {

src/debugging.js

@@ -4,6 +4,7 @@ import {getGlobal} from './prebidGlobal.js';
 import {logMessage, prefixLog} from './utils.js';
 import {createBid} from './bidfactory.js';
 import {loadExternalScript} from './adloader.js';
+import {GreedyPromise} from './utils/promise.js';
 
 export const DEBUG_KEY = '__$$PREBID_GLOBAL$$_debugging__';
 
@@ -12,7 +13,7 @@ function isDebuggingInstalled() {
 }
 
 function loadScript(url) {
-  return new Promise((resolve) => {
+  return new GreedyPromise((resolve) => {
     loadExternalScript(url, 'debugging', resolve);
   });
 }
@@ -21,7 +22,8 @@ export function debuggingModuleLoader({alreadyInstalled = isDebuggingInstalled,
   let loading = null;
   return function () {
     if (loading == null) {
-      loading = new Promise((resolve, reject) => {
+      loading = new GreedyPromise((resolve, reject) => {
+        // run this in a 0-delay timeout to give installedModules time to be populated
         setTimeout(() => {
           if (alreadyInstalled()) {
             resolve();
@@ -44,7 +46,7 @@ export function debuggingControls({load = debuggingModuleLoader(), hook = getHoo
   let promise = null;
   let enabled = false;
   function waitForDebugging(next, ...args) {
-    return (promise || Promise.resolve()).then(() => next.apply(this, args))
+    return (promise || GreedyPromise.resolve()).then(() => next.apply(this, args))
   }
   function enable() {
     if (!enabled) {

src/hook.js

@@ -1,11 +1,11 @@
 import funHooks from 'fun-hooks/no-eval/index.js';
-import {promiseControls} from './utils/promise.js';
+import {defer} from './utils/promise.js';
 
 export let hook = funHooks({
   ready: funHooks.SYNC | funHooks.ASYNC | funHooks.QUEUE
 });
 
-const readyCtl = promiseControls();
+const readyCtl = defer();
 hook.ready = (() => {
   const ready = hook.ready;
   return function () {

src/utils.js

@@ -2,6 +2,7 @@ import { config } from './config.js';
 import clone from 'just-clone';
 import {find, includes} from './polyfill.js';
 import CONSTANTS from './constants.json';
+import {GreedyPromise} from './utils/promise.js';
 export { default as deepAccess } from 'dlv/index.js';
 export { default as deepSetValue } from 'dset';
 
@@ -487,7 +488,7 @@ export function hasOwn(objectToCheck, propertyToCheckFor) {
 * @param {HTMLElement} [doc]
 * @param {HTMLElement} [target]
 * @param {Boolean} [asLastChildChild]
-* @return {HTMLElement}
+* @return {HTML Element}
 */
 export function insertElement(elm, doc, target, asLastChildChild) {
   doc = doc || document;
@@ -517,7 +518,7 @@ export function insertElement(elm, doc, target, asLastChildChild) {
  */
 export function waitForElementToLoad(element, timeout) {
   let timer = null;
-  return new Promise((resolve) => {
+  return new GreedyPromise((resolve) => {
     const onLoad = function() {
       element.removeEventListener('load', onLoad);
       element.removeEventListener('error', onLoad);