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

Index Exchange adapter refresh bug #396

Closed
jeff-crosby opened this issue Jun 8, 2016 · 6 comments · Fixed by #410
Closed

Index Exchange adapter refresh bug #396

jeff-crosby opened this issue Jun 8, 2016 · 6 comments · Fixed by #410
Assignees
Labels

Comments

@jeff-crosby
Copy link

Members of our team discovered a significant bug in the IndexExchange adapter that centers around accurately handling new bid data when refreshing the ads on the page.

The bug causes IndexExchange bids to be "sticky." That is, if Index Exchange bids $5 for the first ad call, then $1 for every future ad call, the $5 bid is still active for all the later auctions.

Thus, if Index bids $1 and another bidder like OpenX bids $3, Index could still win with the "sticky" $5 bid.

We actually resolved the bug with the below diff, but we certainly hope a fix like below can be included in the core prebid project.

    var IndexExchangeAdapter = function IndexExchangeAdapter() {
      var slotIdMap = {};
      var requiredParams = [
      /* 0 */
      'id',
      /* 1 */
      'siteID'];
      var firstAdUnitCode = '';

      function _callBids(request) {
        var bidArr = request.bids;

        if (!utils.hasValidBidRequest(bidArr[0].params, requiredParams, ADAPTER_NAME)) {
          return;
        }

        //Our standard is to always bid for all known slots.
        cygnus_index_args.slots = [];

        var expectedBids = 0;

        //Grab the slot level data for cygnus_index_args
        for (var i = 0; i < bidArr.length; i++) {
          var bid = bidArr[i];
          var sizeID = 0;

          expectedBids++;

          // Expecting nested arrays for sizes
          if (!(bid.sizes[0] instanceof Array)) {
            bid.sizes = [bid.sizes];
          }

          // Create index slots for all bids and sizes
          for (var j = 0; j < bid.sizes.length; j++) {
            var validSize = false;
            for (var k = 0; k < cygnus_index_adunits.length; k++) {
              if (bid.sizes[j][0] === cygnus_index_adunits[k][0] && bid.sizes[j][1] === cygnus_index_adunits[k][1]) {
                validSize = true;
                break;
              }
            }

            if (!validSize) {
              continue;
            }

            if (bid.params.timeout && typeof cygnus_index_args.timeout === 'undefined') {
              cygnus_index_args.timeout = bid.params.timeout;
            }

            var siteID = Number(bid.params.siteID);
            if (!siteID) {
              continue;
            }
            if (siteID && typeof cygnus_index_args.siteID === 'undefined') {
              cygnus_index_args.siteID = siteID;
            }

            if (utils.hasValidBidRequest(bid.params, requiredParams, ADAPTER_NAME)) {
              firstAdUnitCode = bid.placementCode;
              var slotID = bid.params[requiredParams[0]];
              slotIdMap[slotID] = bid;

              sizeID++;
              var size = {
                width: bid.sizes[j][0],
                height: bid.sizes[j][1]
              };

              var slotName = slotID + '_' + sizeID;

              //Doesn't need the if(primary_request) conditional since we are using the mergeSlotInto function which is safe
              cygnus_index_args.slots = mergeSlotInto({
                id: slotName,
                width: size.width,
                height: size.height,
                siteID: siteID || cygnus_index_args.siteID
              }, cygnus_index_args.slots);

              if (bid.params.tier2SiteID) {
                var tier2SiteID = Number(bid.params.tier2SiteID);
                if (typeof tier2SiteID !== 'undefined' && !tier2SiteID) {
                  continue;
                }

                cygnus_index_args.slots = mergeSlotInto({
                  id: 'T1_' + slotName,
                  width: size.width,
                  height: size.height,
                  siteID: tier2SiteID
                }, cygnus_index_args.slots);
              }

              if (bid.params.tier3SiteID) {
                var tier3SiteID = Number(bid.params.tier3SiteID);
                if (typeof tier3SiteID !== 'undefined' && !tier3SiteID) {
                  continue;
                }

                cygnus_index_args.slots = mergeSlotInto({
                  id: 'T2_' + slotName,
                  width: size.width,
                  height: size.height,
                  siteID: tier3SiteID
                }, cygnus_index_args.slots);
              }
            }
          }
        }

        if (cygnus_index_args.slots.length > 20) {
          utils.logError('Too many unique sizes on slots, will use the first 20.', ADAPTER_NAME);
        }

        bidmanager.setExpectedBidsCount(ADAPTER_CODE, expectedBids);
        adloader.loadScript(cygnus_index_start());

        var responded = false;

        // Handle response
        window.cygnus_index_ready_state = function () {
          if (responded) {
            return;
          }
          responded = true;

          try {
            var indexObj = _IndexRequestData.targetIDToBid;
            var lookupObj = cygnus_index_args;

            // Grab all the bids for each slot
            for (var adSlotId in slotIdMap) {
              var bidObj = slotIdMap[adSlotId];
              var adUnitCode = bidObj.placementCode;

              var bids = [];

              // Grab the bid for current slot
              for (var cpmAndSlotId in indexObj) {
                var match = /(T\d_)?(.+)_(\d+)_(\d+)/.exec(cpmAndSlotId);
                var tier = match[1] || '';
                var slotID = match[2];
                var sizeID = match[3];
                var currentCPM = match[4];

                var slotName = slotID + '_' + sizeID;
                var slotObj = getSlotObj(cygnus_index_args, tier + slotName);

                // Bid is for the current slot
                if (slotID === adSlotId) {
                  var bid = bidfactory.createBid(1);
                  bid.cpm = currentCPM / 100;
                  bid.ad = indexObj[cpmAndSlotId][0];
                  bid.ad_id = adSlotId;
                  bid.bidderCode = ADAPTER_CODE;
                  bid.width = slotObj.width;
                  bid.height = slotObj.height;
                  bid.siteID = slotObj.siteID;

                  bids.push(bid);
                }
              }

              var currentBid = undefined;

              //Pick the highest bidding price for this slot
              if (bids.length > 0) {
                // Choose the highest bid
                for (var i = 0; i < bids.length; i++) {
                  var bid = bids[i];
                  if (typeof currentBid === 'undefined') {
                    currentBid = bid;
                    continue;
                  }

                  if (bid.cpm > currentBid.cpm) {
                    currentBid = bid;
                  }
                }

                // No bids for expected bid, pass bid
              } else {
                  var bid = bidfactory.createBid(2);
                  bid.bidderCode = ADAPTER_CODE;
                  currentBid = bid;
                }

              bidmanager.addBidResponse(adUnitCode, currentBid);
            }
          } catch (e) {
            utils.logError('Error calling index adapter', ADAPTER_NAME, e);
            logErrorBidResponse();
 -        }
 +        } finally {
 +          responded = true;
 +          cygnus_index_parse_res = function cygnus_index_parse_res() {};
 +          window.cygnus_index_args = {};
 +          window.cygnus_index_args.slots = [];
 +          _IndexRequestData = {};
 +          _IndexRequestData.impIDToSlotID = {};
 +          _IndexRequestData.reqOptions = {};
 +        }
@protonate
Copy link
Collaborator

cc: @indexexchange @headerbid

@indexexchange
Copy link
Contributor

indexexchange commented Jun 8, 2016

Thanks for your feedback.

There is additional javascript logic that is returned alongside with the demand that ensures that this doesn't happen in production; in general, some of those variables should be persisted across refreshes.

A fix for this issue will be submitted ASAP.

@mkendall07
Copy link
Member

@indexexchange
Any update on this? Thanks!

@mkendall07 mkendall07 added the bug label Jun 13, 2016
@indexexchange
Copy link
Contributor

We are still running final QA on the proposed changes.

On Mon, Jun 13, 2016 at 4:07 PM, Matt Kendall notifications@github.com
wrote:

@indexexchange https://github.com/indexexchange
Any update on this? Thanks!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#396 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AQzvq3uGnNybZURAbkvSy55aiQdg6hfPks5qLbhkgaJpZM4Iw9Ll
.

@indexexchange
Copy link
Contributor

@mkendall07 changes committed, PR was made.

@mkendall07
Copy link
Member

thanks @indexexchange

@jeff-crosby
I've tested the updated indexAdapter, however, I only receive a static bid price so I can't confirm if this resolves the issue. Can you please test it quickly to see if it solves the problem on your end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants