Amphtml: Error: null is not an object (evaluating 'new a.Image')

[choumx]

Error: null is not an object (evaluating 'new a.Image')
at Image (https://raw.githubusercontent.com/ampproject/amphtml/1908121824440/src/window-interface.js:95)
at iframe (https://raw.githubusercontent.com/ampproject/amphtml/1908121824440/src/pixel.js:61)
at win (https://raw.githubusercontent.com/ampproject/amphtml/1908121824440/extensions/amp-analytics/0.1/transport.js:246)
at (https://raw.githubusercontent.com/ampproject/amphtml/1908121824440/extensions/amp-analytics/0.1/transport.js:132)
at requestUrl (https://raw.githubusercontent.com/ampproject/amphtml/1908121824440/extensions/amp-analytics/0.1/requests.js:260)

My guess is the window is destroyed by the time Transport.sendRequest is called. A null check may fix it.

/cc @ampproject/wg-analytics

[Enriqe]

@ampproject/wg-analytics @dvoytenko any updates here?

Still seeing 5.9k occurrences in the current 2001251659540 canary.

[dvoytenko]

@zhouyx, is there some more magic we could do to bring this number down a bit more?

[choumx]

My guess is the window is destroyed by the time Transport.sendRequest is called. A null check may fix it.

Have we tried doing this?

[zhouyx]

No we haven't, shall i try checking the win before creating the pixel?
https://github.com/ampproject/amphtml/blob/47a4915b343430e00b6ea7dd8ead98b92d68ddfe/extensions/amp-analytics/0.1/transport.js#L243-L245

but I think the window is the value from this.win_ which is cached? Do you think it will help?

[dvoytenko]

Hmm. It seemed like the stack traces were circling around Transport.sendRequest. But that API is not used for pixel, is it?

[dvoytenko]

Oh. Just figured it out.

[dvoytenko]

So, yeah, I think the problem is here: https://github.com/ampproject/amphtml/blob/47a4915b343430e00b6ea7dd8ead98b92d68ddfe/src/pixel.js#L74

So we just need to bailout there or somewhere before then when win == null

[zhouyx]

Ah so we pass in param by reference, makes sense. I'll create a quick fix.

[dvoytenko]

The reason why it's hard to notice is probably b/c of the const Image = WindowInterface.getImage(win) in the:

const Image = WindowInterface.getImage(win);
const image = new Image();

It most likely gets inlined by the compiler, thus simply becoming:

const image = new win.Iimage();

[rcebulko]

This is still occurring, and seems to occur only on iPhone. We have a new occurrence of this error appearing in Beta and Experimental exclusively on https://www-corriere-it.cdn.ampproject.org/ pages. It started last night 2020-03-11 some time 8pm-10pm, I'm guessing from a change they pushed to their site. It's currently reporting around 1000 errors an hour; that number will go up 10x if/when those releases are promoted.

[rcebulko]

By digging through the minified stacktrace, I was able to identify that this is happening via sendRequestUsingImage in transport.js

[rcebulko]

It looks like the culprit may be here:

function createNoReferrerPixel(win, src) {
  if (isReferrerPolicySupported()) {
    return createImagePixel(win, src, true);
  } else {
    // if "referrerPolicy" is not supported, use iframe wrapper
    // to scrub the referrer.
    const iframe = createElementWithAttributes(
      /** @type {!Document} */ (win.document),
      'iframe',
      dict({
        'src': 'about:blank',
        'style': 'display:none',
      })
    );
    win.document.body.appendChild(iframe);
    createImagePixel(iframe.contentWindow, src);
    return iframe;
  }
}

All other callees of createPixel assert win. In this case, if the referrer policy isn't supported, it tries to create an image pixel in an iframe and then get the iframe's contentWindow. I suspect at this point in execution the iframe has not actually been added to the DOM and ready hasn't fired, so no contentWindow is available yet. Perhaps moving createImagePixel within an onReady handler would resolve this.

@ampproject/wg-analytics any thoughts on this?

[zhouyx]

Thank you @rcebulko for looking into this. I agree that the iframe contentWindow may not be ready yet.
I wonder what is causing the new occurrence though.

[rcebulko]

@zhouyx Presumably some update to their site hits the exact corner case that triggers this bug

[zhouyx]

I found that they're using the vendor nielsen, which set referrerPolicy to no-referrer. Not sure if that's a recent change.
I also found that all errors are from Safari 13. That might be a browser issue, because referrerPolicy should have been supported. https://caniuse.com/#feat=mdn-html_elements_img_referrerpolicy
However I'm not able to reproduce this bug locally.

Thanks for the investigation. I'll fix the issue follow Ryan's suggestion. Since this is not a recent regression, I don't see a need for cherrypick.

[rcebulko]

@zhouyx I already threw together a PR with a fix, if you'd prefer to take this over feel free to duplicate and close mine.

[rcebulko]

I also can't produce locally because I don't have an iPhone (or a decent emulator of one). I agree it's not recent and probably doesn't merit a cherry-pick, though it will be one of the highest-volume errors being reported for a while.

[zhouyx]

Woohoo! Thank you! Left one comment.

Agreed that error number will remain high for a while. Can we mute this in the error log for two weeks?