fix: Ensure adSlot is available before method calls

[sfrieson-tm]

Ensure the availability of the ad slot before calling methods on it.

First of all I'd like to thank you for your work in keeping this package up to date. We have been dependent on the original NFL and now the Ticketmaster fork, but they don't seem to be making any more updates. We're evaluating our ability to move over to your fork.

Issue

We've had a problem for a while (original and fork) where we are presented with this error. Before it was only on specific pages of our app and on refresh. With the atmedia fork it's on every in-app page transition.

Exception in queued GPT command TypeError: Cannot read property 'defineSizeMapping' of undefined
    at Arguments.<anonymous> (Bling.js:568)
    at Dj.push (pubads_impl_319.js?21063343:1)
    at wh.<anonymous> (pubads_impl_319.js?21063343:1)
    at wh.push (pubads_impl_319.js?21063343:1)
    at Bling.defineSizeMapping (Bling.js:561)
    at Bling.configureSlot (Bling.js:673)
    at Bling.shouldComponentUpdate (Bling.js:455)
    ...

Investigation

I've tried my best to understand what all is happening here functionally and logically, but I am sure that I don't have full knowledge of what is happening. This is my attempt at fixing the issue and the rationale behind it.

The Bling this.defineSizeMapping(adSlot, sizeMapping) method seems to be called before the adSlot is even created yet. The this._adSlot being passed down from the shouldComponentUpdate is undefined. I tried calling this.isAdMounted() to see if I could use that as a check, but it returns true.

When looking for where this._adSlot is set, I found that it is only set as a result of componentDidUpdate, which was a little disheartening. In checking which properties/values need to be available for different paths, the best option I could find is the change I'm proposing here.

Proposed solution

If this._adSlot being set is a result of the this.renderAd() and this.defineSlot() methods, then it seems to logically make sense to say that shouldRender should be true when this._adSlot is false-y. This also makes shouldRefresh become false because it logically has an inverse relationship with shouldRender. The shouldRefresh was true before which allowed its conditional block below to run, call these methods, and fail.

Do you have any better suggestions here? I have seen reference to similar issues in the NFL repo, so I don't think I'm the only one with this problem, but it also doesn't seem widespread either. This error doesn't happen on all the ads on our page.

All tests that were passing before are still passing.

[andrewb]

@sfrieson-tm thanks for the PR and detailed description. We'll do our best to review this soon.

[andrewb]

@sfrieson-tm do you have an easy way to reproduce this error? I'd like to observe it so I can try to understand it more.

[sfrieson-tm]

Hi @andrewb, I unfortunately do not. I can try to make some time to create a small repo to reproduce the issue. Unfortunately, this is a super busy time for me as it is my final few days at this job, and then I'll obviously lose access to our GAM account, etc.

I know there are some quirks to our implementation because of years of different people working on both the GPT and the GAM/DFP sides, so this may be an edge case only found by our unique usage.

[kwidholm-tm]

We are having this issue so I'm commenting just to follow the thread . Thanks.