Skip to content

Commit

Permalink
Prebid core: fix exception in requestBids introduced by prebid#9106 (…
Browse files Browse the repository at this point in the history 
…prebid#9194)

* Revert "Prebid core: return a promise from `requestBids` (prebid#9106)"

This reverts commit 64aff9b.

* Revert "Revert "Prebid core: return a promise from `requestBids` (prebid#9106)""

This reverts commit bf5ee30.

* Prebid core: fix exception in `requestBids` introduced by prebid#9106

* Add comments
  • Loading branch information
@dgirardi
dgirardi committed Nov 3, 2022
1 parent c39afc8 commit 6fa90f1
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 80 deletions.
162 changes: 83 additions & 79 deletions src/prebid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {default as adapterManager, gdprDataHandler, getS2SBidderSet, uspDataHand
import CONSTANTS from './constants.json';
import * as events from './events.js';
import {newMetrics, useMetrics} from './utils/perfMetrics.js';
import {defer} from './utils/promise.js';

const $$PREBID_GLOBAL$$ = getGlobal();
const { triggerUserSyncs } = userSync;
Expand Down Expand Up @@ -622,31 +623,36 @@ $$PREBID_GLOBAL$$.removeAdUnit = function (adUnitCode) {
* @alias module:pbjs.requestBids
*/
$$PREBID_GLOBAL$$.requestBids = (function() {
const delegate = hook('sync', function ({ bidsBackHandler, timeout, adUnits, adUnitCodes, labels, auctionId, ttlBuffer, ortb2, metrics } = {}) {
const delegate = hook('async', function ({ bidsBackHandler, timeout, adUnits, adUnitCodes, labels, auctionId, ttlBuffer, ortb2, metrics, defer } = {}) {
events.emit(REQUEST_BIDS);
const cbTimeout = timeout || config.getConfig('bidderTimeout');
logInfo('Invoking $$PREBID_GLOBAL$$.requestBids', arguments);
const ortb2Fragments = {
global: mergeDeep({}, config.getAnyConfig('ortb2') || {}, ortb2 || {}),
bidder: Object.fromEntries(Object.entries(config.getBidderConfig()).map(([bidder, cfg]) => [bidder, cfg.ortb2]).filter(([_, ortb2]) => ortb2 != null))
}
return startAuction({bidsBackHandler, timeout: cbTimeout, adUnits, adUnitCodes, labels, auctionId, ttlBuffer, ortb2Fragments, metrics});
return startAuction({bidsBackHandler, timeout: cbTimeout, adUnits, adUnitCodes, labels, auctionId, ttlBuffer, ortb2Fragments, metrics, defer});
}, 'requestBids');

return wrapHook(delegate, function requestBids(req = {}) {
// if the request does not specify adUnits, clone the global adUnit array - before
// any hook has a chance to run.
// unlike the main body of `delegate`, this runs before any other hook has a chance to;
// it's also not restricted in its return value in the way `async` hooks are.

// if the request does not specify adUnits, clone the global adUnit array;
// otherwise, if the caller goes on to use addAdUnits/removeAdUnits, any asynchronous logic
// in any hook might see their effects.
req.metrics = newMetrics();
req.metrics.checkpoint('requestBids');
let adUnits = req.adUnits || $$PREBID_GLOBAL$$.adUnits;
req.adUnits = (isArray(adUnits) ? adUnits.slice() : [adUnits]);
return delegate.call(this, req);

req.metrics = newMetrics();
req.metrics.checkpoint('requestBids');
req.defer = defer({promiseFactory: (r) => new Promise(r)})
delegate.call(this, req);
return req.defer.promise;
});
})();

export const startAuction = hook('sync', function ({ bidsBackHandler, timeout: cbTimeout, adUnits, ttlBuffer, adUnitCodes, labels, auctionId, ortb2Fragments, metrics } = {}) {
export const startAuction = hook('async', function ({ bidsBackHandler, timeout: cbTimeout, adUnits, ttlBuffer, adUnitCodes, labels, auctionId, ortb2Fragments, metrics, defer } = {}) {
const s2sBidders = getS2SBidderSet(config.getConfig('s2sConfig') || []);
adUnits = useMetrics(metrics).measureTime('requestBids.validate', () => checkAdUnitSetup(adUnits));

Expand All @@ -658,85 +664,83 @@ export const startAuction = hook('sync', function ({ bidsBackHandler, timeout: c
adUnitCodes = adUnits && adUnits.map(unit => unit.code);
}

return new Promise((resolve) => {
function auctionDone(bids, timedOut, auctionId) {
if (typeof bidsBackHandler === 'function') {
try {
bidsBackHandler(bids, timedOut, auctionId);
} catch (e) {
logError('Error executing bidsBackHandler', null, e);
}
function auctionDone(bids, timedOut, auctionId) {
if (typeof bidsBackHandler === 'function') {
try {
bidsBackHandler(bids, timedOut, auctionId);
} catch (e) {
logError('Error executing bidsBackHandler', null, e);
}
resolve({bids, timedOut, auctionId});
}
defer.resolve({bids, timedOut, auctionId})
}

/*
* for a given adunit which supports a set of mediaTypes
* and a given bidder which supports a set of mediaTypes
* a bidder is eligible to participate on the adunit
* if it supports at least one of the mediaTypes on the adunit
*/
adUnits.forEach(adUnit => {
// get the adunit's mediaTypes, defaulting to banner if mediaTypes isn't present
const adUnitMediaTypes = Object.keys(adUnit.mediaTypes || { 'banner': 'banner' });

/*
* for a given adunit which supports a set of mediaTypes
* and a given bidder which supports a set of mediaTypes
* a bidder is eligible to participate on the adunit
* if it supports at least one of the mediaTypes on the adunit
*/
adUnits.forEach(adUnit => {
// get the adunit's mediaTypes, defaulting to banner if mediaTypes isn't present
const adUnitMediaTypes = Object.keys(adUnit.mediaTypes || { 'banner': 'banner' });

// get the bidder's mediaTypes
const allBidders = adUnit.bids.map(bid => bid.bidder);
const bidderRegistry = adapterManager.bidderRegistry;

const bidders = allBidders.filter(bidder => !s2sBidders.has(bidder));

const tid = adUnit.ortb2Imp?.ext?.tid || generateUUID();
adUnit.transactionId = tid;
if (ttlBuffer != null && !adUnit.hasOwnProperty('ttlBuffer')) {
adUnit.ttlBuffer = ttlBuffer;
// get the bidder's mediaTypes
const allBidders = adUnit.bids.map(bid => bid.bidder);
const bidderRegistry = adapterManager.bidderRegistry;

const bidders = allBidders.filter(bidder => !s2sBidders.has(bidder));

const tid = adUnit.ortb2Imp?.ext?.tid || generateUUID();
adUnit.transactionId = tid;
if (ttlBuffer != null && !adUnit.hasOwnProperty('ttlBuffer')) {
adUnit.ttlBuffer = ttlBuffer;
}
// Populate ortb2Imp.ext.tid with transactionId. Specifying a transaction ID per item in the ortb impression array, lets multiple transaction IDs be transmitted in a single bid request.
deepSetValue(adUnit, 'ortb2Imp.ext.tid', tid);

bidders.forEach(bidder => {
const adapter = bidderRegistry[bidder];
const spec = adapter && adapter.getSpec && adapter.getSpec();
// banner is default if not specified in spec
const bidderMediaTypes = (spec && spec.supportedMediaTypes) || ['banner'];

// check if the bidder's mediaTypes are not in the adUnit's mediaTypes
const bidderEligible = adUnitMediaTypes.some(type => includes(bidderMediaTypes, type));
if (!bidderEligible) {
// drop the bidder from the ad unit if it's not compatible
logWarn(unsupportedBidderMessage(adUnit, bidder));
adUnit.bids = adUnit.bids.filter(bid => bid.bidder !== bidder);
} else {
adunitCounter.incrementBidderRequestsCounter(adUnit.code, bidder);
}
// Populate ortb2Imp.ext.tid with transactionId. Specifying a transaction ID per item in the ortb impression array, lets multiple transaction IDs be transmitted in a single bid request.
deepSetValue(adUnit, 'ortb2Imp.ext.tid', tid);

bidders.forEach(bidder => {
const adapter = bidderRegistry[bidder];
const spec = adapter && adapter.getSpec && adapter.getSpec();
// banner is default if not specified in spec
const bidderMediaTypes = (spec && spec.supportedMediaTypes) || ['banner'];

// check if the bidder's mediaTypes are not in the adUnit's mediaTypes
const bidderEligible = adUnitMediaTypes.some(type => includes(bidderMediaTypes, type));
if (!bidderEligible) {
// drop the bidder from the ad unit if it's not compatible
logWarn(unsupportedBidderMessage(adUnit, bidder));
adUnit.bids = adUnit.bids.filter(bid => bid.bidder !== bidder);
} else {
adunitCounter.incrementBidderRequestsCounter(adUnit.code, bidder);
}
});
adunitCounter.incrementRequestsCounter(adUnit.code);
});
adunitCounter.incrementRequestsCounter(adUnit.code);
});

if (!adUnits || adUnits.length === 0) {
logMessage('No adUnits configured. No bids requested.');
auctionDone();
} else {
const auction = auctionManager.createAuction({
adUnits,
adUnitCodes,
callback: auctionDone,
cbTimeout,
labels,
auctionId,
ortb2Fragments,
metrics,
});

let adUnitsLen = adUnits.length;
if (adUnitsLen > 15) {
logInfo(`Current auction ${auction.getAuctionId()} contains ${adUnitsLen} adUnits.`, adUnits);
}
if (!adUnits || adUnits.length === 0) {
logMessage('No adUnits configured. No bids requested.');
auctionDone();
} else {
const auction = auctionManager.createAuction({
adUnits,
adUnitCodes,
callback: auctionDone,
cbTimeout,
labels,
auctionId,
ortb2Fragments,
metrics,
});

adUnitCodes.forEach(code => targeting.setLatestAuctionForAdUnit(code, auction.getAuctionId()));
auction.callBids();
let adUnitsLen = adUnits.length;
if (adUnitsLen > 15) {
logInfo(`Current auction ${auction.getAuctionId()} contains ${adUnitsLen} adUnits.`, adUnits);
}
});

adUnitCodes.forEach(code => targeting.setLatestAuctionForAdUnit(code, auction.getAuctionId()));
auction.callBids();
}
}, 'startAuction');

export function executeCallbacks(fn, reqBidsConfigObj) {
Expand Down
18 changes: 17 additions & 1 deletion test/spec/unit/pbjs_api_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,19 @@ describe('Unit: Prebid Module', function () {
});

describe('returns a promise that resolves', () => {
function delayHook(next, ...args) {
setTimeout(() => next(...args))
}

beforeEach(() => {
// make sure the return value works correctly when hooks give up priority
$$PREBID_GLOBAL$$.requestBids.before(delayHook)
});

afterEach(() => {
$$PREBID_GLOBAL$$.requestBids.getHooks({hook: delayHook}).remove();
});

Object.entries({
'immediately, without bidsBackHandler': (req) => $$PREBID_GLOBAL$$.requestBids(req),
'after bidsBackHandler': (() => {
Expand Down Expand Up @@ -1665,7 +1678,10 @@ describe('Unit: Prebid Module', function () {
sinon.assert.match(bids[bid.adUnitCode].bids[0], bid)
done();
});
completeAuction([bid]);
// `completeAuction` won't work until we're out of `delayHook`
// and the mocked auction has been set up;
// setTimeout here takes us after the setTimeout in `delayHook`
setTimeout(() => completeAuction([bid]));
})
})
})
Expand Down

0 comments on commit 6fa90f1

Please sign in to comment.