diff --git a/modules/adpod.js b/modules/adpod.js index 4ab8e4e5ab9..0318785d55e 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -28,7 +28,6 @@ import { import { addBidToAuction, AUCTION_IN_PROGRESS, - doCallbacksIfTimedout, getPriceByGranularity, getPriceGranularity } from '../src/auction.js'; @@ -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 diff --git a/src/adapterManager.js b/src/adapterManager.js index 6513d41ca3c..554fde21734 100644 --- a/src/adapterManager.js +++ b/src/adapterManager.js @@ -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) { @@ -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)); @@ -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 @@ -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(); } }); diff --git a/src/auction.js b/src/auction.js index 54621669c59..c3712c0a4df 100644 --- a/src/auction.js +++ b/src/auction.js @@ -62,7 +62,6 @@ import { adUnitsFilter, bind, deepAccess, - flatten, generateUUID, getValue, isEmpty, @@ -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; @@ -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 = []; @@ -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); } } @@ -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') || {}; @@ -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() { @@ -387,7 +382,6 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a addBidReceived, addBidRejected, addNoBid, - executeCallback, callBids, addWinningBid, setBidTargeting, @@ -557,12 +551,6 @@ 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); @@ -570,8 +558,6 @@ export function addBidToAuction(auctionInstance, 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. @@ -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); } @@ -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} 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; -} diff --git a/src/utils.js b/src/utils.js index 2ce4f9cc8bb..5ad8b5031bd 100644 --- a/src/utils.js +++ b/src/utils.js @@ -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 || {}); } diff --git a/test/spec/auctionmanager_spec.js b/test/spec/auctionmanager_spec.js index d8598dc2063..be4a7f819cd 100644 --- a/test/spec/auctionmanager_spec.js +++ b/test/spec/auctionmanager_spec.js @@ -5,7 +5,7 @@ import { adjustBids, getMediaTypeGranularity, getPriceByGranularity, - addBidResponse + addBidResponse, resetAuctionState } from 'src/auction.js'; import CONSTANTS from 'src/constants.json'; import * as auctionModule from 'src/auction.js'; @@ -23,6 +23,7 @@ import {AuctionIndex} from '../../src/auctionIndex.js'; import {expect} from 'chai'; import {deepClone} from '../../src/utils.js'; import { IMAGE as ortbNativeRequest } from 'src/native.js'; +import {PrebidServer} from '../../modules/prebidServerBidAdapter/index.js'; var assert = require('assert'); @@ -48,6 +49,7 @@ function mockBid(opts) { let bidderCode = opts && opts.bidderCode; return { + adUnitCode: opts?.adUnitCode || ADUNIT_CODE, 'ad': 'creative', 'cpm': '1.99', 'width': 300, @@ -68,6 +70,11 @@ function mockBid(opts) { transactionId: this.transactionId, auctionId: this.auctionId } + }, + _ctx: { + adUnits: opts?.adUnits, + src: opts?.src, + uniquePbsTid: opts?.uniquePbsTid, } }; } @@ -96,6 +103,9 @@ function mockBidRequest(bid, opts) { 'bidderCode': bidderCode || bid.bidderCode, 'auctionId': opts && opts.auctionId, 'bidderRequestId': requestId, + src: bid?._ctx?.src, + adUnitsS2SCopy: bid?._ctx?.src === CONSTANTS.S2S.SRC ? bid?._ctx?.adUnits : undefined, + uniquePbsTid: bid?._ctx?.src === CONSTANTS.S2S.SRC ? bid?._ctx?.uniquePbsTid : undefined, 'bids': [ { 'bidder': bidderCode || bid.bidderCode, @@ -108,7 +118,8 @@ function mockBidRequest(bid, opts) { 'bidId': bid.requestId, 'bidderRequestId': requestId, 'auctionId': opts && opts.auctionId, - 'mediaTypes': mediaType + 'mediaTypes': mediaType, + src: bid?._ctx?.src } ], 'auctionStart': 1505250713622, @@ -160,6 +171,7 @@ describe('auctionmanager.js', function () { indexAuctions = []; indexStub = sinon.stub(auctionManager, 'index'); indexStub.get(() => new AuctionIndex(() => indexAuctions)); + resetAuctionState(); }); afterEach(() => { @@ -915,6 +927,11 @@ describe('auctionmanager.js', function () { beforeEach(function () { ajaxStub = sinon.stub(ajaxLib, 'ajaxBuilder').callsFake(mockAjaxBuilder); adUnits = [{ + mediaTypes: { + banner: { + sizes: [] + } + }, code: ADUNIT_CODE, transactionId: ADUNIT_CODE, bids: [ @@ -1150,12 +1167,19 @@ describe('auctionmanager.js', function () { }); describe('when auction timeout is 20', function () { - let eventsEmitSpy; + let eventsEmitSpy, auctionDone; - function setupBids(auctionId) { - bids = [mockBid(), mockBid({ bidderCode: BIDDER_CODE1 })]; - let bidRequests = bids.map(bid => mockBidRequest(bid, {auctionId})); + function respondToRequest(requestIndex) { + server.requests[requestIndex].respond(200, {}, 'response body'); + } + + function runAuction() { + let bidRequests = bids.map(bid => mockBidRequest(bid, {auctionId: auction.getAuctionId()})); makeRequestsStub.returns(bidRequests); + return new Promise((resolve) => { + auctionDone = resolve; + auction.callBids(); + }) } beforeEach(function () { @@ -1164,102 +1188,127 @@ describe('auctionmanager.js', function () { transactionId: ADUNIT_CODE, bids: [ {bidder: BIDDER_CODE, params: {placementId: 'id'}}, + {bidder: BIDDER_CODE1, params: {placementId: 'id'}}, ] }]; adUnitCodes = [ADUNIT_CODE]; eventsEmitSpy = sinon.spy(events, 'emit'); + bids = [mockBid(), mockBid({ bidderCode: BIDDER_CODE1 })]; + const spec1 = mockBidder(BIDDER_CODE, [bids[0]]); + registerBidder(spec1); + const spec2 = mockBidder(BIDDER_CODE1, [bids[1]]); + registerBidder(spec2); + auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: () => auctionDone(), cbTimeout: 20}); }); + afterEach(function () { events.emit.restore(); }); - function respondToRequest(requestIndex) { - server.requests[requestIndex].respond(200, {}, 'response body'); - } - - it('resolves .end on timeout', (done) => { - registerBidder(mockBidder(BIDDER_CODE, [bids[0]])); - registerBidder(mockBidder(BIDDER_CODE1, [bids[1]])); + it('resolves .end on timeout', () => { let endResolved = false; - function callback() { - expect(endResolved).to.be.true; - done() - } - auction = auctionModule.newAuction({adUnits, adUnitCodes, callback, cbTimeout: 20}); - setupBids(auction.getAuctionId()); - auction.callBids(); - respondToRequest(0); auction.end.then(() => { endResolved = true; }) + const pm = runAuction().then(() => { + expect(endResolved).to.be.true; + }); + respondToRequest(0); + return pm; }) - it('should emit BID_TIMEOUT and AUCTION_END for timed out bids', function (done) { - const spec1 = mockBidder(BIDDER_CODE, [bids[0]]); - registerBidder(spec1); - const spec2 = mockBidder(BIDDER_CODE1, [bids[1]]); - registerBidder(spec2); - - function auctionCallback() { + it('should emit BID_TIMEOUT and AUCTION_END for timed out bids', function () { + const pm = runAuction().then(() => { const bidTimeoutCall = eventsEmitSpy.withArgs(CONSTANTS.EVENTS.BID_TIMEOUT).getCalls()[0]; const timedOutBids = bidTimeoutCall.args[1]; assert.equal(timedOutBids.length, 1); assert.equal(timedOutBids[0].bidder, BIDDER_CODE1); // Check that additional properties are available - assert.equal(timedOutBids[0].params.placementId, 'id'); + assert.equal(timedOutBids[0].params[0].placementId, 'id'); const auctionEndCall = eventsEmitSpy.withArgs(CONSTANTS.EVENTS.AUCTION_END).getCalls()[0]; const auctionProps = auctionEndCall.args[1]; assert.equal(auctionProps.adUnits, adUnits); assert.equal(auctionProps.timeout, 20); assert.equal(auctionProps.auctionStatus, AUCTION_COMPLETED) - done(); - } - auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: auctionCallback, cbTimeout: 20}); - setupBids(auction.getAuctionId()); - - auction.callBids(); + }); respondToRequest(0); + return pm; }); - it('should NOT emit BID_TIMEOUT when all bidders responded in time', function (done) { - const spec1 = mockBidder(BIDDER_CODE, [bids[0]]); - registerBidder(spec1); - const spec2 = mockBidder(BIDDER_CODE1, [bids[1]]); - registerBidder(spec2); - - function respondToRequest(requestIndex) { - server.requests[requestIndex].respond(200, {}, 'response body'); - } - function auctionCallback() { + it('should NOT emit BID_TIMEOUT when all bidders responded in time', function () { + const pm = runAuction().then(() => { assert.ok(eventsEmitSpy.withArgs(CONSTANTS.EVENTS.BID_TIMEOUT).notCalled, 'did not emit event BID_TIMEOUT'); - done(); - } - auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: auctionCallback, cbTimeout: 20}); - setupBids(auction.getAuctionId()); - auction.callBids(); + }); respondToRequest(0); respondToRequest(1); + return pm; }); - it('should NOT emit BID_TIMEOUT for bidders which responded in time but with an empty bid', function (done) { - const spec1 = mockBidder(BIDDER_CODE, []); - registerBidder(spec1); - const spec2 = mockBidder(BIDDER_CODE1, []); - registerBidder(spec2); - function auctionCallback() { + it('should NOT emit BID_TIMEOUT for bidders which responded in time but with an empty bid', function () { + const pm = runAuction().then(() => { const bidTimeoutCall = eventsEmitSpy.withArgs(CONSTANTS.EVENTS.BID_TIMEOUT).getCalls()[0]; const timedOutBids = bidTimeoutCall.args[1]; assert.equal(timedOutBids.length, 1); assert.equal(timedOutBids[0].bidder, BIDDER_CODE1); - done(); - } - auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: auctionCallback, cbTimeout: 20}); - setupBids(auction.getAuctionId()); - auction.callBids(); + }); respondToRequest(0); + return pm; }); + + it('should NOT emit BID_TIMEOUT for bidders that replied through S2S', () => { + adapterManager.registerBidAdapter(new PrebidServer(), 'pbs'); + config.setConfig({ + s2sConfig: [{ + accountId: '1', + enabled: true, + defaultVendor: 'appnexus', + bidders: ['mock-s2s-1'], + adapter: 'pbs' + }, { + accountId: '1', + enabled: true, + defaultVendor: 'rubicon', + bidders: ['mock-s2s-2'], + adapter: 'pbs' + }] + }) + adUnits[0].bids.push({bidder: 'mock-s2s-1'}, {bidder: 'mock-s2s-2'}) + const s2sAdUnits = deepClone(adUnits); + bids.unshift( + mockBid({bidderCode: 'mock-s2s-1', src: CONSTANTS.S2S.SRC, adUnits: s2sAdUnits, uniquePbsTid: '1'}), + mockBid({bidderCode: 'mock-s2s-2', src: CONSTANTS.S2S.SRC, adUnits: s2sAdUnits, uniquePbsTid: '2'}) + ); + Object.assign(s2sAdUnits[0], { + mediaTypes: { + banner: { + sizes: [[300, 250], [300, 600]] + } + }, + bids: [ + { + bidder: 'mock-s2s-1', + bid_id: bids[0].requestId + }, + { + bidder: 'mock-s2s-2', + bid_id: bids[1].requestId + } + ] + }) + + const pm = runAuction().then(() => { + const toBids = eventsEmitSpy.withArgs(CONSTANTS.EVENTS.BID_TIMEOUT).getCalls()[0].args[1] + expect(toBids.map(bid => bid.bidder)).to.eql([ + 'mock-s2s-2', + BIDDER_CODE, + BIDDER_CODE1, + ]) + }); + respondToRequest(1); + return pm; + }) }); }); diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index a6164f919ef..14e530c1a9b 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -47,7 +47,6 @@ describe('adpod.js', function () { addBidToAuctionStub = sinon.stub(auction, 'addBidToAuction').callsFake(function (auctionInstance, bid) { auctionBids.push(bid); }); - doCallbacksIfTimedoutStub = sinon.stub(auction, 'doCallbacksIfTimedout'); clock = sinon.useFakeTimers(); config.setConfig({ cache: { @@ -61,7 +60,6 @@ describe('adpod.js', function () { logWarnStub.restore(); logInfoStub.restore(); addBidToAuctionStub.restore(); - doCallbacksIfTimedoutStub.restore(); clock.restore(); config.resetConfig(); auctionBids = []; @@ -633,7 +631,6 @@ describe('adpod.js', function () { callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, videoMT); callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, videoMT); - expect(doCallbacksIfTimedoutStub.calledTwice).to.equal(true); expect(logWarnStub.calledOnce).to.equal(true); expect(auctionBids.length).to.equal(0); });