Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prebid 8: remove NO_BID bids #9902

Merged
merged 6 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions modules/currency.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ function wrapFunction(fn, context, params) {
bid.currency = adServerCurrency;
}
} catch (e) {
logWarn('Returning NO_BID, getCurrencyConversion threw error: ', e);
// TODO: in v8, this should not continue with a "NO_BID"
params[1] = params[2](CONSTANTS.REJECTION_REASON.CANNOT_CONVERT_CURRENCY);
logWarn('getCurrencyConversion threw error: ', e);
params[2](CONSTANTS.REJECTION_REASON.CANNOT_CONVERT_CURRENCY);
return;
}
}
return fn.apply(context, params);
Expand Down
3 changes: 1 addition & 2 deletions modules/prebidServerBidAdapter/ortbConverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ const PBS_CONVERTER = ortbConverter({

// because core has special treatment for PBS adapter responses, we need some additional processing
bidResponse.requestTimestamp = context.requestTimestamp;
const status = bid.price !== 0 ? CONSTANTS.STATUS.GOOD : CONSTANTS.STATUS.NO_BID;
return {
bid: Object.assign(createBid(status, {
bid: Object.assign(createBid(CONSTANTS.STATUS.GOOD, {
src: CONSTANTS.S2S.SRC,
bidId: bidRequest ? (bidRequest.bidId || bidRequest.bid_Id) : null,
transactionId: context.adUnit.transactionId,
Expand Down
7 changes: 3 additions & 4 deletions modules/priceFloors.js
Original file line number Diff line number Diff line change
Expand Up @@ -745,10 +745,9 @@ export const addBidResponseHook = timedBidResponseHook('priceFloors', function a
// now do the compare!
if (shouldFloorBid(floorData, floorInfo, bid)) {
// bid fails floor -> throw it out
// continue with a "NO_BID" bid, TODO: remove this in v8
const flooredBid = reject(CONSTANTS.REJECTION_REASON.FLOOR_NOT_MET);
logWarn(`${MODULE_NAME}: ${flooredBid.bidderCode}'s Bid Response for ${adUnitCode} was rejected due to floor not met (adjusted cpm: ${bid?.floorData?.cpmAfterAdjustments}, floor: ${floorInfo?.matchingFloor})`, bid);
return fn.call(this, adUnitCode, flooredBid, reject);
reject(CONSTANTS.REJECTION_REASON.FLOOR_NOT_MET);
logWarn(`${MODULE_NAME}: ${bid.bidderCode}'s Bid Response for ${adUnitCode} was rejected due to floor not met (adjusted cpm: ${bid?.floorData?.cpmAfterAdjustments}, floor: ${floorInfo?.matchingFloor})`, bid);
return;
}
return fn.call(this, adUnitCode, bid, reject);
});
Expand Down
4 changes: 0 additions & 4 deletions modules/pubmaticAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ function setBidStatus(bid, args) {
bid.status = SUCCESS;
delete bid.error; // it's possible for this to be set by a previous timeout
break;
case CONSTANTS.STATUS.NO_BID:
bid.status = NO_BID;
delete bid.error;
break;
default:
bid.status = ERROR;
bid.error = {
Expand Down
24 changes: 4 additions & 20 deletions src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ import adapterManager from './adapterManager.js';
import CONSTANTS from './constants.json';
import {GreedyPromise} from './utils/promise.js';
import {useMetrics} from './utils/perfMetrics.js';
import {createBid} from './bidfactory.js';
import {adjustCpm} from './utils/cpm.js';
import {getGlobal} from './prebidGlobal.js';

Expand Down Expand Up @@ -494,26 +493,11 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM

function rejectBidResponse(adUnitCode, bid, reason) {
return handleBidResponse(adUnitCode, bid, (done) => {
// return a "NO_BID" replacement that the caller can decide to continue with
// TODO: remove this in v8; see https://github.com/prebid/Prebid.js/issues/8956
const noBid = createBid(CONSTANTS.STATUS.NO_BID, bid.getIdentifiers?.());
Object.assign(noBid, Object.fromEntries(Object.entries(bid).filter(([k]) => !noBid.hasOwnProperty(k) && ![
'ad',
'adUrl',
'vastXml',
'vastUrl',
'native',
].includes(k))));
noBid.status = CONSTANTS.BID_STATUS.BID_REJECTED;
noBid.cpm = 0;

bid.rejectionReason = reason;
logWarn(`Bid from ${bid.bidder || 'unknown bidder'} was rejected: ${reason}`, bid)
events.emit(CONSTANTS.EVENTS.BID_REJECTED, bid);
auctionInstance.addBidRejected(bid);
done();

return noBid;
})
}

Expand Down Expand Up @@ -552,12 +536,12 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM
waitFor((bidderRequest && bidderRequest.bidderRequestId) || '', addBidResponse.call({
dispatch: acceptBidResponse,
}, adUnitCode, bid, (() => {
let rejection;
let rejected = false;
return (reason) => {
if (rejection == null) {
rejection = rejectBidResponse(adUnitCode, bid, reason);
if (!rejected) {
rejectBidResponse(adUnitCode, bid, reason);
rejected = true;
}
return rejection;
}
})()));
}
Expand Down
3 changes: 1 addition & 2 deletions src/constants.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
},
"DEBUG_MODE": "pbjs_debug",
"STATUS": {
"GOOD": 1,
"NO_BID": 2
"GOOD": 1
},
"CB": {
"TYPE": {
Expand Down
22 changes: 0 additions & 22 deletions test/spec/auctionmanager_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1788,28 +1788,6 @@ describe('auctionmanager.js', function () {
})
});

it('should return a NO_BID replacement', () => {
const noBid = cbs.addBidResponse.reject(AU_CODE, {...bid, statusMessage: 'Bid available', status: CONSTANTS.BID_STATUS.RENDERED}, 'Rejected');
sinon.assert.match(noBid, {
status: CONSTANTS.BID_STATUS.BID_REJECTED,
statusMessage: 'Bid returned empty or error response',
cpm: 0,
requestId: bid.requestId,
auctionId: bid.auctionId,
adUnitCode: AU_CODE,
rejectionReason: undefined,
});
});

it('should return NO_BID replacement when rejected bid is not a "proper" bid', () => {
const noBid = cbs.addBidResponse.reject(AU_CODE, {});
sinon.assert.match(noBid, {
status: CONSTANTS.BID_STATUS.BID_REJECTED,
statusMessage: 'Bid returned empty or error response',
cpm: 0,
});
})

it('addBidResponse hooks should not be able to reject the same bid twice', () => {
cbs.addBidResponse(AU_CODE, bid);
expect(auction.addBidRejected.calledOnce).to.be.true;
Expand Down
24 changes: 12 additions & 12 deletions test/spec/modules/currency_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,15 +366,15 @@ describe('currency', function () {
expect(innerBid.cpm).to.equal(1);
});

it('should result in NO_BID when currency support is not enabled and fromCurrency is not USD', function () {
it('should reject bid when currency support is not enabled and fromCurrency is not USD', function () {
setConfig({});

var bid = makeBid({ 'cpm': 1, 'currency': 'GBP' });
var innerBid;
let bidAdded = false;
addBidResponseHook(function(adCodeId, bid) {
innerBid = bid;
bidAdded = true;
}, 'elementId', bid, reject);
expect(innerBid.status).to.equal('rejected');
expect(bidAdded).to.be.false;
expect(reject.calledOnce).to.be.true;
});

Expand All @@ -390,32 +390,32 @@ describe('currency', function () {
expect(bid).to.equal(innerBid);
});

it('should result in NO_BID when fromCurrency is not supported in file', function () {
it('should reject bid when fromCurrency is not supported in file', function () {
// RESET to request currency file
setConfig({ 'adServerCurrency': undefined });

fakeCurrencyFileServer.respondWith(JSON.stringify(getCurrencyRates()));
setConfig({ 'adServerCurrency': 'JPY' });
fakeCurrencyFileServer.respond();
var bid = makeBid({ 'cpm': 1, 'currency': 'ABC' });
var innerBid;
let bidAdded = false;
addBidResponseHook(function(adCodeId, bid) {
innerBid = bid;
bidAdded = true;
}, 'elementId', bid, reject);
expect(innerBid.status).to.equal('rejected');
expect(bidAdded).to.be.false;
expect(reject.calledOnce).to.be.true;
});

it('should result in NO_BID when adServerCurrency is not supported in file', function () {
it('should reject bid when adServerCurrency is not supported in file', function () {
fakeCurrencyFileServer.respondWith(JSON.stringify(getCurrencyRates()));
setConfig({ 'adServerCurrency': 'ABC' });
fakeCurrencyFileServer.respond();
var bid = makeBid({ 'cpm': 1, 'currency': 'GBP' });
var innerBid;
let bidAdded = false;
addBidResponseHook(function(adCodeId, bid) {
innerBid = bid;
bidAdded = true;
}, 'elementId', bid, reject);
expect(innerBid.status).to.equal('rejected');
expect(bidAdded).to.be.false;
expect(reject.calledOnce).to.be.true;
});

Expand Down
2 changes: 1 addition & 1 deletion test/spec/modules/invisiblyAnalyticsAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('Invisibly Analytics Adapter test suite', function () {
hb_source: 'server',
},
getStatusCode() {
return CONSTANTS.STATUS.NO_BID;
return CONSTANTS.STATUS.GOOD;
},
};

Expand Down
2 changes: 1 addition & 1 deletion test/spec/modules/livewrappedAnalyticsAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const BID3 = {
auctionId: '25c6d7f5-699a-4bfc-87c9-996f915341fa',
mediaType: 'banner',
getStatusCode() {
return CONSTANTS.STATUS.NO_BID;
return CONSTANTS.STATUS.GOOD;
}
};

Expand Down
4 changes: 2 additions & 2 deletions test/spec/modules/priceFloors_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1835,7 +1835,7 @@ describe('the price floors module', function () {
transactionId: 'au',
};
beforeEach(function () {
returnedBidResponse = {};
returnedBidResponse = null;
reject = sinon.stub().returns({status: 'rejected'});
indexStub = sinon.stub(auctionManager, 'index');
indexStub.get(() => stubAuctionIndex({adUnits: [adUnit]}));
Expand Down Expand Up @@ -1867,7 +1867,7 @@ describe('the price floors module', function () {
_floorDataForAuction[AUCTION_ID].data.values = { 'banner': 1.0 };
runBidResponse();
expect(reject.calledOnce).to.be.true;
expect(returnedBidResponse.status).to.equal('rejected');
expect(returnedBidResponse).to.not.exist;
});
it('if it finds a rule and does not floor should update the bid accordingly', function () {
_floorDataForAuction[AUCTION_ID] = utils.deepClone(basicFloorConfig);
Expand Down
6 changes: 0 additions & 6 deletions test/spec/unit/core/targeting_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,6 @@ describe('targeting tests', function () {
bidExpiryStub.restore();
});

it('should filter out NO_BID bids', () => {
bidsReceived = [mkBid(sampleBid, CONSTANTS.STATUS.NO_BID)];
const tg = targetingInstance.getAllTargeting();
expect(tg[bidsReceived[0].adUnitCode]).to.eql({});
});

describe('when handling different adunit targeting value types', function () {
const adUnitCode = '/123456/header-bid-tag-0';
const adServerTargeting = {};
Expand Down