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

Error: "Cannot read property 'create' of undefined" #17997

Closed
dhanendrannewscorp opened this issue Sep 12, 2018 · 29 comments
Closed

Error: "Cannot read property 'create' of undefined" #17997

dhanendrannewscorp opened this issue Sep 12, 2018 · 29 comments

Comments

@dhanendrannewscorp
Copy link

Related: #17205

What's the issue?

Console error is showing on the AMP pages, which is blocking the amp-ad and amp-iframe contents.

How do we reproduce the issue?

https://www.thesun.co.uk/motors/7175820/true-cost-owning-car-revealed-east-anglians/amp
On the above page we can able to see the console errors which sometime disappear. But on few continues refreshed we can able to see the error.

Which AMP version is affected?

AMP ⚡ HTML – Version 1536699515199

screen shot 2018-09-12 at 11 51 06 am

screen shot 2018-09-12 at 11 57 54 am

When I have raised this issue with WordPress VIP, they mentioned as

If there were an issue with the amp-wp plugin, you would expect to see an AMP page fail validation because invalid AMP markup is not being removed. But in this case, the testing URL you've provided is passing AMP validation, and the JS error is happening in friendly-iframe-embed.js, which is being loaded directly from the ampproject Github repository. This leads us to believe that the issue lies in the AMP runtime JS and not within the amp-wp plugin and we would suggest opening an issue with the ampproject to see if they can identify why their runtime JS is failing in this instance.

@kevinkassimo
Copy link
Member

kevinkassimo commented Sep 12, 2018

Seen this error before.

Originated from info.create.length in document-register-element polyfill:

https://github.com/WebReflection/document-register-element/blob/07bea8eb13e00c3f493f9b641c43e9bc343160fe/src/document-register-element.js#L995

For some reason, it seems that for

info = constructors[nodeNames.get(self.constructor)];

We occasionally pass HTMLAnchorElement as self here, resulting info be undefined, and thus caused the error.

/to @jridgewell

@jridgewell
Copy link
Contributor

You can ignore this, it'll continue to work regardless.

We'll be releasing #17205 soon, which will fix the underlying issue.

@dhanendrannewscorp
Copy link
Author

Thanks @kevinkassimo and @jridgewell . I see #17205 is merged to master, may I know when this will be released?

@adelinamart
Copy link
Contributor

I can still test this in the current production and new canary also. @jridgewell please check. Thank you.

@jridgewell
Copy link
Contributor

It's still there. Fix will involve moving to a new Custom Elements polyfill, which I'll be doing testing in the next week.

@dhanendrannewscorp
Copy link
Author

@jridgewell did you managed to test this last week? Did it worked well? Thanks

@jridgewell
Copy link
Contributor

The test suite fails due to a difference in the old polyfill (which depended on the V0 native in our test suite, because we run our tests in Chrome), and the polyfill. We have a way to fix it, and I'll be working on it soon.

@dhanendrannewscorp
Copy link
Author

@jridgewell did you get a chance to work on the fix? Did the tested passed?

@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @jridgewell Do you have any updates?

@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @jridgewell Do you have any updates?

1 similar comment
@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @jridgewell Do you have any updates?

@nbeloglazov
Copy link
Contributor

nbeloglazov commented Jan 18, 2019

What's the status of this issue? If the error is expected, is it possible to at least silence it so that it doesn't add unnecessary noise and causes alarm among developers?

@jeffkaufman
Copy link
Contributor

I've seen several people get sidetracked in investigations, including me at least once, where they start trying to reproduce an issue, hit this error, and then think the error is responsible (because why would AMP throw an error when everything is working normally?)

@jridgewell
Copy link
Contributor

We have this prioritized for this quarter, likely starting in Feb.

@jeffkaufman
Copy link
Contributor

I've now seen two more situations where this confused people trying to debug unrelated issues.

@jeffkaufman
Copy link
Contributor

This should be higher than P3

@sparhami
Copy link

sparhami commented Feb 19, 2019

@jridgewell I'm seeing a significant (400x) increase in volume in the error with the latest canary. I know you mentioned that there shouldn't be any problem caused by this error, but the increase in errors seems to be a bit odd to me. Not sure if there is anything odd going on here.

Edit: it seems like the change in volume might be due to us going back to the original custom elements polyfill.

@jridgewell
Copy link
Contributor

Did @ampproject/a4a team start shipping FIE in more browsers? Otherwise, it's safe to ignore.

it seems like the change in volume might be due to us going back to the original custom elements polyfill.

We never actually turned on the new polyfill, it was just removed from prod builds to save space. We'll be doing the CE experiment in the next few months, likely.

@fox94610
Copy link

This is still an issue. This error fires on ad loads. We have ads loaded as they scroll into view. Long after the page has loaded, the ad comes into view and the error fires. Error fires for regular ads as well.
ss5
ss6

@fox94610
Copy link

fox94610 commented Apr 10, 2019

Still checking to see if anyone has looked into this issue. This bug is still out in the wild for amp ads. If you send me a message I can send you a staging URL revealing the error regularly.

@jridgewell
Copy link
Contributor

We already have the fix, but it's not been deployed yet. We'll be doing it soon. In the meantime, this error has no effect on the page, you can safely ignore it.

@fox94610
Copy link

Thank you!!!! Much appreciated.

@yogurito
Copy link

Is the fix already deployed? We still see this error message in console.

@jridgewell
Copy link
Contributor

The fix is still waiting to go out into experiment phase. We'll be doing that sometime this quarter.

@zombifier
Copy link
Contributor

Ping on this. It's not urgent or critical but the first time I came across this it's still annoying since I thought something was wrong. Thanks.

@jridgewell
Copy link
Contributor

The experiment is live (though it's not possible to opt into receiving the experimental code). Initial results look good, but we still need more time before shipping this wider.

@jeffkaufman
Copy link
Contributor

FYI this came up again today, where someone spent a long time debugging down the wrong path thinking that this error message was the reason an unrelated thing wasn't working

@jridgewell
Copy link
Contributor

#25141 We're close!

@jridgewell
Copy link
Contributor

Closed by #25141.

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

No branches or pull requests