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

Kargo Bid Adapter: meta.advertiserDomains passing incorrectly #7909

Closed
robertrmartinez opened this issue Jan 5, 2022 · 2 comments · Fixed by #7910
Closed

Kargo Bid Adapter: meta.advertiserDomains passing incorrectly #7909

robertrmartinez opened this issue Jan 5, 2022 · 2 comments · Fixed by #7910

Comments

@robertrmartinez
Copy link
Collaborator

robertrmartinez commented Jan 5, 2022

Type of issue

Description

We are seeing scenarios where bid adapters are setting invalid meta.advertiserDomains

This field should be an array of strings

We noticed at least one adapter sending it as an double array with one string in it like so:

image

[['nytimes.com']]

I am unsure if this is happening ALL of the time for KARGO, or if there are certain scenarions where their server responds with a single adomain or it is always with the array of single element.

They get a bid response from their exchange =>

Notice that it returns advertiserdomains as an array of advertisers (which is exactly what it should be)

image

BUT their code wraps this inside of another array as seen here:

https://github.com/prebid/Prebid.js/blob/master/modules/kargoBidAdapter.js#L77

Which results in image

which is technically not correct!

We found this in our Analytics aggregations so I added a PR for us to handle this for now: #7908

But want to let Kargo know just in case!

@robertrmartinez
Copy link
Collaborator Author

robertrmartinez commented Jan 5, 2022

@jsadwith As you were the last person to update the kargoBidAdapter I wanted to let you know this issue we uncovered.

I would update the adapter and tag you in a PR, but I do not know the inner-workings of the kargo adserver so wanted you to check in and make sure how the field in the server response is coming.

Whenever I see it on our pages it looks to always be set as array of strings, so if that is true, the "fix" would be to just pass it along as is and not wrap it in another array.

Let me know what you think!

@jsadwith
Copy link
Contributor

jsadwith commented Jan 5, 2022

@robertrmartinez Thanks for the note. This is definitely an issue from our end, so I created a PR to remove the extra array - #7910

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

Successfully merging a pull request may close this issue.

2 participants