Prebid 8: remove NO_BID bids (#9902)

* remove NO_BID bids: currency * remove NO_BID bids: priceFloors * remove NO_BID bids: PBS * remove NO_BID status * fix lint
prebid · Jun 5, 2023 · bfc6dad · bfc6dad
1 parent dd6c174
commit bfc6dad
Show file tree

Hide file tree

Showing 12 changed files with 28 additions and 79 deletions.
diff --git a/modules/currency.js b/modules/currency.js
@@ -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);

diff --git a/modules/prebidServerBidAdapter/ortbConverter.js b/modules/prebidServerBidAdapter/ortbConverter.js
@@ -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,

diff --git a/modules/priceFloors.js b/modules/priceFloors.js
@@ -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);
 });

diff --git a/modules/pubmaticAnalyticsAdapter.js b/modules/pubmaticAnalyticsAdapter.js
@@ -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 = {

diff --git a/src/auction.js b/src/auction.js
@@ -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';
 
@@ -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;
     })
   }
 
@@ -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;
           }
         })()));
       }

diff --git a/src/constants.json b/src/constants.json
@@ -11,8 +11,7 @@
   },
   "DEBUG_MODE": "pbjs_debug",
   "STATUS": {
-    "GOOD": 1,
-    "NO_BID": 2
+    "GOOD": 1
   },
   "CB": {
     "TYPE": {

diff --git a/test/spec/auctionmanager_spec.js b/test/spec/auctionmanager_spec.js
@@ -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;

diff --git a/test/spec/modules/currency_spec.js b/test/spec/modules/currency_spec.js
@@ -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;
     });
 
@@ -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;
     });
 

diff --git a/test/spec/modules/invisiblyAnalyticsAdapter_spec.js b/test/spec/modules/invisiblyAnalyticsAdapter_spec.js
@@ -53,7 +53,7 @@ describe('Invisibly Analytics Adapter test suite', function () {
       hb_source: 'server',
     },
     getStatusCode() {
-      return CONSTANTS.STATUS.NO_BID;
+      return CONSTANTS.STATUS.GOOD;
     },
   };
 

diff --git a/test/spec/modules/livewrappedAnalyticsAdapter_spec.js b/test/spec/modules/livewrappedAnalyticsAdapter_spec.js
@@ -71,7 +71,7 @@ const BID3 = {
   auctionId: '25c6d7f5-699a-4bfc-87c9-996f915341fa',
   mediaType: 'banner',
   getStatusCode() {
-    return CONSTANTS.STATUS.NO_BID;
+    return CONSTANTS.STATUS.GOOD;
   }
 };
 

diff --git a/test/spec/modules/priceFloors_spec.js b/test/spec/modules/priceFloors_spec.js
@@ -1817,7 +1817,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]}));
@@ -1849,7 +1849,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);

diff --git a/test/spec/unit/core/targeting_spec.js b/test/spec/unit/core/targeting_spec.js
@@ -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 = {};