amphtml: : "Illegal mutation" error should provide more information

Currently, if I try to use setInterval() or setTimeout() on load, or if I try to make mutations >5 seconds after a user gesture, I get an error message like this:

[amp-script] amp-script[script="myscript"].js was terminated due to illegal mutation.

Most importantly, it would be much more useful to know what the illegal mutation was. Otherwise, as a developer, I don’t know what I’ve done wrong. If I’ve tried to make changes 5 seconds after a user interaction, or tried a delayed mutation after the hydration phrase on load, I should be told exactly what that is, so I know not to do it again!
Less importantly, if the error occurs in an inline script, it would be helpful to be told that instead of amp-script[script="myscript"].js.

This probably seems trivial, but letting developers know what they’ve done wrong will help them find and fix the error quickly!

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 28 (19 by maintainers)

Most upvoted comments

Yeah, still can’t reproduce.

So, if it’s easy to check whether the sanitizer is doing some sanitizing without throwing a notification of some sort, can we do that?

In the meantime, how about something like this:

CHILD_LIST: Blocked n attempts to modify DOM element children, innerHTML, or the like. For variable-sized <amp-script> containers, a user action has to happen first.
ATTRIBUTES: Blocked n attempts to modify DOM element attributes, styles, or the like. For variable-sized <amp-script> containers, a user action has to happen first.
CHARACTER_DATA: Blocked n attempts to modify textContent or the like. For variable-sized <amp-script> containers, a user action has to happen first.

I’m not sure when PROPERTIES gets thrown. Maybe someone could point to me the code? Are we actually throwing ATTRIBUTES sometimes when we should be throwing PROPERTIES?

I know my language there is wordy. I just haven’t succeeded in compressing the bit about variable-sized <amp-script> containers. But of course this string could just be stored once…

morsssss on Mar 20, 2020

Thanks for filing this. I think error messages may already be a lot more descriptive after #26401.

You should see console errors like “Dropped 2x CHILD_LIST mutation(s)” before termination. Also, termination only happens for layout=container now.

I’ve also thought about removing the termination end state entirely since we’re now filtering out (“dropping”) illegal mutations anyways. However, no-termination can cause the worker and main page to go out of sync which might be more confusing.

Hi, we are using amp-script to identify the device ( iOS/Android ) and show the app download image. It worked fine so far, but suddenly we are getting the below exception.
[amp-script] Dropped 2x “ATTRIBUTES” mutation(s); user gesture is required with [layout=container].

Please advise how to debug this issue

quickref on Feb 16, 2020

@samouri thank you, I now that this is not a good place to talk about, I miss understood the issue title. actually, I fixed it, it was because of the wrong place to wait for getBoundingClientRectAsync, now https://github.com/ampproject/worker-dom/issues/835 is the problem.

sayjeyhi on Apr 18, 2020

@sayjeyhi, I was able to run your example without any errors. Can you provide a fully reproducible case?

samouri on Apr 17, 2020

Then how about combining ATTRIBUTES and PROPERTIES into a single message:

Blocked n attempts to modify DOM element attributes, properties, or styles. For variable-sized <amp-script> containers, a user action has to happen first.

morsssss on Mar 30, 2020

Going to make that issue today, I was totally unable to reproduce these silent failures. And I was able to get the CHARACTER_DATA error.

Maybe it was me who was failing, not at all silently…

morsssss on Mar 20, 2020