Skip to content

Commit

Permalink
Fix for early auction close with video + done cb cleanup (prebid#3024)
Browse files Browse the repository at this point in the history 
* done cb cleanup

* rename some vars

* added unit tests, updates after changes requested

* add deprecated notice to function
  • Loading branch information
@jaiminpanchal27 @mkendall07
jaiminpanchal27 authored and mkendall07 committed Sep 11, 2018
1 parent 73e9ef1 commit f272031
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 84 deletions.
8 changes: 1 addition & 7 deletions modules/prebidServerBidAdapter/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,13 +630,7 @@ export function PrebidServer() {
utils.logError('error parsing response: ', result.status);
}

const videoBid = bids.some(bidResponse => bidResponse.bid.mediaType === 'video');
const cacheEnabled = config.getConfig('cache.url');

// video bids with cache enabled need to be cached first before they are considered done
if (!(videoBid && cacheEnabled)) {
done();
}
done();
doClientSideSyncs(requestedBidders);
}

Expand Down
6 changes: 2 additions & 4 deletions src/adaptermanager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,7 @@ exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb, requestCallbac
if (s2sBidRequest.ad_units.length) {
let doneCbs = serverBidRequests.map(bidRequest => {
bidRequest.start = timestamp();
bidRequest.doneCbCallCount = 0;
return doneCb(bidRequest.bidderRequestId)
return doneCb;
});

// only log adapters that actually have adUnit bids
Expand Down Expand Up @@ -372,12 +371,11 @@ exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb, requestCallbac
utils.logMessage(`CALLING BIDDER ======= ${bidRequest.bidderCode}`);
events.emit(CONSTANTS.EVENTS.BID_REQUESTED, bidRequest);
bidRequest.doneCbCallCount = 0;
let done = doneCb(bidRequest.bidderRequestId);
let ajax = ajaxBuilder(requestBidsTimeout, requestCallbacks ? {
request: requestCallbacks.request.bind(null, bidRequest.bidderCode),
done: requestCallbacks.done
} : undefined);
adapter.callBids(bidRequest, addBidResponse, done, ajax);
adapter.callBids(bidRequest, addBidResponse, doneCb, ajax);
});
}

Expand Down
16 changes: 1 addition & 15 deletions src/adapters/bidderFactory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,22 +177,8 @@ export function newBidder(spec) {
// register any required usersync pixels.
const responses = [];
function afterAllResponses(bids) {
const bidsArray = bids ? (bids[0] ? bids : [bids]) : [];

const videoBid = bidsArray.some(bid => bid.mediaType === 'video');
const cacheEnabled = config.getConfig('cache.url');

// video bids with cache enabled need to be cached first before they are considered done
if (!(videoBid && cacheEnabled)) {
done();
}

// TODO: the code above needs to be refactored. We should always call done when we're done. if the auction
// needs to do cleanup before _it_ can be done it should handle that itself in the auction. It should _not_
// require us, the bidders, to conditionally call done. That makes the whole done API very flaky.
// As soon as that is refactored, we can move this emit event where it should be, within the done function.
done();
events.emit(CONSTANTS.EVENTS.BIDDER_DONE, bidderRequest);

registerSyncs(responses, bidderRequest.gdprConsent);
}

Expand Down
114 changes: 70 additions & 44 deletions src/auction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
* @property {function(): void} callBids - sends requests to all adapters for bids
*/

import { uniques, flatten, timestamp, adUnitsFilter, delayExecution, getBidderRequest } from './utils';
import { uniques, flatten, timestamp, adUnitsFilter, getBidderRequest, deepAccess } from './utils';
import { getPriceBucketString } from './cpmBucketManager';
import { getNativeTargeting } from './native';
import { getCacheUrl, store } from './videoCache';
Expand All @@ -58,6 +58,7 @@ import { userSync } from 'src/userSync';
import { createHook } from 'src/hook';
import find from 'core-js/library/fn/array/find';
import includes from 'core-js/library/fn/array/includes';
import { OUTSTREAM } from './video';

const { syncUsers } = userSync;
const utils = require('./utils');
Expand Down Expand Up @@ -155,29 +156,21 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels})
}
}

function done(bidRequestId) {
const innerBidRequestId = bidRequestId;
return delayExecution(function() {
const request = find(_bidderRequests, (bidRequest) => {
return innerBidRequestId === bidRequest.bidderRequestId;
});

// this is done for cache-enabled video bids in tryAddVideoBid, after the cache is stored
request.doneCbCallCount += 1;
bidsBackAll();
}, 1);
function auctionDone(bidderCount) {
let doneCalled = 0;
return function() {
doneCalled++;
if (doneCalled === bidderCount) {
closeAuction();
}
}
}

/**
* Execute bidBackHandler if all bidders have called done.
*/
function bidsBackAll() {
if (_bidderRequests.every((bidRequest) => bidRequest.doneCbCallCount >= 1)) {
// when all bidders have called done callback atleast once it means auction is complete
utils.logInfo(`Bids Received for Auction with id: ${_auctionId}`, _bidsReceived);
_auctionStatus = AUCTION_COMPLETED;
executeCallback(false, true);
}
function closeAuction() {
// when all bidders have called done callback atleast once it means auction is complete
utils.logInfo(`Bids Received for Auction with id: ${_auctionId}`, _bidsReceived);
_auctionStatus = AUCTION_COMPLETED;
executeCallback(false, true);
}

function callBids() {
Expand Down Expand Up @@ -206,7 +199,11 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels})
};
events.emit(CONSTANTS.EVENTS.AUCTION_INIT, auctionInit);

adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(this), done.bind(this), {
let callbacks = auctionCallbacks(auctionDone(bidRequests.length), this);
let boundObj = {
auctionAddBidResponse: callbacks.addBidResponse
}
adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(boundObj), callbacks.adapterDone, {
request(source, origin) {
increment(outstandingRequests, origin);
increment(requests, source);
Expand Down Expand Up @@ -288,7 +285,6 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels})
addBidReceived,
executeCallback,
callBids,
bidsBackAll,
addWinningBid,
getWinningBids: () => _winningBids,
getTimeout: () => _timeout,
Expand All @@ -301,6 +297,50 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels})
}
}

export const addBidResponse = createHook('asyncSeries', function(adUnitCode, bid) {
this.auctionAddBidResponse(adUnitCode, bid);
}, 'addBidResponse');

export function auctionCallbacks(auctionDone, auctionInstance) {
let outstandingBidsAdded = 0;
let doneCalled = false;

function afterBidAdded() {
outstandingBidsAdded--;
if (doneCalled && outstandingBidsAdded === 0) {
auctionDone()
}
}

function addBidResponse(adUnitCode, bid) {
outstandingBidsAdded++;
let bidRequests = auctionInstance.getBidRequests();
let auctionId = auctionInstance.getAuctionId();

let bidRequest = getBidderRequest(bidRequests, bid.bidderCode, adUnitCode);
let bidResponse = getPreparedBidForAuction({adUnitCode, bid, bidRequest, auctionId});

if (bidResponse.mediaType === 'video') {
tryAddVideoBid(auctionInstance, bidResponse, afterBidAdded);
} else {
addBidToAuction(auctionInstance, bidResponse);
afterBidAdded();
}
}

function adapterDone() {
doneCalled = true;
if ((outstandingBidsAdded === 0)) {
auctionDone();
}
}

return {
addBidResponse,
adapterDone
}
}

function doCallbacksIfTimedout(auctionInstance, bidResponse) {
if (bidResponse.timeToRespond > auctionInstance.getTimeout() + config.getConfig('timeoutBuffer')) {
auctionInstance.executeCallback(true);
Expand All @@ -316,9 +356,11 @@ function addBidToAuction(auctionInstance, bidResponse) {
}

// Video bids may fail if the cache is down, or there's trouble on the network.
function tryAddVideoBid(auctionInstance, bidResponse, bidRequest) {
function tryAddVideoBid(auctionInstance, bidResponse, afterBidAdded) {
let addBid = true;
if (config.getConfig('cache.url')) {
const context = deepAccess(bidResponse, 'context');

if (config.getConfig('cache.url') && context !== OUTSTREAM) {
if (!bidResponse.videoCacheKey) {
addBid = false;
store([bidResponse], function (error, cacheIds) {
Expand All @@ -331,10 +373,8 @@ function tryAddVideoBid(auctionInstance, bidResponse, bidRequest) {
if (!bidResponse.vastUrl) {
bidResponse.vastUrl = getCacheUrl(bidResponse.videoCacheKey);
}
// only set this prop after the bid has been cached to avoid early ending auction early in bidsBackAll
bidRequest.doneCbCallCount += 1;
addBidToAuction(auctionInstance, bidResponse);
auctionInstance.bidsBackAll();
afterBidAdded();
}
});
} else if (!bidResponse.vastUrl) {
Expand All @@ -344,24 +384,10 @@ function tryAddVideoBid(auctionInstance, bidResponse, bidRequest) {
}
if (addBid) {
addBidToAuction(auctionInstance, bidResponse);
afterBidAdded();
}
}

export const addBidResponse = createHook('asyncSeries', function(adUnitCode, bid) {
let auctionInstance = this;
let bidRequests = auctionInstance.getBidRequests();
let auctionId = auctionInstance.getAuctionId();

let bidRequest = getBidderRequest(bidRequests, bid.bidderCode, adUnitCode);
let bidResponse = getPreparedBidForAuction({adUnitCode, bid, bidRequest, auctionId});

if (bidResponse.mediaType === 'video') {
tryAddVideoBid(auctionInstance, bidResponse, bidRequest);
} else {
addBidToAuction(auctionInstance, bidResponse);
}
}, 'addBidResponse');

// Postprocess the bids so that all the universal properties exist, no matter which bidder they came from.
// This should be called before addBidToAuction().
function getPreparedBidForAuction({adUnitCode, bid, bidRequest, auctionId}) {
Expand Down
9 changes: 9 additions & 0 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ export function parseGPTSingleSizeArray(singleSize) {
}
};

/**
* @deprecated This function will be removed soon
*/
exports.getTopWindowLocation = function() {
if (exports.inIframe()) {
let loc;
Expand All @@ -202,6 +205,9 @@ exports.getTopWindowLocation = function() {
return exports.getWindowLocation();
}

/**
* @deprecated This function will be removed soon
*/
exports.getTopFrameReferrer = function () {
try {
// force an exception in x-domain environments. #1509
Expand All @@ -221,6 +227,9 @@ exports.getTopFrameReferrer = function () {
}
};

/**
* @deprecated This function will be removed soon
*/
exports.getAncestorOrigins = function () {
if (window.document.location && window.document.location.ancestorOrigins &&
window.document.location.ancestorOrigins.length >= 1) {
Expand Down
2 changes: 1 addition & 1 deletion src/video.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { config } from '../src/config';
import includes from 'core-js/library/fn/array/includes';

const VIDEO_MEDIA_TYPE = 'video';
const OUTSTREAM = 'outstream';
export const OUTSTREAM = 'outstream';

/**
* Helper functions for working with video-enabled adUnits
Expand Down
Loading

0 comments on commit f272031

Please sign in to comment.