-
Notifications
You must be signed in to change notification settings - Fork 0
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
KRKPD-996: refactors interpretResponse #30
Conversation
modules/kargoBidAdapter.js
Outdated
height: adUnit.height, | ||
ttl: 300, | ||
creativeId: adUnit.creativeID, | ||
cpm: Number(adUnit.cpm) || 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we could track errors around these default statements? My concern is we might mask issues returning invalid bids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we don't currently track any adapter errors, so adding this here wouldn't be useful. We should look into adding adapter error tracking and track these fallbacks as types of errors in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I've got a ticket for that now under the prebid maintenance epic
* refactors interpretResponse * updates comment * simpler * removes unnecessary comment * removes more unnecessary comments * revert ttl * reverts ttl test values * revert some || changes * removes comments --------- Co-authored-by: “Nick <“nick.llerandi”@kargo.com>
Type of change
Description of change
(https://github.com/prebid/Prebid.js/blob/de42f8d23da4d84c6e761aec1c2e3510e6dd3bd9/modules/rubiconBidAdapter.js#L685))