Prebid.js: IX error after update

[Hamper]

Type of issue

bug

Description

Request error if banner.sizes is big, but only one size used for bidder params. I think problem is new missing sizes logic from ix module.

Steps to reproduce

Add many sizes to mediaTypes.banner.sizes, add ix bidder settings with one size like [300, 250]

Test page

https://jsfiddle.net/gv19koqt

Expected results

Valid ix request with imps only for sizes used as params.size in bidder settings

Actual results

Bad Request because all sizes in request

Platform details

Prebid.js 4.4.0

Other information

[bretg]

Adding @ix-prebid-development to take a look

[muuki88]

We discovered this bug as well. It was introduced in #5436 ( 41f77997854ba86309a8cf2009bffb146e346308 ), which is released since 3.26.0.

The root cause is that the request URL gets too long as the complete payload is send as a encoded query parameter. This could be fixed by using POST and sending the payload as json body.

Our workaround is to define the same ad unit multiple times only with the ix sizes. This is a hack we learned from Xandr, which feels wrong, but works:

adUnit: {
  code: 'ad-1',
  mediaTypes: { banner: [ /* regular sizes */ ] }
 bids: [ /* all bid adapters that can handle more sizes */ ]
},
// only added if sizes should be requested
adUnit: {
  code: 'ad-1',
  mediaTypes: { banner: [ [ 300, 250 ]] }
  bids: [ { bidderCode: 'ix', params: { siteId: '...', size: [300, 250] }} ]
} 

[Hamper]

Is server support post requests? Can I just change method to 'POST' in buildRequest as workaround?

[patmmccann]

@muuki88 the workaround seems to remove the need for the missingSizes part of the PR. Do you envision the workaround as temporary?

[muuki88]

Do you envision the workaround as temporary?

Yes. This doesn't seem correct to define a thing that should be unique (an ad slot) multiple times.

The IX adapter has caused quite some pain over the last 2 years.

• Not setting a bidder alias when the bidderCode was changed
• requests per size in the initial versions
• warnings if "sizes don't appear"
• a bid object per size where each sizes needs to be specified again
• and now this mess with the long GET requests

We have integrated ~10 bidders. And IX is by far the most needlessly complicated and buggy implementation :disappointed:
PubMatic improved signifactly. They were on par at some point :stuck_out_tongue:

So in envision an IX bid adapter that can be configured like most other adapters. Without the need to specify a size as the sizes are in der mediaType.banner.sizes array. A plain POST request with a readable json format. And no warnings for things that are not wrong. I always see the Xandr adapter as a gold standard. This thing just works. There are others that are as simple implemented as Xandr as well.

[kizzard]

@Hamper @muuki88 Thank you for reporting this issue. We're evaluating it internally and will provide updates as soon as possible.

[muuki88]

Thanks @kizzard I'm sorry if my word choice was too harsh. In general we are happy with the
cooperation and the revenues :kissing_heart:

[kizzard]

@muuki88 @Hamper @patmmccann thanks for bearing with us on this. Can you share the date you first noticed the problem, and if it was noticed in production or caught in testing first? This will help us with the required impact analysis. Thank you!

[muuki88]

Can you share the date you first noticed the problem, and if it was noticed in production or caught in testing first

As mentioned before

We discovered this bug as well. It was introduced in #5436 ( 41f7799 ), which is released since 3.26.0.

To clarify. We caught this in production, because revenues dropped by 80% for index.

I cannot give the exact number how much ad slots / sizes are required until the bug appears, but it's not that much. I guess 8 ad slots with each 4 sizes should be enough.

So there's no specific date as we rolled back the prebid, fixed the sizes and then rolled forward.

[patmmccann]

@muuki88 @Hamper @patmmccann thanks for bearing with us on this. Can you share the date you first noticed the problem, and if it was noticed in production or caught in testing first? This will help us with the required impact analysis. Thank you!

@kizzard we're catching this in upgrade tests. It seems the solution might be to limit the number of iterations of this for loop https://github.com/ix-prebid-support/Prebid.js/blob/d93792d32845bedc32664ff72310307ad3c4ad72/modules/ixBidAdapter.js#L524

[muuki88]

It seems the solution might be to limit the number of iterations of this for loop

