Skip to content

Commit

Permalink
Fix Android crash when worklet throws error (#3558)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tomekzaw authored and piaskowyk committed Oct 12, 2022
1 parent 907d937 commit 0ac7659
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 18 deletions.
2 changes: 0 additions & 2 deletions Common/cpp/SharedItems/ShareableValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,6 @@ jsi::Value ShareableValue::toJSValue(jsi::Runtime &rt) {
} else {
res = funPtr->call(rt, args, count);
}
} catch (jsi::JSError &e) {
throw e;
} catch (std::exception &e) {
std::string str = e.what();
runtimeManager->errorHandler->setError(str);
Expand Down
7 changes: 3 additions & 4 deletions android/src/main/cpp/AndroidErrorHandler.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "AndroidErrorHandler.h"
#include <fbjni/fbjni.h>
#include <exception>
#include <string>
#include "Logger.h"

Expand All @@ -17,11 +18,9 @@ void AndroidErrorHandler::raiseSpec() {
return;
}

static const auto cls = javaClassStatic();
static auto method = cls->getStaticMethod<void(std::string)>("raise");
method(cls, error->message);

// mark error as handled before this method throws exception
this->error->handled = true;
throw std::runtime_error(this->error->message);
}

std::shared_ptr<Scheduler> AndroidErrorHandler::getScheduler() {
Expand Down
5 changes: 1 addition & 4 deletions android/src/main/cpp/AndroidErrorHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@

namespace reanimated {

class AndroidErrorHandler : public JavaClass<AndroidErrorHandler>,
public ErrorHandler {
class AndroidErrorHandler : public ErrorHandler {
std::shared_ptr<ErrorWrapper> error;
std::shared_ptr<Scheduler> scheduler;
void raiseSpec() override;

public:
static auto constexpr kJavaDescriptor =
"Lcom/swmansion/reanimated/AndroidErrorHandler;";
explicit AndroidErrorHandler(std::shared_ptr<Scheduler> scheduler);
std::shared_ptr<Scheduler> getScheduler() override;
std::shared_ptr<ErrorWrapper> getError() override;
Expand Down

This file was deleted.

0 comments on commit 0ac7659

Please sign in to comment.