Skip to content
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

VASTClient not throwing error or emitting wrong error code for invalid wrapper VAST #476

Closed
ccampbell5 opened this issue Sep 21, 2024 · 3 comments

Comments

@ccampbell5
Copy link

Hi, I'm using version 6.0.1 and noticing inconsistent behavior with the VASTClient in how it processes wrapper VASTs that contain errors. The following sample code contains two wrapper VASTs; expected vs actual behavior is indicated in comments. (Just swap the test constant.) Also attaching the package.json file. package.json

  1. The expects303Error VAST is a two-layer wrapper that eventually points to an empty VAST document: <VAST xsi:noNamespaceSchemaLocation="vast.xsd" version="3.0"/>. According to the docs, the error code for "No VAST response after one or more Wrappers" is 303. However, no 'VAST-error' event is emitted for this VAST.
  2. The expectsThrowError wrapper VAST points to an unsupported 1.0 VAST document; in this case, my understanding is that the VASTClient's parseVAST method should return a rejected Promise since the root element is not 'VAST'. However, the resulting promise is not rejected, and the parser emits a 'VAST-error' with the 301 (timeout) code. (If you follow the VASTAdTagURI link and copy the VAST 1.0 XML from there, then pass it to parseVAST, then the expected rejected Promise is returned.)
import { VASTClient } from "@dailymotion/vast-client";
import xmlDom from "@xmldom/xmldom";
import fetch from "node-fetch";


const client = new VASTClient();

client.getParser().on("VAST-error", (err) => {
  console.log("VAST-error: ", err);
});

const getXML = (text) => {
  const parser = new xmlDom.DOMParser({
    errorHandler: (level, msg) => {
      throw new Error(`[Level: ${level}]: ${msg}`);
    },
  });

  const xml = parser.parseFromString(text, "text/xml");
  const err = xml
    ?.querySelector?.("parsererror")
    ?.querySelector?.("div")?.textContent;

  if (err) {
    throw new Error(err);
  }

  return xml;
};

// Expected:
// Should emit a 303 error since
// https://pubads.g.doubleclick.net/gampad/ads?iu=/21775744923/external/single_ad_samples&sz=640x480&cust_params=sample_ct%3Dredirecterror&ciu_szs=300x250%2C728x90&gdfp_req=1&output=vast&unviewed_position_start=1&env=vp&impl=s&nofb=1&correlator=
// will emit a 303 error

// Actual: No error is emitted
const expects303Error = `<VAST version="4.0" xmlns="http://www.iab.com/VAST">
<Ad id="20011" sequence="1" conditionalAd="false">
  <Wrapper followAdditionalWrappers="true" allowMultipleAds="1" fallbackOnNoAd="0">
    <AdSystem version="4.0">iabtechlab</AdSystem>
    <Error>http://example.com/error</Error>
    <Impression id="Impression-ID">http://example.com/track/impression</Impression>
    <Creatives>
      <Creative id="5480" sequence="1" adId="2447226">
        <CompanionAds>
          <Companion id="1232" width="100" height="150" assetWidth="250" assetHeight="200" expandedWidth="350" expandedHeight="250" apiFramework="VPAID" adSlotID="3214" pxratio="1400">
            <StaticResource creativeType="image/png">
              <![CDATA[https://www.iab.com/wp-content/uploads/2014/09/iab-tech-lab-6-644x290.png]]>
            </StaticResource>
            <CompanionClickThrough>
              <![CDATA[https://iabtechlab.com]]>
            </CompanionClickThrough>
          </Companion>
        </CompanionAds>
      </Creative>
    </Creatives>
    <VASTAdTagURI>
      <![CDATA[https://pubads.g.doubleclick.net/gampad/ads?iu=/21775744923/external/single_ad_samples&sz=640x480&cust_params=sample_ct%3Dredirecterror&ciu_szs=300x250%2C728x90&gdfp_req=1&output=vast&unviewed_position_start=1&env=vp&impl=s&nofb=1&correlator=]]>
    </VASTAdTagURI>
  </Wrapper>
</Ad>
</VAST>
`;

