firing new adRenderFailed event when renderAd() fails #2210
Conversation
vedantseta
changed the title
vedantseta
changed the title
|
added a reason key, so can GROUP BY easily. |
vedantseta
force-pushed
the
feature/ad-render-fail-event
branch
from
a8d49d8
to
ba7fcbe
Compare
|
@vedantseta Thanks for PR. Please add unit tests |
|
yes, sure. will add the same in few days. |
|
@jaiminpanchal27 , can you please check |
|
@vedantseta Can you also submit a docs PR to the docs repo for this contribution. You need to update https://github.com/prebid/prebid.github.io/blob/master/dev-docs/publisher-api-reference.md for new event to be added to the list on this page. http://prebid.org/dev-docs/publisher-api-reference.html#module_pbjs.onEvent |
|
@jaiminpanchal27 , have updated the docs of the same |
|
@vedantseta Thanks for contributing. Since this is core change, someone from our team will do a 2nd review and merge. |
src/prebid.js
Outdated
| @@ -229,7 +233,10 @@ $$PREBID_GLOBAL$$.renderAd = function (doc, id) { | |||
| if (renderer && renderer.url) { | |||
| renderer.render(bid); | |||
| } else if ((doc === document && !utils.inIframe()) || mediaType === 'video') { | |||
| utils.logError(`Error trying to write ad. Ad render call ad id ${id} was prevented from writing to the main document.`); | |||
| error.status = true; | |||
| error.bid = bid; | |||
There was a problem hiding this comment.
all this repeat code can go into a function. Pass the status string.
|
@mkendall07 , can you please check now? |
|
@jaiminpanchal27 @mkendall07 have refactored the code, can you please check? |
src/prebid.js
Outdated
|
|
||
| data.reason = reason; | ||
| data.message = message; | ||
| if (data.bid) { |
There was a problem hiding this comment.
I wanted to check if this logic is properly configured. It seems like this if condition would never be met because the data is initialized as empty each time the function is called, so the data.bid property would always be undefined. Can you please take a look and clarify?
There was a problem hiding this comment.
agreed. Changing that
* 'master' of https://github.com/prebid/Prebid.js: EngageBDR New Bid Adapter (prebid#2309) [FEAT] adunit sizes support (prebid#2320) Support aliases in prebidServer (prebid#2257) Changing default currency file to https (prebid#2306) Update stalebot labels (prebid#2319) Enhance location detection within utils (prebid#2167) if cache markup is not enabled, set it to the default value 0 (prebid#2302) Serverbid Bid Adapter: Added archon alias (prebid#2293) Smart Ad Server: Fix bug when multi bids (prebid#2170) NEW adapter AdtelligentBidAdapter (prebid#2137) add optional param to bridgewellBidAdapter (prebid#2289) Increment Pre Version Prebid 1.6.0 Release Unit test fixes (prebid#2301) PBS videoCacheKey and vastUrl (prebid#2101) Add Oneplanetonly Bid Adapter (prebid#2269) firing new adRenderFailed event when renderAd() fails (prebid#2210) Add Content Ignite adapter (prebid#2268) add hb_cache_id, hb_uuid should be deprecated and replaced by hb_cache_id (prebid#2273) Update Yieldlab adapter and add official maintainer (prebid#2231) Update for Media.net adapter (prebid#2232) Update to Rubicon Adapter for mediaTypes support (prebid#2272) message formatting (prebid#2285) Yieldbot impression image creation fix (prebid#2277) Updated Bid params (prebid#2275) Audience Network: Add 'pbv' and 'cb' query params (prebid#2252) Add e-planning analytics adapter (prebid#2211) Add vastUrl for Gamma Adapter Video (prebid#2261) update params for test bid (prebid#2267) Updated adUnitCode (prebid#2262)
* firing adRenderFailed when ad render fails * test case added * refactored adRenderFail event * Condition changed
Type of change
Description of change
This pull request resolves #2133 . If renderAd() fails for some reason, an event
adRenderFailedis fired with data object which includes the error message and bid object if available.