Skip to content
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

Fix Android crash when worklet throws error #3558

Merged
merged 19 commits into from
Oct 12, 2022

Conversation

tomekzaw
Copy link
Member

@tomekzaw tomekzaw commented Sep 9, 2022

Description

This PR improves developer experience of error handling in worklets by showing a RedBox with full error message rather than crashing the whole app on Android.

Note: Android with Fabric enabled works only if building with NDK 21 via yarn react-native run-android [--active-arch-only], crashes when building with NDK 24 from Android Studio, see Shopify/flash-list#550 (comment) for details (this is not an issue with Reanimated)

Scenario Before After
Paper Fabric Paper Fabric
worklet 💥 💥
nested worklet 💥 💥
useAnimatedStyle 💥 💥
useDerivedValue 💥 💥
useFrameCallback 💥 💥
GestureDetector 💥 💥
useAnimatedGestureHandler 💥 💥
useAnimatedScrollHandler 💥 💥
useScrollViewOffset 💥 💥

Test code and steps to reproduce

Building with NDK 21 on M1-based Mac

if (System.properties['os.arch'] == "aarch64") {
// For M1 Users we need to use the NDK 24 which added support for aarch64
ndkVersion = "24.0.8215888"
} else {
// Otherwise we default to the side-by-side NDK version from AGP.
ndkVersion = "21.4.7075529"
}

-            ndkVersion = "24.0.8215888"
+            ndkVersion = "21.4.7075529"

ERROR: Unknown host CPU architecture: arm64

code ~/Library/Android/sdk/ndk/21.4.7075529/ndk-build
 #!/bin/sh
 DIR="$(cd "$(dirname "$0")" && pwd)"
-$DIR/build/ndk-build "$@"
+arch -x86_64 $DIR/build/ndk-build "$@"

Checklist

  • Included code example that can be used to test this change
  • Updated TS types
  • Added TS types tests
  • Added unit / integration tests
  • Updated documentation
  • Ensured that CI passes

@tomekzaw tomekzaw force-pushed the @tomekzaw/fix-android-crash-worklet-error branch from ade7e9d to 8a51e6a Compare October 3, 2022 14:36
@tomekzaw tomekzaw force-pushed the @tomekzaw/fix-android-crash-worklet-error branch from e00480f to b3afa8b Compare October 3, 2022 14:55
@tomekzaw tomekzaw marked this pull request as ready for review October 3, 2022 17:52
Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

@piaskowyk piaskowyk merged commit 559a842 into main Oct 12, 2022
@piaskowyk piaskowyk deleted the @tomekzaw/fix-android-crash-worklet-error branch October 12, 2022 09:29
piaskowyk pushed a commit that referenced this pull request Oct 12, 2022
This PR improves developer experience of error handling in worklets by
showing a RedBox with full error message rather than crashing the whole
app on Android.

**Note:** Android with Fabric enabled works only if building with NDK 21
via `yarn react-native run-android [--active-arch-only]`, crashes when
building with NDK 24 from Android Studio, see
Shopify/flash-list#550 (comment)
for details (this is not an issue with Reanimated)

<table>
<thead>
<tr>
<th rowspan="2">Scenario</th>
<th colspan="2">Before</th>
<th colspan="2">After</th>
</tr>
<tr>
<th>Paper</th>
<th>Fabric</th>
<th>Paper</th>
<th>Fabric</th>
</thead>
<tbody>
<tr>
<td>worklet</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>nested worklet</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useAnimatedStyle</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useDerivedValue</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useFrameCallback</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>GestureDetector</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useAnimatedGestureHandler</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useAnimatedScrollHandler</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useScrollViewOffset</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
</tbody>
</table>

https://github.com/software-mansion/react-native-reanimated/blob/48af341d51ca289dc007fdfd81d124ae0c267523/FabricExample/android/build.gradle#L11-L17

```diff
-            ndkVersion = "24.0.8215888"
+            ndkVersion = "21.4.7075529"
```

`ERROR: Unknown host CPU architecture: arm64`

```console
code ~/Library/Android/sdk/ndk/21.4.7075529/ndk-build
```

```diff

 DIR="$(cd "$(dirname "$0")" && pwd)"
-$DIR/build/ndk-build "$@"
+arch -x86_64 $DIR/build/ndk-build "$@"
```

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Added TS types tests
- [ ] Added unit / integration tests
- [ ] Updated documentation
- [ ] Ensured that CI passes
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Description

This PR improves developer experience of error handling in worklets by
showing a RedBox with full error message rather than crashing the whole
app on Android.

**Note:** Android with Fabric enabled works only if building with NDK 21
via `yarn react-native run-android [--active-arch-only]`, crashes when
building with NDK 24 from Android Studio, see
Shopify/flash-list#550 (comment)
for details (this is not an issue with Reanimated)

<table>
<thead>
<tr>
<th rowspan="2">Scenario</th>
<th colspan="2">Before</th>
<th colspan="2">After</th>
</tr>
<tr>
<th>Paper</th>
<th>Fabric</th>
<th>Paper</th>
<th>Fabric</th>
</thead>
<tbody>
<tr>
<td>worklet</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>nested worklet</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useAnimatedStyle</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useDerivedValue</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useFrameCallback</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>GestureDetector</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useAnimatedGestureHandler</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useAnimatedScrollHandler</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
<tr>
<td>useScrollViewOffset</td>
<td align="center">💥</td>
<td align="center">💥</td>
<td align="center">✅</td>
<td align="center">✅</td>
</tr>
</tbody>
</table>

## Test code and steps to reproduce

### Building with NDK 21 on M1-based Mac


https://github.com/software-mansion/react-native-reanimated/blob/48af341d51ca289dc007fdfd81d124ae0c267523/FabricExample/android/build.gradle#L11-L17

```diff
-            ndkVersion = "24.0.8215888"
+            ndkVersion = "21.4.7075529"
```

`ERROR: Unknown host CPU architecture: arm64`

```console
code ~/Library/Android/sdk/ndk/21.4.7075529/ndk-build
```

```diff
 #!/bin/sh
 DIR="$(cd "$(dirname "$0")" && pwd)"
-$DIR/build/ndk-build "$@"
+arch -x86_64 $DIR/build/ndk-build "$@"
```

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Added TS types tests
- [ ] Added unit / integration tests
- [ ] Updated documentation
- [ ] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants