Wordpress-android: JSApplicationIllegalArgumentException: Error while updating property 'flexDirection' in shadow node of type: RCTView

Created on 6 Sep 2019  路  14Comments  路  Source: wordpress-mobile/WordPress-Android

Sentry Issue: WORDPRESS-ANDROID-1D

IllegalArgumentException: method com.facebook.react.uimanager.LayoutShadowNode.setFlexDirection argument 1 has type java.lang.String, got com.facebook.react.bridge.DynamicFromObject
    at java.lang.reflect.Method.invoke(Method.java)
    at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateShadowNodeProp(ViewManagersPropertyCache.java:107)
    at com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackShadowNodeSetter.setProperty(ViewManagerPropertyUpdater.java:156)
    at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:60)
    at com.facebook.react.uimanager.ReactShadowNodeImpl.updateProperties(ReactShadowNodeImpl.java:319)
...
(12 additional frame(s) were not displayed)

JSApplicationIllegalArgumentException: Error while updating property 'flexDirection' in shadow node of type: RCTView
    at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateShadowNodeProp(ViewManagersPropertyCache.java:121)
    at com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackShadowNodeSetter.setProperty(ViewManagerPropertyUpdater.java:156)
    at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:60)
    at com.facebook.react.uimanager.ReactShadowNodeImpl.updateProperties(ReactShadowNodeImpl.java:319)
    at com.facebook.react.uimanager.UIImplementation.createView(UIImplementation.java:265)
...
(11 additional frame(s) were not displayed)

Error while updating property 'flexDirection' in shadow node of type: RCTView

Over the last 14 days
Latest affected version: 13.1
~732 events
~604 users affected

General [Type] Crash gutenberg-mobile

Most helpful comment

@mkevins @geriux Great investigation so far.

I was tempted at first to try a Rx style debounce function for only allowing a one click per X milliseconds. But even at a full second debouncing limit I could get 2 editors to open simultaneously.

I also considered a more generalized onClick handler applied multiple places in the app that prevents double clicks, but I then realized that we'd also want to prevent the case where someone clicks on two different posts in a post list simultaneously, since two editors can crash the app, and that is a special case in the app. For example elsewhere, we can click two different images simultaneously on Media List and open two different media activities, but that might not necessarily be bad.

From what I can tell, it seems that a proper fix for this would require maintenence of several post action button's disabled states on a level which could be mutated (back to enabled) upon return from the EditPostActivity.

This was the type of solution I landed on as well.

I have a Draft PR here with a functioning fix for the issue. But I believe that the code should be updated to follow Android Architecture components conventions. I was thinking instead of sending a reference of the containing Fragment to our Post List ViewHolder (link), a better solution would be to use a LifeCycle object perhaps passing information to our PostListAdapter using LiveData and an observer. Unfortunately I'm a little rusty on the newest patterns there so I ran out of time while updating the Draft PR to have the cleaner implementation.

My next move would be to clean up the implementation of my fix PR https://github.com/wordpress-mobile/WordPress-Android/pull/12586, and then get some feedback, unless someone uses my solution to submit something sooner.

All 14 comments

@daniloercoli think you could take a look at this one when you get a chance?

90-day impact: ~8 per day
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

90-day impact: ~13 per day
Users affected in the last 90 days: 908
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

90-day impact: ~18 per day
Users affected in the last 90 days: 1100
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

Users affected in the last 90 days: 1300
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

Users affected in the last 90 days: 1400
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

Users affected in the last 90 days: 1600
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

112 events have been tracked for this crash in 14.6.1 since it was released 9d ago on Apr 22.

Number of users affected in the last 90d: 2,000
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

357 events have been tracked for crash WORDPRESS-ANDROID-1D in 14.7 and 14.7.1 since they were released 15d and 4d ago respectively.

Events in the last 90d: 6,300
Events in the last 15d for 14.7: 225
Events in the last 4d for 14.7.1: 132
Users affected in the last 90d: 2,100
Also see: 90d graph
https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

Update to add a graph for the last 90d:

Screen Shot 2020-05-19 at May 19 5 57 16 PM

After checking the logs I saw that the editor was being instanced more than once, you can see in some of them:

Hermes is: true
Hermes is: true

So I wondered if we are somehow allowing for this to happen and it turns out that if you tap very quickly either on the new post button or in the edit button of a post it'll open two editors.

If its an empty post doesn't look like it crashes but if you have content in them it will crash and add another report to this issue in Sentry.

Steps to reproduce

  • Create a post with this content:
<!-- wp:image {"width":610,"height":406,"sizeSlug":"large"} -->
<figure class="wp-block-image size-large is-resized"><img src="https://images.pexels.com/photos/417173/pexels-photo-417173.jpeg?auto=compress&amp;cs=tinysrgb&amp;dpr=2&amp;h=750&amp;w=1260" alt="" width="610" height="406"/><figcaption><strong>Cool image</strong></figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2>Cool images</h2>
<!-- /wp:heading -->

<!-- wp:group -->
<div class="wp-block-group"><div class="wp-block-group__inner-container"><!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:image {"sizeSlug":"large"} -->
<figure class="wp-block-image size-large"><img src="https://images.pexels.com/photos/1366919/pexels-photo-1366919.jpeg?auto=compress&amp;cs=tinysrgb&amp;dpr=2&amp;h=750&amp;w=1260" alt=""/></figure>
<!-- /wp:image --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:image {"sizeSlug":"large"} -->
<figure class="wp-block-image size-large"><img src="https://images.pexels.com/photos/1670187/pexels-photo-1670187.jpeg?auto=compress&amp;cs=tinysrgb&amp;dpr=2&amp;h=750&amp;w=1260" alt=""/></figure>
<!-- /wp:image --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button {"style":{"color":{"background":"#00b2ff"}}} -->
<div class="wp-block-button"><a class="wp-block-button__link has-background" style="background-color:#00b2ff">Button 1</a></div>
<!-- /wp:button -->

