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

Sonobi Adapter - Added referrer param. Fixed timeout error in userSync #2497

Merged
merged 15 commits into from
May 10, 2018

Conversation

JonGoSonobi
Copy link
Contributor

Type of change

  • Bugfix
  • Feature

Description of change

Added a referrer param to override the default window.location.host.
Fix for issue #2489

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

prebid/prebid.github.io#744

Other information

Fixes Issue #2489

@JonGoSonobi
Copy link
Contributor Author

@ndhimehta any movement on this?

@snapwich snapwich merged commit 3d96d9e into prebid:master May 10, 2018
kitwestneat pushed a commit to kitwestneat/Prebid.js that referenced this pull request Jun 4, 2018
@dmitriyshashkin
Copy link
Contributor

@JonGoSonobi tbh, not the best way to fix the problem. Instead of checking if the array is empty you wrap it in a try/catch. Not the most appropriate way to check the length of the array. And then you log the error. The situation that is causing the error is not uncommon or unexpected, requests to server timeout all the time. And now I get an ugly error message each time sonobi times out.

@JonGoSonobi
Copy link
Contributor Author

@dmitriyshashkin The error should only be logged in debug mode. In which case it would let you know that the timeout for the Sonobi adapter is too low and that you should consider increasing it.

@dmitriyshashkin
Copy link
Contributor

@JonGoSonobi TypeError: Cannot read property 'body' of undefined - it's rather difficult to understand that I should consider raising the timeout from this message. Besides prebid already fires the following error Server call for sonobi failed: . Continuing without bids. I'd say it's much more informative

@JonGoSonobi
Copy link
Contributor Author

@dmitriyshashkin PR opened to remove the debug logging. #2686
We left the try catch, otherwise we would have to check for not only the serverResponses Array being not empty, but also that the first index of it has a body and sbi_px is an array. We feel that just surrounding the parsing of server responses in a try catch is a much neater approach.

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

Successfully merging this pull request may close these issues.

5 participants