Skip to content

Commit

Permalink
Unify error messages and add Troubleshooting section to docs (#4915)
Browse files Browse the repository at this point in the history
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Since version checker PR #4914 will add more error messages to
Reanimated and our Troubleshooting section in docs was almost
non-existent I decided to add a new one.

To make it consistent I made all errors thrown by Reanimated start with
`[Reanimated]` and prettified them a bit.

With this I made this tiny script `validate-urls.js` that scans our
source code to get valid (not so exotic) URLs and checks if they are
available.

## Test plan

CI
  • Loading branch information
tjzel committed Aug 22, 2023
1 parent eb250df commit 8ef27c2
Show file tree
Hide file tree
Showing 67 changed files with 495 additions and 212 deletions.
3 changes: 2 additions & 1 deletion Common/cpp/NativeModules/NativeReanimatedModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ jsi::Value NativeReanimatedModule::makeShareableClone(
shareable =
std::make_shared<ShareableString>(value.getSymbol(rt).toString(rt));
} else {
throw std::runtime_error("attempted to convert an unsupported value type");
throw std::runtime_error(
"[Reanimated] Attempted to convert an unsupported value type.");
}
return ShareableJSRef::newHostObject(rt, shareable);
}
Expand Down
16 changes: 8 additions & 8 deletions Common/cpp/ReanimatedRuntime/ReanimatedHermesRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ using namespace react;

