Downgrade errors to warnings when adapter doesn't support imp format #2385
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SyntaxNode
reviewed
Sep 20, 2022
| @@ -78,19 +78,19 @@ func pruneImps(imps []openrtb2.Imp, allowedTypes parsedSupports) (int, []error) | |||
| for i := 0; i < len(imps); i++ { | |||
| if !allowedTypes.banner && imps[i].Banner != nil { | |||
| imps[i].Banner = nil | |||
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("request.imp[%d] uses banner, but this bidder doesn't support it", i)}) | |||
| errs = append(errs, &errortypes.Warning{Message: fmt.Sprintf("request.imp[%d] uses banner, but this bidder doesn't support it", i)}) | |||
There was a problem hiding this comment.
We may want to add a new warning code for this case where a media type is not supported.
|
@AlexBVolcy was any consideration given to @hhhjort's suggestion on the issue that we may still want an error if there are no adapters able to service a format requested in a request? At a glance, it looks like this would require a refactor of the core bidder loop or could perhaps introduce added complexity that we don't feel is worth it but I figured I should ask. |
|
This was discussed offline. The suggestion to throw an error if there are no adapters able to service a requested format is a behavior change outside of the scope of this PR. |
SyntaxNode
approved these changes
Sep 28, 2022
gwi-mmuths
added a commit
to gwinteractive/prebid-server
that referenced
this pull request
Oct 10, 2022
* origin/master: (316 commits) Fix alt bidder code test broken by imp wrapper commit (prebid#2405) ImpWrapper (prebid#2375) Update sspBC adapter to v5.7 (prebid#2394) Change errors to warnings (prebid#2385) TheMediaGrid: support native mediaType (prebid#2377) Huaweiadsadapter gdpr (prebid#2345) Add alternatebiddercodes in Prebid request ext (prebid#2339) Changed FS bidder names to mixed case (prebid#2390) Load bidders names from file system in lowercase (prebid#2388) consumable adapter: add schain and coppa support (prebid#2361) Adapter aliases: moved adapter related configs to bidder.yaml files (prebid#2353) pubmatic adapter: add param prebiddealpriority (prebid#2281) Yieldlab Adapter: Add support for ad unit sizes (prebid#2364) GDPR: add enforce_algo config flag and convert enforce_purpose to bool (prebid#2330) Resolves CVE-2022-27664 (prebid#2376) AMP Endpoint: support parameters consentType and gdprApplies (prebid#2338) Dianomi - added usync for redirect (prebid#2368) Create issue_prioritization.yml (prebid#2372) Update yaml files for gzip supported bidders (prebid#2365) Adnuntius Adapter: Set no cookies as a parameter (prebid#2352) ...
shunj-nb
pushed a commit
to ParticleMedia/prebid-server
that referenced
this pull request
Nov 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses this issue: #2178
The goal of this PR is: In the situation when an adapter doesn't support the format indicated from the request imp, an error is thrown that we want downgraded to a warning since this error is unnecessary to be written to analytics
This error is thrown from the bidder wrapper
InfoAwareBidder's call tooMakeRequests(). So for all the error checks that check if an adapter doesn't support the imp format (i.e.{&errortypes.BadInput{Message: "this bidder does not support site requests"}), we downgrade those to a warning (i.e.{&errortypes.Warning{Message: "this bidder does not support site requests"})Tests were updated with this change.