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

Sovrn adapter - add no bid responses for all placements that are not mentioned in the Sovrn bid response #183

Merged
merged 1 commit into from
Feb 9, 2016

Conversation

baragona
Copy link
Contributor

@baragona baragona commented Feb 8, 2016

Fixed version of #163

My understanding is that the bid manager wants to see addBidResponse called the same number of times as expectedBidsCount, before executing the callback. Sovrn adapter doesn't set expectedBidsCount, so it will be set to the default (from adaptermanager.js):

        // if the bidder didn't explicitly set the number of bids
        // expected, default to the number of bids passed into the bidder

Multiple Sovrn bid requests are merged into a single call to sovrn. When sovrn encounters an error or doesn't bid on anything, they don't sent an explicit no-bid response for each bid request. Before this PR change, the sovrn adapter called addBidResponse only 1 time in this case.

Thus addBidResponse may only be called 1 time. But bidmanager expects it to be called more times, so it does not fire the callback immediately, instead, it waits for the timeout to occur.

I've been running this code in production for a several weeks now and it seems to work well.

@pdramos1
Copy link
Collaborator

pdramos1 commented Feb 8, 2016

Thanks for moving this to a separate PR.

pdramos1 added a commit that referenced this pull request Feb 9, 2016
Sovrn adapter - add no bid responses for all placements that are not mentioned in the Sovrn bid response
@pdramos1 pdramos1 merged commit b220a81 into prebid:master Feb 9, 2016
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 this pull request may close these issues.

2 participants