react-native-gesture-handler icon indicating copy to clipboard operation
react-native-gesture-handler copied to clipboard

Fix possible web crashes

Open billouboq opened this issue 5 months ago • 4 comments

Description

Without being able to reproduce, we saw some possible web crashed happening in react-native-gesture-handler. So adding more guards to avoid that

Here are the sentry crashed : Screenshot 2025-06-12 at 10 53 08 Screenshot 2025-06-12 at 11 02 58

billouboq avatar Jun 12 '25 09:06 billouboq

Hi @billouboq! First of all, thanks for this PR! Is there any stack trace for the first error? Looking at the code, situation where last coordinates are undefined shouldn't happen in this function, so it would be great to see what causes that behavior.

m-bert avatar Jun 16 '25 11:06 m-bert

@m-bert Unfortunatly, we couldn't have more details about that, it was only showing in Sentry and not a single user reported an issue, and we cannot reproduce on our side

billouboq avatar Jun 16 '25 11:06 billouboq

Okay, I'll try to find out what is the root cause. What Gesture Handler version do you use? I assume it was reported on 2.25.0

m-bert avatar Jun 16 '25 13:06 m-bert

Hi again @billouboq! Unfortunately I was not able to reproduce it, though I think it may be caused by one of the following:

  1. Pointer is removed form PointerTracker before handler enters BEGAN state - removing may happen for example when one lifts pointer from the view.
  2. PointerTracker is reset before handler enters BEGAN state - this one can happen when pointercancel event is received.

For now I can't see any other possibility, but also I'm not sure how Gesture Handler may enter one of those two states.

Either way, we decided to fix it in a different way (which hopefully will work). I created #3565 which should solve this problem. It would be great if you could test it, but I understand that it may be challenging.

About this PR, please leave only the second fix, for hasPointerCapture - if CIs pass then we can merge it 😅

m-bert avatar Jun 17 '25 09:06 m-bert

Hi @billouboq! Just to let you know, I've merged #3565, so these changes are no longer necessary in this PR

m-bert avatar Jun 26 '25 10:06 m-bert

Thanks a lot for the changes ! 🙏

billouboq avatar Jun 26 '25 10:06 billouboq

So are you willing to contribute to fix the other crash? 😅 If so, please leave only changes in PointerEventManager

m-bert avatar Jun 26 '25 10:06 m-bert

Will do right now otherwise I know I will never take the time to do it 😁

billouboq avatar Jun 26 '25 10:06 billouboq

@m-bert Done

billouboq avatar Jun 26 '25 10:06 billouboq