// Expected: Should throw an error since the root tag of the XML is not 'VAST'
// Actual: Parses successfully and emits a 301 (wrapper timeout)
const expectsThrowError = `
<VAST version="4.0" xmlns="http://www.iab.com/VAST">
<Ad id="20011" sequence="1" conditionalAd="false">
  <Wrapper followAdditionalWrappers="true" allowMultipleAds="1" fallbackOnNoAd="0">
    <AdSystem version="4.0">iabtechlab</AdSystem>
    <Error>http://example.com/error</Error>
    <Impression id="Impression-ID">http://example.com/track/impression</Impression>
    <Creatives>
      <Creative id="5480" sequence="1" adId="2447226">
        <CompanionAds>
          <Companion id="1232" width="100" height="150" assetWidth="250" assetHeight="200" expandedWidth="350" expandedHeight="250" apiFramework="VPAID" adSlotID="3214" pxratio="1400">
            <StaticResource creativeType="image/png">
              <![CDATA[https://www.iab.com/wp-content/uploads/2014/09/iab-tech-lab-6-644x290.png]]>
            </StaticResource>
            <CompanionClickThrough>
              <![CDATA[https://iabtechlab.com]]>
            </CompanionClickThrough>
          </Companion>
        </CompanionAds>
      </Creative>
    </Creatives>
    <VASTAdTagURI>
      <![CDATA[https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%201-2.0%20Samples/vast1Nonlinear.xml    ]]>
    </VASTAdTagURI>
  </Wrapper>
</Ad>
</VAST>`;

const test = expectsThrowError;

export function replaceUrlCorrelator(url) {
  const newURI = new URL(url);
  newURI.searchParams.set(
    "correlator",
    `${Math.floor(Math.random() * Date.now())}`
  );
  return newURI.toString();
}

async function urlHandlerGet(url, options) {
  const urlWithCorrelator = replaceUrlCorrelator(url);
  const response = await fetch(urlWithCorrelator, {
    ...options,
    credentials: options.withCredentials ? "include" : "omit",
  });
  const text = await response.text();

  return {
    document: getXML(text),
    original: text,
    status: response.status,
  };
}

const options = {
  resolveAll: true,
  wrapperLimit: 10,
  allowMultipleAds: true,
  urlHandler: {
    get: async (url, options) => {
      try {
        const result = await urlHandlerGet(url, options);
        console.log("Result from URL Handler for URL: ");
        console.log(url);
        return {
          xml: result.document,
          details: {
            byteLength: result.original.length,
            statusCode: result.status,
            rawXml: result.original,
          },
        };
      } catch (err) {
        console.log("Error from URL Handler for URL: ");
        console.log(url);
        console.log(err);
        return {
          error: err,
        };
      }
    },
  },
};

const parse = async () => {
  const vastXml = getXML(test);

  client
    .parseVAST(vastXml, options)
    .then((res) => {
      console.log("Parsed!! ");
      console.log(res);
    })
    .catch((err) => {
      console.log("Error! ", err);
    });
};

parse();
@ZacharieTFR
Copy link
Contributor

Thanks for raising the issue, we will look at it asap 😉

@ZacharieTFR
Copy link
Contributor

ZacharieTFR commented Oct 1, 2024

Hello, I've looked at the issues:

  • About expects303Error use case: I found out that without companions ads in the root vast, the 303 error is triggered as expected. Whereas with companion, it's not triggered anymore, it come from this condition

    if ((ad.errorCode || ad.creatives.length === 0) && !ad.VASTAdTagURI) {
    Since companions are inside creative node, ad.creatives won't be empty meaning the condition will not be met. We need to filter them out to know if we got an empty vast in the end.

  • About expectsThrowError use case: here are the culprits

    that prevent rejecting the error and
    ad.errorCode = 301;
    setting the wrong error code, it should be 102: VAST version of response not supported

We're going to fix it, but if you'd like a quick fix, feel free to contribute by opening a pull request! We'll be happy to look into it !

@ZacharieTFR
Copy link
Contributor

Hello,
Closing this issue has it has been fixed in #481 & released in 6.0.2 version. Please feel free to reopen it if you still reproduce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants