-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Issue/cmm 278 react response thread exception #21802
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
Issue/cmm 278 react response thread exception #21802
Conversation
Generated by 🚫 Danger |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr21802-2292055 | |
Commit | 2292055 | |
Direct Download | wordpress-prototype-build-pr21802-2292055.apk |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr21802-2292055 | |
Commit | 2292055 | |
Direct Download | jetpack-prototype-build-pr21802-2292055.apk |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #21802 +/- ##
=======================================
Coverage 39.32% 39.32%
=======================================
Files 2125 2125
Lines 99871 99871
Branches 15385 15385
=======================================
Hits 39277 39277
Misses 57114 57114
Partials 3480 3480 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given we do not have reproduction steps, it is difficult to be confident this will address the crash. That said, given the crash reports, the proposed changes make sense.
I smoked tested both Gutenberg Mobile and GutenbergKit—adding various block types, uploading media, etc. I also tested offline editing. I did not encounter any issues. 🚀
Fixes #CMM-278
This PR makes sure we call
onSuccess
andonError
inReactNativeRequestHandler.handleResponse
on the main thread.We were unable to reproduce the problem, but the crash is a
CalledFromWrongThreadException
so calling from the main thread seems like the right solution (as suggested by Sentry's "Seer" AI). Note this is our leading crash right now, with 334 users and 1.1K events.To test, I recommend simply setting a breakpoint on the two changes and verify it all works as expected.