magesuite / theme-creativeshop

Open Software License 3.0
38 stars 24 forks source link

fix(addtocart): Fix `Cannot read property 'length' of null` error #48

Closed jtomaszewski closed 2 years ago

jtomaszewski commented 3 years ago

We have quite a bit of errors on Sentry that look like this:

TypeError: Cannot read property 'length' of null
  at t._onDone(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/js/category.js:1:48422)
  at HTMLDocument.<anonymous>(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/js/category.js:1:52633)
  at HTMLDocument.dispatch(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:5232:27)
  at HTMLDocument.elemData.handle(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:4884:29)
  at Object.trigger(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:5136:13)
  at Object.jQuery.event.trigger(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery/jquery-migrate.js:633:22)
  at HTMLDocument.<anonymous>(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:5866:18)
  at Function.each(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:376:20)
  at jQuery.fn.init.each(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:142:18)
  at jQuery.fn.init.trigger(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:5865:16)
  at Object.success(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/Magento_Catalog/js/catalog-add-to-cart.js:122:33)
  at fire(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:3238:32)
  at Object.fireWith [as resolveWith](/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:3368:8)
  at done(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:9846:15)
  at XMLHttpRequest.callback(/static/version1607965126/frontend/Creativestyle/theme-groomershop/pl_PL/jquery.js:10317:9)

Not sure yet how exactly it can be reproduced. But this should fix it. You already have such a check at https://github.com/magesuite/theme-creativeshop/blob/next/src/components/addtocart/addtocart.ts#L268 , so seems like it's just missing at the other places.

jtomaszewski commented 3 years ago

Note, this hasn't resolved the root issue. A few days a day we get a Sentry event about this this error happening. Result is: user clicks "Add to cart" button and nothing happens.

It's very difficult to reproduce though. (Maybe it depends on network speed and JS loading order?) And because of that I'm not yet able to tell how it should be fixed.

msiewierska commented 3 years ago

I tried to reproduce it and I was able to see the bug only twice, on list view mode, when I was clicking on add to cart button very fast, maybe before the whole page was loaded. We would like to merge your fix, thank you for your contribution. Could you only remove console.warn and place the warning in a comment?

jtomaszewski commented 3 years ago

The thing is, the issue is still there ;( We've been trying hard to reproduce and fix it; but it seems to be quite difficult to repro it (I couldn't do it; my friend made it once or twice though. And we know our customers do repro it on a daily basis, as we get Sentry warnings about it - in our own theme, we have Sentry.captureException instead of the console.warn.)

drabikowy commented 3 years ago

Hello @jtomaszewski Thank you for pointing out this issue. We saw it a few times in our projects, but indeed it is hard to catch. We decided to create an internal ticket to investigate why these elements can be unavailable instead of applying checks. Sorry for the delays with our Github activity, but we have to keep the focus on our projects. However, we'll try to catch up more frequently with MRs. We will keep this open and let you know if we find anything.

msiewierska commented 2 years ago

@jtomaszewski Some time ago we introduce a check to adtocart.ts file

Zrzut ekranu 2021-10-8 o 16 24 18

The concept is quite similar to you changes, scripts is not executed if there is no $component variable, which is:

this._$component = $('.atc-ajax-processing').closest( .${this._options.componentClass} );`

It seems to me that your PR can be closed now.

jtomaszewski commented 2 years ago

It'll hide the error from Sentry that's for sure. Whether that fixes the original error (assuming it exists - person clicks "add to cart" button and it does nothing), I don't know. I guess we can close it, until somebody reports it.