Skip to content

Commit

Permalink
Core: improve and fix bid timeout logic (#10379)
Browse files Browse the repository at this point in the history 
* Clean up timeout logic: doCallbacksIfTimedOut

* Core: simplify bid timeouts, fix s2s timeout edge case

* use more descriptive varnames
  • Loading branch information
@dgirardi
dgirardi authored Sep 14, 2023
1 parent 61d4f5c commit 8075455
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 142 deletions.
4 changes: 0 additions & 4 deletions modules/adpod.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
import {
addBidToAuction,
AUCTION_IN_PROGRESS,
doCallbacksIfTimedout,
getPriceByGranularity,
getPriceGranularity
} from '../src/auction.js';
Expand Down Expand Up @@ -212,9 +211,6 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) {
store(bidList, function (error, cacheIds) {
if (error) {
logWarn(`Failed to save to the video cache: ${error}. Video bid(s) must be discarded.`);
for (let i = 0; i < bidList.length; i++) {
doCallbacksIfTimedout(auctionInstance, bidList[i]);
}
} else {
for (let i = 0; i < cacheIds.length; i++) {
// when uuid in response is empty string then the key already existed, so this bid wasn't cached
Expand Down
37 changes: 20 additions & 17 deletions src/adapterManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,13 @@ adapterManager.callBids = (adUnits, bidRequests, addBidResponse, doneCb, request
return;
}

let [clientBidRequests, serverBidRequests] = bidRequests.reduce((partitions, bidRequest) => {
let [clientBidderRequests, serverBidderRequests] = bidRequests.reduce((partitions, bidRequest) => {
partitions[Number(typeof bidRequest.src !== 'undefined' && bidRequest.src === CONSTANTS.S2S.SRC)].push(bidRequest);
return partitions;
}, [[], []]);

var uniqueServerBidRequests = [];
serverBidRequests.forEach(serverBidRequest => {
serverBidderRequests.forEach(serverBidRequest => {
var index = -1;
for (var i = 0; i < uniqueServerBidRequests.length; ++i) {
if (serverBidRequest.uniquePbsTid === uniqueServerBidRequests[i].uniquePbsTid) {
Expand All @@ -413,14 +413,17 @@ adapterManager.callBids = (adUnits, bidRequests, addBidResponse, doneCb, request
let uniquePbsTid = uniqueServerBidRequests[counter].uniquePbsTid;
let adUnitsS2SCopy = uniqueServerBidRequests[counter].adUnitsS2SCopy;

let uniqueServerRequests = serverBidRequests.filter(serverBidRequest => serverBidRequest.uniquePbsTid === uniquePbsTid);
let uniqueServerRequests = serverBidderRequests.filter(serverBidRequest => serverBidRequest.uniquePbsTid === uniquePbsTid);

if (s2sAdapter) {
let s2sBidRequest = {'ad_units': adUnitsS2SCopy, s2sConfig, ortb2Fragments};
if (s2sBidRequest.ad_units.length) {
let doneCbs = uniqueServerRequests.map(bidRequest => {
bidRequest.start = timestamp();
return doneCb.bind(bidRequest);
return function () {
onTimelyResponse(bidRequest.bidderRequestId);
doneCbs.apply(bidRequest, arguments);
}
});

const bidders = getBidderCodes(s2sBidRequest.ad_units).filter((bidder) => adaptersServerSide.includes(bidder));
Expand All @@ -435,7 +438,7 @@ adapterManager.callBids = (adUnits, bidRequests, addBidResponse, doneCb, request
// make bid requests
s2sAdapter.callBids(
s2sBidRequest,
serverBidRequests,
serverBidderRequests,
addBidResponse,
() => doneCbs.forEach(done => done()),
s2sAjax
Expand All @@ -449,35 +452,35 @@ adapterManager.callBids = (adUnits, bidRequests, addBidResponse, doneCb, request
});

// handle client adapter requests
clientBidRequests.forEach(bidRequest => {
bidRequest.start = timestamp();
clientBidderRequests.forEach(bidderRequest => {
bidderRequest.start = timestamp();
// TODO : Do we check for bid in pool from here and skip calling adapter again ?
const adapter = _bidderRegistry[bidRequest.bidderCode];
config.runWithBidder(bidRequest.bidderCode, () => {
const adapter = _bidderRegistry[bidderRequest.bidderCode];
config.runWithBidder(bidderRequest.bidderCode, () => {
logMessage(`CALLING BIDDER`);
events.emit(CONSTANTS.EVENTS.BID_REQUESTED, bidRequest);
events.emit(CONSTANTS.EVENTS.BID_REQUESTED, bidderRequest);
});
let ajax = ajaxBuilder(requestBidsTimeout, requestCallbacks ? {
request: requestCallbacks.request.bind(null, bidRequest.bidderCode),
request: requestCallbacks.request.bind(null, bidderRequest.bidderCode),
done: requestCallbacks.done
} : undefined);
const adapterDone = doneCb.bind(bidRequest);
const adapterDone = doneCb.bind(bidderRequest);
try {
config.runWithBidder(
bidRequest.bidderCode,
bidderRequest.bidderCode,
bind.call(
adapter.callBids,
adapter,
bidRequest,
bidderRequest,
addBidResponse,
adapterDone,
ajax,
onTimelyResponse,
config.callbackWithBidder(bidRequest.bidderCode)
() => onTimelyResponse(bidderRequest.bidderRequestId),
config.callbackWithBidder(bidderRequest.bidderCode)
)
);
} catch (e) {
logError(`${bidRequest.bidderCode} Bid Adapter emitted an uncaught error when parsing their bidRequest`, {e, bidRequest});
logError(`${bidderRequest.bidderCode} Bid Adapter emitted an uncaught error when parsing their bidRequest`, {e, bidRequest: bidderRequest});
adapterDone();
}
});
Expand Down
70 changes: 15 additions & 55 deletions src/auction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ import {
adUnitsFilter,
bind,
deepAccess,
flatten,
generateUUID,
getValue,
isEmpty,
Expand Down Expand Up @@ -142,7 +141,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
const _adUnitCodes = adUnitCodes;
const _auctionId = auctionId || generateUUID();
const _timeout = cbTimeout;
const _timelyBidders = new Set();
const _timelyRequests = new Set();
const done = defer();
let _bidsRejected = [];
let _callback = callback;
Expand All @@ -152,7 +151,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
let _winningBids = [];
let _auctionStart;
let _auctionEnd;
let _timer;
let _timeoutTimer;
let _auctionStatus;
let _nonBids = [];

Expand Down Expand Up @@ -183,24 +182,20 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
}

function startAuctionTimer() {
const timedOut = true;
const timeoutCallback = executeCallback.bind(null, timedOut);
let timer = setTimeout(timeoutCallback, _timeout);
_timer = timer;
_timeoutTimer = setTimeout(() => executeCallback(true), _timeout);
}

function executeCallback(timedOut, cleartimer) {
// clear timer when done calls executeCallback
if (cleartimer) {
clearTimeout(_timer);
function executeCallback(timedOut) {
if (!timedOut) {
clearTimeout(_timeoutTimer);
}
if (_auctionEnd === undefined) {
let timedOutBidders = [];
let timedOutRequests = [];
if (timedOut) {
logMessage(`Auction ${_auctionId} timedOut`);
timedOutBidders = getTimedOutBids(_bidderRequests, _timelyBidders);
if (timedOutBidders.length) {
events.emit(CONSTANTS.EVENTS.BID_TIMEOUT, timedOutBidders);
timedOutRequests = _bidderRequests.filter(rq => !_timelyRequests.has(rq.bidderRequestId)).flatMap(br => br.bids)
if (timedOutRequests.length) {
events.emit(CONSTANTS.EVENTS.BID_TIMEOUT, timedOutRequests);
}
}

Expand All @@ -226,8 +221,8 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
logError('Error executing bidsBackHandler', null, e);
} finally {
// Calling timed out bidders
if (timedOutBidders.length) {
adapterManager.callTimedOutBidders(adUnits, timedOutBidders, _timeout);
if (timedOutRequests.length) {
adapterManager.callTimedOutBidders(adUnits, timedOutRequests, _timeout);
}
// Only automatically sync if the publisher has not chosen to "enableOverride"
let userSyncConfig = config.getConfig('userSync') || {};
Expand All @@ -245,11 +240,11 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
// when all bidders have called done callback atleast once it means auction is complete
logInfo(`Bids Received for Auction with id: ${_auctionId}`, _bidsReceived);
_auctionStatus = AUCTION_COMPLETED;
executeCallback(false, true);
executeCallback(false);
}

function onTimelyResponse(bidderCode) {
_timelyBidders.add(bidderCode);
function onTimelyResponse(bidderRequestId) {
_timelyRequests.add(bidderRequestId);
}

function callBids() {
Expand Down Expand Up @@ -387,7 +382,6 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
addBidReceived,
addBidRejected,
addNoBid,
executeCallback,
callBids,
addWinningBid,
setBidTargeting,
Expand Down Expand Up @@ -557,21 +551,13 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM
}
}

export function doCallbacksIfTimedout(auctionInstance, bidResponse) {
if (bidResponse.timeToRespond > auctionInstance.getTimeout() + config.getConfig('timeoutBuffer')) {
auctionInstance.executeCallback(true);
}
}

// Add a bid to the auction.
export function addBidToAuction(auctionInstance, bidResponse) {
setupBidTargeting(bidResponse);

useMetrics(bidResponse.metrics).timeSince('addBidResponse', 'addBidResponse.total');
auctionInstance.addBidReceived(bidResponse);
events.emit(CONSTANTS.EVENTS.BID_RESPONSE, bidResponse);

doCallbacksIfTimedout(auctionInstance, bidResponse);
}

// Video bids may fail if the cache is down, or there's trouble on the network.
Expand Down Expand Up @@ -618,16 +604,11 @@ const _storeInCache = (batch) => {
const { auctionInstance, bidResponse, afterBidAdded } = batch[i];
if (error) {
logWarn(`Failed to save to the video cache: ${error}. Video bid must be discarded.`);

doCallbacksIfTimedout(auctionInstance, bidResponse);
} else {
if (cacheId.uuid === '') {
logWarn(`Supplied video cache key was already in use by Prebid Cache; caching attempt was rejected. Video bid must be discarded.`);

doCallbacksIfTimedout(auctionInstance, bidResponse);
} else {
bidResponse.videoCacheKey = cacheId.uuid;

if (!bidResponse.vastUrl) {
bidResponse.vastUrl = getCacheUrl(bidResponse.videoCacheKey);
}
Expand Down Expand Up @@ -1017,24 +998,3 @@ function groupByPlacement(bidsByPlacement, bid) {
bidsByPlacement[bid.adUnitCode].bids.push(bid);
return bidsByPlacement;
}

/**
* Returns a list of bids that we haven't received a response yet where the bidder did not call done
* @param {BidRequest[]} bidderRequests List of bids requested for auction instance
* @param {Set} timelyBidders Set of bidders which responded in time
*
* @typedef {Object} TimedOutBid
* @property {string} bidId The id representing the bid
* @property {string} bidder The string name of the bidder
* @property {string} adUnitCode The code used to uniquely identify the ad unit on the publisher's page
* @property {string} auctionId The id representing the auction
*
* @return {Array<TimedOutBid>} List of bids that Prebid hasn't received a response for
*/
function getTimedOutBids(bidderRequests, timelyBidders) {
const timedOutBids = bidderRequests
.map(bid => (bid.bids || []).filter(bid => !timelyBidders.has(bid.bidder)))
.reduce(flatten, []);

return timedOutBids;
}
3 changes: 1 addition & 2 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,7 @@ export function isValidMediaTypes(mediaTypes) {
export function getUserConfiguredParams(adUnits, adUnitCode, bidder) {
return adUnits
.filter(adUnit => adUnit.code === adUnitCode)
.map((adUnit) => adUnit.bids)
.reduce(flatten, [])
.flatMap((adUnit) => adUnit.bids)
.filter((bidderData) => bidderData.bidder === bidder)
.map((bidderData) => bidderData.params || {});
}
Expand Down
Loading

0 comments on commit 8075455

Please sign in to comment.