<!-- wp:button {"style":{"color":{"background":"#1eeb82"}},"className":"is-style-fill"} -->
<div class="wp-block-button is-style-fill"><a class="wp-block-button__link has-background" style="background-color:#1eeb82">button 2</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons --></div></div>
<!-- /wp:group -->
  • Open the WordPress Android app.
  • Go to Posts.
  • Look for your newly created post.
  • Tap on Edit several times (you have to be quick).
  • Expect to see two editors loading and then crashing.

We should prevent loading more than one editor.

I struggled to reproduce this one on a real device (I guess I'm not fast enough at tapping :smile: ). However, I did manage to reproduce it via the emulator (Pixel 3 XL - API 28). Some notes on what I observed:

  • Some "double-sessions" don't result in a crash
  • The "double-sessions" (the editor opening twice) are possible on both Gutenberg and Aztec
  • After successfully causing a "double-session", the behavior of the app was subsequently unstable (at times, the UI would "shiver" or act in other unpredictable ways - janky is an understatement)


Editing the same post twice at once with Aztec


It seems there is a race condition between when the edit button is pressed and the EditPostActivity is launched. The onClickListener is set on the button in the PostListItemViewHolder:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListItemViewHolder.kt#L89

This button click invokes an action in a lambda set in PostListItemUiStateHelper:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt#L89

The invoked action is passed in as a lambda from PostListViewModel:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt#L368

which calls the handlePostButton method of the PostActionHandler:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/ui/posts/PostActionHandler.kt#L73

which calls through a chain of methods to check for conflicts / autosave revisions, pending media uploads, etc. and conditionally invoking triggerPostListAction with an EditPost action:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/ui/posts/PostActionHandler.kt#L198

The triggerPostListAction is passed into the postActionHandler as a small block which calls postValue on the private SingleLiveEvent receiver _postListAction:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt#L176

SingleLiveEvent.postValue is derived from MutableLiveData for which the postValue has this notable behavior:

If you called this method multiple times before a main thread executed a posted task, only the last value would be dispatched.

The problem seems to be that this event and the related dispatched actions are occurring quickly enough that all values are dispatched, resulting in multiple launches of the EditPostActivity:
https://github.com/wordpress-mobile/WordPress-Android/blob/c4bdaee364f0e6fc7ca9ec6dd383e9d04f87878f/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java#L676

In searching for a potential solution, I investigated post actions to try to find a way to make the edit-post-action idempotent.

I experimented with setting the launchMode of EditPostActivity to singleTop, but this did not prevent the editor from launching twice, I suspect because the first EditPostActivity is not yet at the top of the stack when the second one is launched.


Patch to set EditPostActivity's launch mode to singleTop

diff --git a/WordPress/src/main/AndroidManifest.xml b/WordPress/src/main/AndroidManifest.xml
index 6ab18b2a3a..6b5081bc0b 100644
--- a/WordPress/src/main/AndroidManifest.xml
+++ b/WordPress/src/main/AndroidManifest.xml
@@ -207,7 +207,8 @@
         <activity
             android:name=".ui.posts.EditPostActivity"
             android:theme="@style/WordPress.Editor.NoActionBar"
-            android:windowSoftInputMode="stateHidden|adjustResize">
+            android:windowSoftInputMode="stateHidden|adjustResize"
+            android:launchMode="singleTop">
             <meta-data
                 android:name="android.support.PARENT_ACTIVITY"
                 android:value=".ui.posts.PostsListActivity" />

I also tried a few other hacky experiments conditionally wrapping the edit post action and button listener in a lambda with a flag to disable the button (only allowing a single press), but this was problematic when returning to the post list.

From what I can tell, it seems that a proper fix for this would require maintenence of several post action button's disabled states on a level which could be mutated (back to enabled) upon return from the EditPostActivity.

Events in the last 90d: 7,700
Users affected in the last 90d: 2,500
WORDPRESS-ANDROID-1D: https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

image

@mkevins @geriux Great investigation so far.

I was tempted at first to try a Rx style debounce function for only allowing a one click per X milliseconds. But even at a full second debouncing limit I could get 2 editors to open simultaneously.

I also considered a more generalized onClick handler applied multiple places in the app that prevents double clicks, but I then realized that we'd also want to prevent the case where someone clicks on two different posts in a post list simultaneously, since two editors can crash the app, and that is a special case in the app. For example elsewhere, we can click two different images simultaneously on Media List and open two different media activities, but that might not necessarily be bad.

From what I can tell, it seems that a proper fix for this would require maintenence of several post action button's disabled states on a level which could be mutated (back to enabled) upon return from the EditPostActivity.

This was the type of solution I landed on as well.

I have a Draft PR here with a functioning fix for the issue. But I believe that the code should be updated to follow Android Architecture components conventions. I was thinking instead of sending a reference of the containing Fragment to our Post List ViewHolder (link), a better solution would be to use a LifeCycle object perhaps passing information to our PostListAdapter using LiveData and an observer. Unfortunately I'm a little rusty on the newest patterns there so I ran out of time while updating the Draft PR to have the cleaner implementation.

My next move would be to clean up the implementation of my fix PR https://github.com/wordpress-mobile/WordPress-Android/pull/12586, and then get some feedback, unless someone uses my solution to submit something sooner.

Events in the last 90d: 8,800
Users affected in the last 90d: 2,200
WORDPRESS-ANDROID-1D: https://sentry.io/share/issue/ab9754690c664b8b849fdf9c28f23830/

image

Was this page helpful?
0 / 5 - 0 ratings