dailymotion / vast-client-js

VAST (up to 6) parsing library for JavaScript
https://iabtechlab.com/wp-content/uploads/2022/09/VAST_4.3.pdf
MIT License
367 stars 216 forks source link

Bug: Fallback not working if top Ad Elements returns empty Vast or fails in case of multiple Ad Elements #465

Closed shubham-si closed 4 months ago

shubham-si commented 10 months ago

https://github.com/dailymotion/vast-client-js/blob/master/src/parser/vast_parser.js#L461C25-L461C25

const resolvedAds = util.flatten(unwrappedAds); default to [] Link

if (!resolvedAds && this.remainingAds.length > 0) {

There seems to be a issue here !resolvedAds. Since !Array<T> gives false always, thus the code within if will not be executed at all.

So If any Ad element (going from top to bottom) fails to fetch xml or return empty vast, it will not fallback to next Ad Element.

<VAST xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vast.xsd" version="3.0">
<Ad>
<Wrapper fallbackOnNoAd="true">
<AdSystem/>
<Error>
<![CDATA[ https://error-pixel?[ERRORCODE] ]]>
</Error>
<VASTAdTagURI>
<![CDATA[ ]]>
</VASTAdTagURI>
<Extensions>
<Extension type="waterfall" fallback_index="0"/>
</Extensions>
</Wrapper>
</Ad>
<Ad>
<Wrapper fallbackOnNoAd="true">
<AdSystem/>
<Error>
<![CDATA[ https://error-pixel?[ERRORCODE] ]]>
</Error>
<VASTAdTagURI>
<![CDATA[ does-not-exist ]]>
</VASTAdTagURI>
<Extensions>
<Extension type="waterfall" fallback_index="1"/>
</Extensions>
</Wrapper>
</Ad>
<Ad>
<Wrapper>
<AdSystem/>
<Error>
<![CDATA[ https://error-pixel?[ERRORCODE] ]]>
</Error>
<VASTAdTagURI>
<![CDATA[ https://glomex.github.io/vast-ima-player/linear-ad.xml ]]>
</VASTAdTagURI>
<Extensions>
<Extension type="waterfall" fallback_index="2"/>
</Extensions>
</Wrapper>
</Ad>
</VAST>

Fix: if (resolvedAds.length === 0 && this.remainingAds.length > 0) {

so that if resolvedAds fails and we have remainingAds, can be proceeded with them.

Rapha0511 commented 10 months ago

Hello ! thank you for reporting this issue, we will take a look at it asap !

shubham-si commented 9 months ago

@Rapha0511 is the bug fixed ?

ZacharieTFR commented 4 months ago

@shubham-si Hello, the bug has been fixed and released in 6.0.0 version ! I'm closing this issue, but but feel free to reopen it if you have any further problems.