Skip to content

Commit

Permalink
Wrap NullPointerExceptions when thrown by native code called from JS …
Browse files Browse the repository at this point in the history
…for readability
  • Loading branch information
hsource committed Jun 27, 2023
1 parent e06d99b commit 2cf8c0a
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
17 changes: 17 additions & 0 deletions Libraries/ReactNative/PaperUIManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,23 @@ const UIManagerJS = {
hasViewManagerConfig(viewManagerName: string): boolean {
return getViewManagerConfig(viewManagerName) != null;
},
dispatchViewManagerCommand(
reactTag: number,
commandName: number | string,
commandArgs: any[],
) {
if (typeof reactTag !== 'number') {
let stringifiedArgs = '(failed to stringify)';
try {
stringifiedArgs = JSON.stringify(commandArgs);
} catch (err) {
// Do nothing. We have a default message
}
throw new Error(`dispatchViewManagerCommand: found null reactTag with args ${stringifiedArgs}`);
}

return NativeUIManager.dispatchViewManagerCommand(reactTag, commandName, commandArgs);
},
};

// TODO (T45220498): Remove this.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
import static com.facebook.infer.annotation.Assertions.assertNotNull;
import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;

import android.text.Editable;
import android.text.SpannableStringBuilder;

import androidx.annotation.Nullable;
import com.facebook.debug.holder.PrinterHolder;
import com.facebook.debug.tags.ReactDebugOverlayTags;
Expand Down Expand Up @@ -356,37 +359,44 @@ public void invoke(JSInstance jsInstance, ReadableArray parameters) {
mArgumentExtractors[i].extractArgument(jsInstance, parameters, jsArgumentsConsumed);
jsArgumentsConsumed += mArgumentExtractors[i].getJSArgumentsNeeded();
}
} catch (UnexpectedNativeTypeException e) {
} catch (UnexpectedNativeTypeException | NullPointerException e) {
throw new NativeArgumentsParseException(
e.getMessage()
+ " (constructing arguments for "
+ traceName
+ " at argument index "
+ getAffectedRange(
jsArgumentsConsumed, mArgumentExtractors[i].getJSArgumentsNeeded())
+ ")",
+ ") with parameters "
+ parameters.toArrayList(),
e);
}

try {
mMethod.invoke(mModuleWrapper.getModule(), mArguments);
} catch (IllegalArgumentException ie) {
throw new RuntimeException("Could not invoke " + traceName, ie);
} catch (IllegalAccessException iae) {
throw new RuntimeException("Could not invoke " + traceName, iae);
} catch (IllegalArgumentException | IllegalAccessException e) {
throw new RuntimeException(createInvokeExceptionMessage(traceName, parameters), e);
} catch (InvocationTargetException ite) {
// Exceptions thrown from native module calls end up wrapped in InvocationTargetException
// which just make traces harder to read and bump out useful information
if (ite.getCause() instanceof RuntimeException) {
throw (RuntimeException) ite.getCause();
}
throw new RuntimeException("Could not invoke " + traceName, ite);
throw new RuntimeException(createInvokeExceptionMessage(traceName, parameters), ite);
}
} finally {
SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush();
}
}

/**
* Makes it easier to determine the cause of an error invoking a native method from Javascript
* code by adding the function and parameters.
*/
private static String createInvokeExceptionMessage(String traceName, ReadableArray parameters) {
return "Could not invoke " + traceName + " with parameters " + parameters.toArrayList();
}

/**
* Determines how the method is exported in JavaScript: METHOD_TYPE_ASYNC for regular methods
* METHOD_TYPE_PROMISE for methods that return a promise object to the caller. METHOD_TYPE_SYNC
Expand Down
3 changes: 3 additions & 0 deletions WanderlogPatches.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,6 @@ rebase on upstream's `main`
- Prevent crash when runAnimationStep called with low frameTimeNanos
- Summary: React Native was crashing on OnePlus and Oppo devices. This is a workaround
- Pull request: https://github.com/facebook/react-native/pull/37487
- Wrap NullPointerExceptions when thrown by native code called from JS for readability
- Summary: These crashing exceptions were really hard to debug in Bugsnag since they don't print the method name. We wrap it and add that.
- Pull request: https://github.com/facebook/react-native/pull/38060

0 comments on commit 2cf8c0a

Please sign in to comment.