Core & multiple modules: refactor usage of Promise to avoid deferrals when possible (6.29.x)

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';
 
 const MODULE_NAME = 'User ID';
 const COOKIE = 'cookie';
@@ -523,24 +523,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;
@@ -559,7 +555,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);
@@ -568,7 +564,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));
         }
       }))
   );
@@ -592,7 +588,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
         });
       }
     }
-    if (refresh) {
+    if (refresh && initialized) {
       done = cancelAndTry(
         done
           .catch(() => null)
@@ -607,7 +603,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));
             }
           }))
       );
@@ -640,8 +636,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(() => {
@@ -786,7 +782,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)
+  );
 }
 
 /**
@@ -933,6 +938,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);
@@ -968,6 +975,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));
   }
 }
 
@@ -976,7 +994,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 = [];
@@ -1021,10 +1039,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;
 
@@ -375,9 +376,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) {
@@ -389,9 +390,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/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);