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 'style' of null #22533

Closed
calebcordry opened this issue May 28, 2019 · 7 comments · Fixed by #25139 or #26788
Closed

Error: Cannot read property 'style' of null #22533

calebcordry opened this issue May 28, 2019 · 7 comments · Fixed by #25139 or #26788

Comments

@calebcordry
Copy link
Member

calebcordry commented May 28, 2019

About 12k errors in new canary on stackdriver. Sample stack trace:

Error: Cannot read property 'style' of null
at style (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/src/style.js:119)
at (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/src/style.js:157)
at (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/extensions/amp-lightbox-gallery/0.1/swipe-to-dismiss.js:252)
at adjustForSwipePosition_ (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/extensions/amp-lightbox-gallery/0.1/swipe-to-dismiss.js:383)
at (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/src/service/resources-impl.js:1032)
at (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/src/service/vsync-impl.js:459)
at callTaskNoInline (https://raw.githubusercontent.com/ampproject/amphtml/1905222334000/src/service/vsync-impl.js:417)

Looks to be amp-lightbox-gallery related.
cc/ @ampproject/wg-ui-and-a11y

@calebcordry
Copy link
Member Author

go/ampe/COuC3YCVi7zk7wE

@sparhami sparhami self-assigned this May 28, 2019
@sparhami
Copy link

sparhami commented May 28, 2019

Looks like there was a spike on Sunday in the error rate, but has been lower before/after. I can't reproduce this when loading the canary version. I'm not 100% sure what is going on here. Here are something I know:

The error is happening here, with swipeElement_ being null:

setStyles(dev().assertElement(this.swipeElement_), {

This is set on the first touch (from touchstart) here:

this.swipeElement_ = swipeElement;

via:

swipeElement: dev().assertElement(this.carousel_),

At that point, this.carousel_ cannot not be null, since it is initialized before the lightbox opens.

I refactored this logic, but the data flow should be the same. The one thing I did change was I made the function use an object parameter instead of simple arguments, but I can't imagine that has any effect.

The minified code looks something like:

                    a.Na.startSwipe({
                        swipeElement: a.I,
                        hiddenElement: c || b,
                        mask: a.aa,
                        overlay: a.T
                    })

and

        U.prototype.startSwipe = function(a) {
            var b = this
              , c = a.swipeElement
              , d = a.hiddenElement
              , e = a.mask
              , f = a.overlay;
            this.oa = c;
            this.Ba = d;
            this.aa = e;
            this.ja = f;
            this.Ea(function() {
                Ab(b)
            })
        }

It is a little odd that the object properties are not being renamed, but it seems to be fine.

@sparhami
Copy link

Reopening until we can confirm whether or not we fixed this.

@sparhami sparhami reopened this Oct 18, 2019
@jridgewell
Copy link
Contributor

This is still not fixed, and it's by far the biggest error report in production.

jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 1, 2019
I think we're receving badly ordered gestures, which is causing ampproject#22533.

Now, if we receive a subsequent gesture without first receving the `first` geture, we'll early exit. That should solve the bug. I'm also adding error reporting, so we can track down exactly how this is happening.
jridgewell added a commit that referenced this issue Nov 4, 2019
I think we're receving badly ordered gestures, which is causing #22533.

Now, if we receive a subsequent gesture without first receving the `first` geture, we'll early exit. That should solve the bug. I'm also adding error reporting, so we can track down exactly how this is happening.
@dreamofabear
Copy link

As of writing, both this error and the attempted fix ("subsequent without first" error in #25375) are both present and firing at high volume in RTV 1911191835190. The latter is about half the volume of the first.

micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this issue Dec 27, 2019
I think we're receving badly ordered gestures, which is causing ampproject#22533.

Now, if we receive a subsequent gesture without first receving the `first` geture, we'll early exit. That should solve the bug. I'm also adding error reporting, so we can track down exactly how this is happening.
@Enriqe
Copy link
Contributor

Enriqe commented Jan 28, 2020

@ampproject/wg-ui-and-a11y Can you please reassign this issue? Seeing 8k occurrences in the current canary 2001251659540 in the last day.

@kristoferbaxter
Copy link
Contributor

Assigning @wassgha to take a look.

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