-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Error: null is not an object (evaluating 'new a.Image') #23935
Comments
@ampproject/wg-analytics @dvoytenko any updates here? Still seeing 5.9k occurrences in the current 2001251659540 canary. |
@zhouyx, is there some more magic we could do to bring this number down a bit more? |
Have we tried doing this? |
No we haven't, shall i try checking the amphtml/extensions/amp-analytics/0.1/transport.js Lines 243 to 245 in 47a4915
but I think the window is the value from |
Hmm. It seemed like the stack traces were circling around |
Oh. Just figured it out. |
So, yeah, I think the problem is here: Line 74 in 47a4915
So we just need to bailout there or somewhere before then when |
Ah so we pass in param by reference, makes sense. I'll create a quick fix. |
The reason why it's hard to notice is probably b/c of the
It most likely gets inlined by the compiler, thus simply becoming:
|
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. |
By digging through the minified stacktrace, I was able to identify that this is happening via |
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 @ampproject/wg-analytics any thoughts on this? |
Thank you @rcebulko for looking into this. I agree that the iframe contentWindow may not be ready yet. |
@zhouyx Presumably some update to their site hits the exact corner case that triggers this bug |
I found that they're using the vendor nielsen, which set 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. |
@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. |
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. |
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? |
My guess is the
window
is destroyed by the timeTransport.sendRequest
is called. A null check may fix it./cc @ampproject/wg-analytics
The text was updated successfully, but these errors were encountered: