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
@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:

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.
<!-- 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&cs=tinysrgb&dpr=2&h=750&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&cs=tinysrgb&dpr=2&h=750&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&cs=tinysrgb&dpr=2&h=750&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 -->
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:
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.
@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.
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.
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
Fragmentto our Post ListViewHolder(link), a better solution would be to use a LifeCycle object perhaps passing information to ourPostListAdapterusing 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.