// ReentrancyCheck is copied from React Native
// from ReactCommon/hermes/executor/HermesExecutorFactory.cpp
// https://github.com/facebook/react-native/blob/main/ReactCommon/hermes/executor/HermesExecutorFactory.cpp
// https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp
struct ReanimatedReentrancyCheck {
// This is effectively a very subtle and complex assert, so only
// include it in builds which would include asserts.
// This is effectively a very subtle and complex assert, so only
// include it in builds which would include asserts.
#ifndef NDEBUG
ReanimatedReentrancyCheck() : tid(std::thread::id()), depth(0) {}

Expand Down Expand Up @@ -67,13 +67,13 @@ struct ReanimatedReentrancyCheck {
// Returns true if tid and expected were the same. If they
// were, then the stored tid referred to no thread, and we
// atomically saved this thread's tid. Now increment depth.
assert(depth == 0 && "No thread id, but depth != 0");
assert(depth == 0 && "[Reanimated] No thread id, but depth != 0");
++depth;
} else if (expected == this_id) {
// If the stored tid referred to a thread, expected was set to
// that value. If that value is this thread's tid, that's ok,
// just increment depth again.
assert(depth != 0 && "Thread id was set, but depth == 0");
assert(depth != 0 && "[Reanimated] Thread id was set, but depth == 0");
++depth;
} else {
// The stored tid was some other thread. This indicates a bad
Expand All @@ -87,13 +87,13 @@ struct ReanimatedReentrancyCheck {
void after() {
assert(
tid.load(std::memory_order_relaxed) == std::this_thread::get_id() &&
"No thread id in after()");
"[Reanimated] No thread id in after()");
if (--depth == 0) {
// If we decremented depth to zero, store no-thread into tid.
std::thread::id expected = std::this_thread::get_id();
bool didWrite = tid.compare_exchange_strong(
expected, std::thread::id(), std::memory_order_relaxed);
assert(didWrite && "Decremented to zero, but no tid write");
assert(didWrite && "[Reanimated] Decremented to zero, but no tid write");
}
}

Expand All @@ -110,7 +110,7 @@ struct ReanimatedReentrancyCheck {
// jsi::Runtime. So the inheritance is: ReanimatedHermesRuntime ->
// WithRuntimeDecorator -> DecoratedRuntime -> jsi::Runtime You can find out
// more about this in ReactCommon/jsi/jsi/Decorator.h or by following this link:
// https://github.com/facebook/react-native/blob/main/ReactCommon/jsi/jsi/decorator.h
// https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/jsi/jsi/decorator.h
class ReanimatedHermesRuntime
: public jsi::WithRuntimeDecorator<ReanimatedReentrancyCheck> {
public:
Expand Down
13 changes: 5 additions & 8 deletions Common/cpp/ReanimatedRuntime/RuntimeInitialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ _Last updated_: 13/09/2022 by @Kwasow

This document describes the current way of initializing Hermes and connecting
it to the debugger. The work I did was mainly based on
[HermesExecutorFactory.cpp](https://github.com/facebook/react-native/blob/main/ReactCommon/hermes/executor/HermesExecutorFactory.cpp)

[HermesExecutorFactory.cpp](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp)
from React Native.

## Runtime initalization
## Runtime initialization

If you take a look at `NativeProxy` (both on Android and iOS) you'll find
that it only makes a call to `ReanimatedRuntime::make(jsQueue)`. This
Expand Down Expand Up @@ -35,6 +36,7 @@ appended to the source code. The generated source map will be a base64 encoded
json.

A workletized function would look like this (after formattings):

```js
function _f(number) {
console.log(_WORKLET, number);
Expand All @@ -48,12 +50,7 @@ And the base64 string after decoding is:
{
"version": 3,
"mappings": "AAasB,SAACA,EAAD,CAACA,MAAD,EAAoB;AAEtCC,SAAO,CAACC,GAARD,CAAYE,QAAZF,EAAsBD,MAAtBC;AAFkB",
"names": [
"number",
"console",
"log",
"_WORKLET"
],
"names": ["number", "console", "log", "_WORKLET"],
"sources": [
"/Users/karol/Git/react-native-reanimated/FabricExample/src/WorkletExample.tsx"
],
Expand Down
7 changes: 2 additions & 5 deletions Common/cpp/SharedItems/Shareables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ std::unique_ptr<jsi::Function> &CoreFunction::getFunction(jsi::Runtime &rt) {
std::shared_ptr<Shareable> extractShareableOrThrow(
jsi::Runtime &rt,
const jsi::Value &maybeShareableValue,
const char *errorMessage) {
const std::string &errorMessage) {
if (maybeShareableValue.isObject()) {
auto object = maybeShareableValue.asObject(rt);
if (object.isHostObject<ShareableJSRef>(rt)) {
Expand All @@ -53,10 +53,7 @@ std::shared_ptr<Shareable> extractShareableOrThrow(
} else if (maybeShareableValue.isUndefined()) {
return Shareable::undefined();
}
throw std::runtime_error(
errorMessage != nullptr
? errorMessage
: "expecting the object to be of type ShareableJSRef");
throw std::runtime_error("[Reanimated] " + errorMessage);
}

Shareable::~Shareable() {}
Expand Down
15 changes: 7 additions & 8 deletions Common/cpp/SharedItems/Shareables.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,20 +129,19 @@ class ShareableJSRef : public jsi::HostObject {
std::shared_ptr<Shareable> extractShareableOrThrow(
jsi::Runtime &rt,
const jsi::Value &maybeShareableValue,
const char *errorMessage = nullptr);
const std::string &errorMessage =
"expecting the object to be of type ShareableJSRef.");

template <typename T>
std::shared_ptr<T> extractShareableOrThrow(
jsi::Runtime &rt,
const jsi::Value &shareableRef,
const char *errorMessage = nullptr) {
const std::string &errorMessage =
"provided shareable object is of an incompatible type.") {
auto res = std::dynamic_pointer_cast<T>(
extractShareableOrThrow(rt, shareableRef, errorMessage));
if (!res) {
throw std::runtime_error(
errorMessage != nullptr
? errorMessage
: "provided shareable object is of an incompatible type");
throw std::runtime_error("[Reanimated] " + errorMessage);
}
return res;
}
Expand Down Expand Up @@ -308,7 +307,7 @@ class ShareableHandle : public Shareable {
remoteValue_ = std::make_unique<jsi::Value>(
runtimeHelper_->valueUnpacker->call(rt, initObj));
initializer_ = nullptr; // we can release ref to initializer as this
// method should be called at most once
// method should be called at most once
}
return jsi::Value(rt, *remoteValue_);
}
Expand Down Expand Up @@ -401,7 +400,7 @@ class ShareableScalar : public Shareable {
return jsi::Value(data_.number);
default:
throw std::runtime_error(
"attempted to convert object that's not of a scalar type");
"[Reanimated] Attempted to convert object that's not of a scalar type.");
}
}

Expand Down
2 changes: 1 addition & 1 deletion Common/cpp/Tools/JSISerializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ std::string JSISerializer::stringifyJSIValueRecursively(
return stringifyObject(object);
}

throw std::runtime_error("unsupported value type");
throw std::runtime_error("[Reanimated] Unsupported value type.");
}

std::string stringifyJSIValue(jsi::Runtime &rt, const jsi::Value &value) {
Expand Down
2 changes: 1 addition & 1 deletion Common/cpp/Tools/JsiUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ inline int get<int>(jsi::Runtime &, const jsi::Value *value) {
template <>
inline bool get<bool>(jsi::Runtime &, const jsi::Value *value) {
if (!value->isBool()) {
throw jsi::JSINativeException("Expected a boolean");
throw jsi::JSINativeException("[Reanimated] Expected a boolean.");
}
return value->getBool();
}
Expand Down
2 changes: 1 addition & 1 deletion Common/cpp/Tools/SingleInstanceChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ SingleInstanceChecker<T>::SingleInstanceChecker() {
// to co-exist during a reload.
assertWithMessage(
instanceCount_ <= 1,
"More than one instance of " + className +
"[Reanimated] More than one instance of " + className +
" present. This may indicate a memory leak due to a retain cycle.");

instanceCount_++;
Expand Down
2 changes: 1 addition & 1 deletion Common/cpp/hidden_headers/Logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Logger {
template <typename T>
static void log(T value) {
if (instance == nullptr) {
throw std::runtime_error("no logger specified");
throw std::runtime_error("[Reanimated] No logger specified.");
}
instance->log(value);
}
Expand Down
1 change: 0 additions & 1 deletion RNReanimated.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ require_relative './scripts/reanimated_utils'
reanimated_package_json = JSON.parse(File.read(File.join(__dir__, "package.json")))
config = find_config()
assert_no_multiple_instances(config)
assert_no_reanimated2_with_new_architecture(reanimated_package_json)
assert_latest_react_native_with_new_architecture(config, reanimated_package_json)

fabric_enabled = ENV['RCT_NEW_ARCH_ENABLED'] == '1'
Expand Down
4 changes: 1 addition & 3 deletions __tests__/matrixUtils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,7 @@ describe('Matrix util functions', () => {
[1, 1, 1, 1],
[1, 1, 1, 0],
];
expect(() => decomposeMatrix(incorrectMatrix)).toThrowError(
new Error('Invalid transform matrix!')
);
expect(() => decomposeMatrix(incorrectMatrix)).toThrowError();
});

it('Decompose identity into identities', () => {
Expand Down
31 changes: 11 additions & 20 deletions android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ def resolveReactNativeDirectory() {
}

throw new GradleException(
"[Reanimated] Unable to resolve react-native location in " +
"node_modules. You should project extension property (in app/build.gradle) " +
"`REACT_NATIVE_NODE_MODULES_DIR` with path to react-native."
"[Reanimated] Unable to resolve react-native location in node_modules. You should project extension property (in `app/build.gradle`) `REACT_NATIVE_NODE_MODULES_DIR` with path to react-native."
)
}

Expand All @@ -109,7 +107,7 @@ def getPlaygroundAppName() { // only for the development
}
})
} catch (_) {
throw new GradleException("Couldn't determine playground app name.")
throw new GradleException("[Reanimated] Couldn't determine playground app name.")
}
return playgroundAppName
}
Expand Down Expand Up @@ -258,7 +256,7 @@ android {
targetSdkVersion safeExtGet("targetSdkVersion", 30)
versionCode 1
versionName "1.0"
buildConfigField "boolean", "IS_NEW_ARCHITECTURE_ENABLED", IS_NEW_ARCHITECTURE_ENABLED.toString()
buildConfigField("boolean", "IS_NEW_ARCHITECTURE_ENABLED", IS_NEW_ARCHITECTURE_ENABLED.toString())
externalNativeBuild {
cmake {
arguments "-DANDROID_STL=c++_shared",
Expand Down Expand Up @@ -427,12 +425,7 @@ abstract class NoMultipleInstancesAssertionTask extends DefaultTask {
String parsedLocation = files.stream().map({
File file -> "- " + file.toString().replace("/package.json", "")
}).collect().join("\n")
String exceptionMessage = "\n[react-native-reanimated] Multiple versions of Reanimated " +
"were detected. Only one instance of react-native-reanimated can be installed in a " +
"project. You need to resolve the conflict manually. Check out the documentation: " +
"https://docs.swmansion.com/react-native-reanimated/docs/fundamentals/" +
"troubleshooting#multiple-versions-of-reanimated-were-detected \n\nConflict " +
"between: \n" + parsedLocation + "\n"
String exceptionMessage = "\n[Reanimated] Multiple versions of Reanimated were detected in `build.gradle`. See `https://docs.swmansion.com/react-native-reanimated/docs/guides/troubleshooting#multiple-versions-of-reanimated-were-detected-in-buildgradle` for more details.\n\nConflict between: \n" + parsedLocation + "\n"
throw new GradleException(exceptionMessage)
}
}
Expand All @@ -449,9 +442,7 @@ def assertLatestReactNativeWithNewArchitecture = task assertLatestReactNativeWit
onlyIf { IS_NEW_ARCHITECTURE_ENABLED && REANIMATED_MAJOR_VERSION == 3 && REACT_NATIVE_MINOR_VERSION < 72 }
doFirst {
throw new GradleException(
"\n[react-native-reanimated] Reanimated " + REANIMATED_VERSION + " supports the New Architecture " +
"only on the latest minor release of React Native. Please upgrade to React Native 0.72.0+ " +
"or downgrade to an older version of Reanimated v3."
"\n[Reanimated] Outdated version of React Native for New Architecture. Reanimated " + REANIMATED_VERSION + " supports the New Architecture on React Native 0.72.0+. See https://docs.swmansion.com/react-native-reanimated/docs/guides/Troubleshooting#outdated-version-of-react-native-for-new-architecture for more information."
)
}
}
Expand Down Expand Up @@ -716,7 +707,7 @@ if (REACT_NATIVE_MINOR_VERSION < 71) {
"${resolveBuildType()}.aar"
)
if (!hermesAAR.exists()) {
throw new GradleException("Could not find hermes-engine AAR", null)
throw new GradleException("[Reanimated] Could not find hermes-engine AAR.", null)
}

def soFiles = zipTree(hermesAAR).matching({ it.include "**/*.so" })
Expand All @@ -731,12 +722,12 @@ if (REACT_NATIVE_MINOR_VERSION < 71) {
doLast {
def hermesPackagePath = findNodeModulePath(projectDir, "hermes-engine")
if (!hermesPackagePath) {
throw new GradleException("Could not find the hermes-engine npm package", null)
throw new GradleException("[Reanimated] Could not find the hermes-engine npm package.", null)
}

def hermesAAR = file("$hermesPackagePath/android/hermes-${resolveBuildType()}.aar") // e.g. hermes-debug.aar
if (!hermesAAR.exists()) {
throw new GradleException("The hermes-engine npm package is missing \"android/hermes-${resolveBuildType()}.aar\"", null)
throw new GradleException("[Reanimated] The hermes-engine npm package is missing \"android/hermes-${resolveBuildType()}.aar\".", null)
}

def soFiles = zipTree(hermesAAR).matching({ it.include "**/*.so" })
Expand All @@ -757,12 +748,12 @@ if (REACT_NATIVE_MINOR_VERSION < 71) {
doLast {
def jscPackagePath = findNodeModulePath(projectDir, "jsc-android")
if (!jscPackagePath) {
throw new GradleException("Could not find the jsc-android npm package", null)
throw new GradleException("[Reanimated] Could not find the jsc-android npm package.", null)
}

def jscDist = file("$jscPackagePath/dist")
if (!jscDist.exists()) {
throw new GradleException("The jsc-android npm package is missing its \"dist\" directory", null)
throw new GradleException("[Reanimated] The jsc-android npm package is missing its \"dist\" directory.", null)
}

def jscAAR = fileTree(jscDist).matching({ it.include "**/android-jsc/**/*.aar" }).singleFile
Expand Down Expand Up @@ -938,6 +929,6 @@ afterEvaluate {
extractSOFiles.dependsOn(prepareJSC)
}
} else {
throw GradleScriptException("Unknown JS runtime ${JS_RUNTIME}.")
throw GradleScriptException("[Reanimated] Unknown JS runtime ${JS_RUNTIME}.")
}
}
4 changes: 2 additions & 2 deletions android/src/main/cpp/NativeProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ void NativeProxy::installJSIBindings(
auto &rnRuntime = *rnRuntime_;
auto isReducedMotion = getIsReducedMotion();
RuntimeDecorator::decorateRNRuntime(rnRuntime, uiRuntime, isReducedMotion);

registerEventHandler();
setupLayoutAnimations();

Expand Down Expand Up @@ -508,7 +507,8 @@ void NativeProxy::setupLayoutAnimations() {
yogaValues.setProperty(rt, key, value);
}
} catch (std::invalid_argument e) {
throw std::runtime_error("Failed to convert value to number");
throw std::runtime_error(
"[Reanimated] Failed to convert value to number.");
}
}
nativeReanimatedModule->layoutAnimationsManager()
Expand Down
6 changes: 3 additions & 3 deletions android/src/main/cpp/NativeProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,9 @@ class NativeProxy : public jni::HybridClass<NativeProxy> {
std::shared_ptr<UIManager> uiManager_;
std::shared_ptr<ReanimatedCommitHook> commitHook_;

// removed temporary, new event listener mechanism need fix on the RN side
// std::shared_ptr<facebook::react::Scheduler> reactScheduler_;
// std::shared_ptr<EventListener> eventListener_;
// removed temporary, new event listener mechanism need fix on the RN side
// std::shared_ptr<facebook::react::Scheduler> reactScheduler_;
// std::shared_ptr<EventListener> eventListener_;
#endif
#ifdef RCT_NEW_ARCH_ENABLED
void installJSIBindings(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ private static WritableArray copyReadableArray(ReadableArray array) {
copy.pushArray(copyReadableArray(array.getArray(i)));
break;
default:
throw new IllegalStateException("Unknown type of ReadableArray");
throw new IllegalStateException("[Reanimated] Unknown type of ReadableArray.");
}
}
return copy;
Expand Down Expand Up @@ -462,7 +462,7 @@ private static void addProp(WritableMap propMap, String key, Object value) {
propMap.putMap(key, (ReadableMap) value);
}
} else {
throw new IllegalStateException("Unknown type of animated value");
throw new IllegalStateException("[Reanimated] Unknown type of animated value.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import java.util.concurrent.Future;

// This class is an almost exact copy of MessageQueueThreadImpl taken from here:
// https://github.com/facebook/react-native/blob/main/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java
// https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java
// The only method that has changed is `quitSynchronous()` (see comment above
// function implementation for details).
@DoNotStrip
Expand Down
Loading

0 comments on commit 8ef27c2

Please sign in to comment.