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

Bad Input Error if pbjs s2s config contains alias configuration for a disabled adapter #3592

Open
muuki88 opened this issue Dec 4, 2024 · 8 comments · May be fixed by zxPhoenix/prebid-server-java#1 or #3650

Comments

@muuki88
Copy link
Contributor

muuki88 commented Dec 4, 2024

Deploying alias configurations for prebid server through prebid.js is dangerous, because if a bidder is being disabled, it can stop the entire monetization of prebid server. Disabled bidders will stop the entire bid requests with a 400 if the alias configuration contains a disabled bidder.

🪄 Steps to reproduce

Prebid.js s2s config

    s2sConfig: [
      // Server side only configuration
      {
        accountId: accountId,
        adapter: 'prebidServer',
        endpoint: { p1Consent: 'https://prebid-server/openrtb2/auction' },
        syncEndpoint: { p1Consent: 'https://prebid-server/cookie_sync' },
        bidders: [ 'bidderA', 'bidderB' ],
        extPrebid: {
          aliases: {
            'bidder_c_alias': 'bidder_c' // note that this bidder isn't even in the bidders array
          }
        }
      }
    ],

and send a request to a prebid server where BidderC is not enabled.

This will cause a 400 with

Invalid request format: request.ext.prebid.aliases.bidder_c_alias refers to disabled bidder: bidder_c

💡 Proposal

We turn the 400 into an error metric. Related to #3209

Related

If the bidder being alias is unknown also a 400 is being returned:

Invalid request format: request.ext.prebid.aliases.bidder_c_alias refers to unknown bidder: bidder_c

which I guess is okaish, but a metric would also be better.

@bretg
Copy link
Collaborator

bretg commented Dec 13, 2024

I guess I had hoped that this sort of situation would have been addressed by prebid/prebid-server#3735, but that issue was more about imp.ext, and didn't deal with aliases clearly enough.

So yes, I'm supportive of changing the disabled bidder behavior in general. Fail-Fast has shown itself to be a little too draconian in the edge cases.

Proposal A: Never hard fail on an unknown bidder. Just warn and log a metric.

This would solve this problem and probably other edge-cases.

Proposal B: Never hard fail on a bidder that exists but is not enabled. Just warn and log a metric.

Does PBS even have this info?

@bretg bretg moved this from Triage to Community Review in Prebid Server Prioritization Dec 13, 2024
zxPhoenix added a commit to zxPhoenix/prebid-server-java that referenced this issue Dec 23, 2024
zxPhoenix added a commit to zxPhoenix/prebid-server-java that referenced this issue Dec 24, 2024
@pclinger
Copy link

pclinger commented Jan 7, 2025

I would like to add my support to getting this prioritized and resolved. Thank you @zxPhoenix for putting together a PR to address the problem.

@bretg
Copy link
Collaborator

bretg commented Jan 8, 2025

@muuki88 , @pclinger - any input about which of these proposals is your preferred solution?

Proposal A: Never hard fail on an unknown bidder, whether alias or no. Just warn and log a metric.

Proposal B: Never hard fail on a bidder that exists but is not enabled, whether alias or no. Just warn and log a metric.

@bretg
Copy link
Collaborator

bretg commented Jan 10, 2025

Discussed in committee. No resolution yet.

We could have a config which opts into a warning mode rather than hard failing.

@muuki88
Copy link
Contributor Author

muuki88 commented Jan 11, 2025

Sorry, I wasn't available last PMC @bretg

What's the argument to keep this a failing behaviour? I'm curious why hard failing is a good default behaviour 😅

I can see the point of a strict mode, like some frameworks or languages have. It's nice for test environments.

@bretg
Copy link
Collaborator

bretg commented Jan 13, 2025

What's the argument to keep this a failing behaviour?

The idea behind hard-failing has always been that publishers are more likely to notice a hard fail when they test it in their QA environment. They test, it obviously fails, they fix, life is good. Gentle warnings are more likely to be ignored and drag on for months until someone looks at the reports.

This has been a bedrock philosophy of Prebid Server since 2017, and while many of us have recognized it's got downsides, it may take some time to update the people and the software. :-)

One thing we discussed last week was that once a bidder is disabled, it's currently invisible to PBS-core, at least in Java. We would need to create list of disabled bidders in order to treat them differently from biddercodes that never existed in the first place. That seems unnecessary to some.

To move it forward, I'd propose a new config:

settings:
    fail-on-unknown-bidders: false   // defaults to true at least until a major release.

@muuki88
Copy link
Contributor Author

muuki88 commented Jan 13, 2025

I'll take the setting 😄

The idea behind hard-failing has always been that publishers are more likely to notice a hard fail when they test it in their QA environment

Quote from the internet: "everybody has a test system. Some have the luxury of a production system" 😂

In general I agree with the approach. Sometimes it's annoying, but it makes sense 😁

@SyntaxNode
Copy link

Prebid Server's "fail fast, fail loud" strategy for error handling is intended to assist publishers with identifying problems with their bidder configuration. Errors which halt the auction are highly visible in metrics and are hard to miss. Conversely, warnings in the response body necessitate the use of analytics services to parse and raise alerts, demanding considerably more effort from publishers to safeguard themselves.

What is referred to as "dangerous" in this thread is a protection for publishers from defining an invalid configuration. Why would a publisher define a server side only configuration referring to bidders which do not exist, or are disabled, on the server side?

In recent years, Prebid Server has converted many fatal errors to harmless warnings, and I'm curious for feedback from the publishers in the Prebid community on how this approach is working. Based on the lack of resolution at the last meeting, it would appear opinions are divided.

Prebid Server will operate just fine ignoring unknown or disabled bidders, and does not need to be opinionated in undesirable ways. I support an account-level configuration to provide publishers with the option of choosing their own behavior, and hosts are welcome to set their own host-level configuration as they desire. Let's leave the default matching the current "fail fast, fail loud" behavior for minimal disruption.

For PBS-Go, we can provide both a fail-on-unknown-bidders and fail-on-disabled-bidders settings for fine tuning. For PBS-Java, we can provide only the fail-on-unknown-bidders for now, with the option to include disabled bidders in the future if there is demand.

Publishers often involve their Prebid Server host in debugging, so it makes sense for PBS to track account-level metrics for the count of unknown and disabled bidders, along with sampled logs containing specific bidder names with the account id. Although account-level cardinality can be a concern, especially for time series data, tracking just one or two metrics in this scenario should not pose a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment