feat(unruly-bid-adapter): use bidResponse siteId when configuring the renderer #3865
Conversation
| @@ -6,7 +6,7 @@ import { VIDEO } from '../src/mediaTypes' | |||
| function configureUniversalTag (exchangeRenderer) { | |||
| parent.window.unruly = parent.window.unruly || {}; | |||
| parent.window.unruly['native'] = parent.window.unruly['native'] || {}; | |||
| parent.window.unruly['native'].siteId = parent.window.unruly['native'].siteId || exchangeRenderer.siteId; | |||
| parent.window.unruly['native'].siteId = parent.window.unruly['native'].siteId || exchangeRenderer.config.siteId; | |||
There was a problem hiding this comment.
can you please add a check to verify exchangeRenderer.config is defined before trying to access siteId?
I notice this exchangeRenderer object comes from your serverBid, and you may guarantee that this is always defined, but we would prefer to be defensive here please !
There was a problem hiding this comment.
Want to void Error: Cannot read property 'siteId' of undefined
There was a problem hiding this comment.
Roger that—throwing meaningful exceptions unless siteId is available.
There was a problem hiding this comment.
@paprikka Thanks,
quick question, where exactly does this exchangeRenderer.config come from?
The Unruly server request?
Is it set by the publisher somewhere?
There was a problem hiding this comment.
exchangeRenderer.config comes from the bidResponse (passed as an ext).
There was a problem hiding this comment.
Right, okay.
So the suggestion here is to not throw an error, but instead use the log utils to log the error and return the expected value back to prebid core.
for example, Prebid Core expects the bids back as an array. So instead of throwing the error,
Can we update to log the error, and simply return back an empty array to Prebid core?
Thanks!
There was a problem hiding this comment.
Sure, done. Thanks!
ghost
force-pushed
the
feat/bid-response-site-id
branch
from
a4bb5a6 to
d97238f
Compare
|
Regarding the CI issue: I've re-run the tests and everything seems fine on our side, can't access the CircleCI logs though—can you trigger another run? |
| @@ -4,9 +4,12 @@ import { registerBidder } from '../src/adapters/bidderFactory' | |||
| import { VIDEO } from '../src/mediaTypes' | |||
|
|
|||
| function configureUniversalTag (exchangeRenderer) { | |||
| if (!exchangeRenderer.config) throw new Error('UnrulyBidAdapter: Missing renderer config.') | |||
| if (!exchangeRenderer.config.siteId) throw new Error('UnrulyBidAdapter: Missing renderer siteId.') | |||
There was a problem hiding this comment.
Are these checks now irrelevant?
Due to the filter, it seems a bid would never come into this function unless it already has the config and siteID.
Obviously this is not a show-stopper, but if it is irrelevant, please remove it.
If you do not get to it before the release scheduled for tomorrow, I will make sure to still merge it and we can update in a later PR.
There was a problem hiding this comment.
Ah, yes—these are not required any more.
I'm afraid I don't time to deal with this today—will submit another PR later this week if that's ok?
There was a problem hiding this comment.
Yes, please do not forget to update it when you have a chance!
… renderer (prebid#3865) * feat(unruly-bid-adapter): use bidResponse siteId when configuring the renderer * feat(unruly-bid-adapter): bail if siteId is missing * feat(unruly-bid-adapter): log (but don't throw) when siteId is no present
… renderer (prebid#3865) * feat(unruly-bid-adapter): use bidResponse siteId when configuring the renderer * feat(unruly-bid-adapter): bail if siteId is missing * feat(unruly-bid-adapter): log (but don't throw) when siteId is no present
Type of change
Description of change
Ensure that the Unruly Bid Adapter can be used on sites where the Unruly Universal Tag has not been configured separately. This is a backwards-compatible change and it doesn't require any actions on Publisher's side.