I wouldn't do that. With more ad slots this error still happens.

To be honest I don't quite understand the logic of adding missing sizes, requiring each size to be configure and log warnings if a size is missing. In my naive view a fix could look like this

1. Use POST instead of GET for ad requests (no caching issues, no query size length, easier to inspect)
2. Use the mediaTypes.banner.sizes array for sizes. This array should already be filtered with the proper sizes

A bid configuration could then look like this

const adUnits = [{
    code: 'banner-div-a',
    mediaTypes: {
        banner: {
            sizes: [ [300, 250], [300, 600] ]
        }
    },
    bids: [{
        bidder: 'ix',
        params: { siteId: '123456' }
    }]
}];

From a dev perspective this is what I want the configuration to look like. Prebid takes care of most of the filtering, etc.

[sulleh]

Hi all - mikeosullivan from IX here. Thanks for all the feedback!

First off, on this issue: For context, the reason we added support for detecting eligible sizes was to increase publisher monetization, and not rely on an IX-specific configuration for sizes, which can often fall out of date.

Unfortunately, we are hitting some character limits for publishers who have a high number of adunits + sizes available on the page. It's definitely unacceptable to break network requests and impact ad spend, so this is a top issue within the team. IX engineering is evaluating different solutions, and we'll have an update on the planned fix early next week.

Secondly, thanks for the feedback on adapter design - specifically, @muuki88 I've shared your feedback with the team. This is an area we're looking to address in the near future, so your comments are very timely. Thanks again, and more updates to come on Monday.

[sulleh]

Hi there -

A quick update here. The team is working on a fix now, and should be pushed in the next two weeks. The approach will be to monitor the number of characters in a request, and if the request hits a certain length, the adapter will "split" the calls and generate a second call. This should avoid broken requests from browser, CDN, or endpoint character limits.

We feel like having the conditional request approach strikes a nice balance between ensuring we aren't dropping revenue, but limiting outbound network requests to what is needed for a given publisher.

When the new version is live, @muuki88 would it be possible to share a URL (DM or email is also fine!) with the updated adapter code, so we can verify the issue is resolved? It would also be helpful to get the date when you first saw the issue (so URL that demonstrated the issue, and the day you saw it.)

This allows us to answer a question - whether requests that encounter this bug are successfully making network requests, with incomplete data, or whether the outbound request is truly broken from the client side and our endpoint never sees the request.

Thanks and let me know if you have any questions!
mike

[muuki88]

Hi @sulleh

Thanks for the update and for working on a fix :smiling_face_with_three_hearts:

Before I start I want to acknowledge that I have no knowledge about your infrastructure, organizational and resource constraints. So what might seems "easy" is in fact pretty hard. I still try to give a constructive feedback. I'm also open for direct testing. I'll send you a mail (the public one in your account)

IMHO the suggested fix is a band aid, which helps to improve the current situation. I have used this technique as well, due to constraints on our end. Still it woud be nice to see this fixed in a proper way, which involves less heuristics and workarounds due to fundamental implementation issues. The proposed solutions does fix the GET limit, but it doesn't solve

• the configuration overhead (a bid entry for each size). To be fair, this is another topic.
• the automatic adding of sizes (which could be solved by using the prebid banner sizes)
• debuggability - it is even worse. Now I need to take look at multiple GET requests that have json encoded query param
• more requests are being sent now

I'm totally fine with testing and using this in the near future, but would hope for a less complex and more robust implementation when possible :smile:

Update

It would also be nice to turn off the "add missing sizes" feature as implemented at the moment. This would allow us to remove our workaround were we define duplicate ad units just for index exchange to remove the sizes it shouldn't send over the wire.

[sulleh]

Hey there @muuki88 - no problem at all, we appreciate the feedback! I agree that this fix addresses the direct problem, and doesn't solve some of the larger issues you are encountering.

We do want to dig into solving these as well, with a broader update in the future that solves some / all of these problems. This is something the team is going to look into over code freeze / Q4, with a goal of improvements shipping in Q1. Our goal is to be the easiest, most enjoyable partner for you to implement. :)

[ChrisHuie]

Related to #5856

[ChrisHuie]

closing with merge of #5856