Prebid core & currency module: fix race condition on slow fetch of cu…

…rrency conversion file

The currency module hooks into the auction's `addBidResponse` flow to modify bid responses with currency info. To do this it needs to fetch conversion rates from the network, but the auction expects `addBidResponse` to complete synchronously, and things break down if all bidder network calls complete before the currency file fetch.

This fix allows the `addBidResponse` hook to return a promise. If it does, the auction will wait for it before ending.

Addresses #7765

modules/currency.js

@@ -20,6 +20,25 @@ export var currencyRates = {};
 var bidderCurrencyDefault = {};
 var defaultRates;
 
+export const ready = (() => {
+  let isDone, resolver, promise;
+  function reset() {
+    isDone = false;
+    resolver = null;
+    promise = new Promise((resolve) => {
+      resolver = resolve;
+      if (isDone) resolve();
+    })
+  }
+  function done() {
+    isDone = true;
+    if (resolver != null) { resolver() }
+  }
+  reset();
+
+  return {done, reset, promise: () => promise}
+})();
+
 /**
  * Configuration function for currency
  * @param  {string} [config.adServerCurrency = 'USD']
@@ -138,11 +157,15 @@ function initCurrency(url) {
             logInfo('currencyRates set to ' + JSON.stringify(currencyRates));
             currencyRatesLoaded = true;
             processBidResponseQueue();
+            ready.done();
           } catch (e) {
             errorSettingsRates('Failed to parse currencyRates response: ' + response);
           }
         },
-        error: errorSettingsRates
+        error: function (...args) {
+          errorSettingsRates(...args);
+          ready.done();
+        }
       }
     );
   }
@@ -197,6 +220,8 @@ export function addBidResponseHook(fn, adUnitCode, bid) {
   bidResponseQueue.push(wrapFunction(fn, this, [adUnitCode, bid]));
   if (!currencySupportEnabled || currencyRatesLoaded) {
     processBidResponseQueue();
+  } else {
+    fn.bail(ready.promise());
   }
 }
 

src/auction.js

@@ -250,10 +250,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
 
         let callbacks = auctionCallbacks(auctionDone, this);
         adapterManager.callBids(_adUnits, bidRequests, function(...args) {
-          addBidResponse.apply({
-            dispatch: callbacks.addBidResponse,
-            bidderRequest: this
-          }, args)
+          callbacks.addBidResponse.apply(this, args);
         }, callbacks.adapterDone, {
           request(source, origin) {
             increment(outstandingRequests, origin);
@@ -344,6 +341,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
     addWinningBid,
     setBidTargeting,
     getWinningBids: () => _winningBids,
+    getAuctionStart: () => _auctionStart,
     getTimeout: () => _timeout,
     getAuctionId: () => _auctionId,
     getAuctionStatus: () => _auctionStatus,
@@ -355,8 +353,12 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
   }
 }
 
-export const addBidResponse = hook('async', function(adUnitCode, bid) {
-  this.dispatch.call(this.bidderRequest, adUnitCode, bid);
+/**
+ * addBidResponse may return a Promise; if it does, the auction will attempt to
+ * wait for it to complete (successfully or not) before closing.
+ */
+export const addBidResponse = hook('sync', function(adUnitCode, bid) {
+  return this.dispatch.call(this.bidderRequest, adUnitCode, bid);
 }, 'addBidResponse');
 
 export const addBidderRequests = hook('sync', function(bidderRequests) {
@@ -374,6 +376,32 @@ export function auctionCallbacks(auctionDone, auctionInstance) {
   let allAdapterCalledDone = false;
   let bidderRequestsDone = new Set();
   let bidResponseMap = {};
+  const ready = {};
+
+  function waitFor(bidderRequest, result) {
+    const id = bidderRequest.bidderRequestId;
+    if (ready[id] == null) {
+      ready[id] = Promise.resolve();
+    }
+    ready[id] = ready[id].then(() => Promise.resolve(result).catch(() => {}))
+  }
+
+  function guard(bidderRequest, fn) {
+    let timeout = bidderRequest.timeout;
+    if (timeout == null || timeout > auctionInstance.getTimeout()) {
+      timeout = auctionInstance.getTimeout();
+    }
+    const timeRemaining = auctionInstance.getAuctionStart() + timeout - Date.now();
+    const wait = ready[bidderRequest.bidderRequestId];
+    if (wait != null && timeRemaining > 0) {
+      Promise.race([
+        new Promise((resolve) => setTimeout(resolve, timeRemaining)),
+        wait
+      ]).then(fn);
+    } else {
+      fn();
+    }
+  }
 
   function afterBidAdded() {
     outstandingBidsAdded--;
@@ -382,7 +410,7 @@ export function auctionCallbacks(auctionDone, auctionInstance) {
     }
   }
 
-  function addBidResponse(adUnitCode, bid) {
+  function handleBidResponse(adUnitCode, bid) {
     let bidderRequest = this;
 
     bidResponseMap[bid.requestId] = true;
@@ -429,8 +457,15 @@ export function auctionCallbacks(auctionDone, auctionInstance) {
   }
 
   return {
-    addBidResponse,
-    adapterDone
+    addBidResponse: function (...args) {
+      waitFor(this, addBidResponse.apply({
+        dispatch: handleBidResponse,
+        bidderRequest: this
+      }, args));
+    },
+    adapterDone: function () {
+      guard(this, adapterDone.bind(this))
+    }
   }
 }
 

test/helpers/syncPromise.js

@@ -0,0 +1,71 @@
+const orig = {};
+['resolve', 'reject', 'all', 'race', 'allSettled'].forEach((k) => orig[k] = Promise[k].bind(Promise))
+
+// Callbacks attached through Promise.resolve(value).then(...) will usually
+// not execute immediately even if `value` is immediately available. This
+// breaks tests that were written before promises even though they are semantically still valid.
+// They can be made to work by making promises quasi-synchronous.
+
+export function SyncPromise(value, fail = false) {
+  if (value instanceof SyncPromise) {
+    return value;
+  } else if (typeof value === 'object' && typeof value.then === 'function') {
+    return orig.resolve(value);
+  } else {
+    Object.assign(this, {
+      then: function (cb, err) {
+        const handler = fail ? err : cb;
+        if (handler != null) {
+          return new SyncPromise(handler(value));
+        } else {
+          return this;
+        }
+      },
+      catch: function (cb) {
+        if (fail) {
+          return new SyncPromise(cb(value))
+        } else {
+          return this;
+        }
+      },
+      finally: function (cb) {
+        cb();
+        return this;
+      },
+      __value: fail ? {status: 'rejected', reason: value} : {status: 'fulfilled', value}
+    })
+  }
+}
+
+Object.assign(SyncPromise, {
+  resolve: (val) => new SyncPromise(val),
+  reject: (val) => new SyncPromise(val, true),
+  race: (promises) => promises.find((p) => p instanceof SyncPromise) || orig.race(promises),
+  allSettled: (promises) => {
+    if (promises.every((p) => p instanceof SyncPromise)) {
+      return new SyncPromise(promises.map((p) => p.__value))
+    } else {
+      return orig.allSettled(promises);
+    }
+  },
+  all: (promises) => {
+    if (promises.every((p) => p instanceof SyncPromise)) {
+      return SyncPromise.allSettled(promises).then((result) => {
+        const err = result.find((r) => r.status === 'rejected');
+        if (err != null) {
+          return new SyncPromise(err.reason, true);
+        } else {
+          return new SyncPromise(result.map((r) => r.value))
+        }
+      })
+    } else {
+      return orig.all(promises);
+    }
+  }
+})
+
+export function synchronizePromise(sandbox) {
+  Object.keys(orig).forEach((k) => {
+    sandbox.stub(window.Promise, k).callsFake(SyncPromise[k]);
+  })
+}