Skip to content

Commit

Permalink
Implement RCTWarn equivalent on Android
Browse files Browse the repository at this point in the history
Summary:
## Overview
This diff is an RFC to port a logging feature from iOS to Android.

Changelog: [Internal]

## Motivation
On iOS we have the following log functions and behaviors available for logging native warnings and errors:

- **Warnings** (`RCTLogWarn`)
  - Log level 'warn' to console
  - Display warning in LogBox
- **Errors** (`RCTLogError`)
  - Log level 'error' to console
  - Display a native RedBox (needs converted to show a LogBox if available)
- **Logs**
  - We also have `RCTLog`, `RCTTrace`, `RCTAdvice`, `RCTInfo`, which just log to the console.

In Java, we have:
- **Warnings**
  - **None**, added in this diff
- **Errors** (`DevSupportManager.showNewJavaError`)
  - Log level 'error' to console with `FLog.e`
  - Display a native RedBox (needs converted to show a LogBox if available
- **Logs**
  - `ReactSoftException` (crashes the app??)
  - `ReactNoCrashSoftException` (only logs??)
  - Others?

## Details

This diff adds a method to pair with `RCTLogWarn`, `DevSupportManager.showNewJavaWarning`, which will log to the console and show a LogBox warning if LogBox is available.

## Concerns

I have a few concerns/questions about the state of logging on Android:
- Should/can we move all of the logging to it's own class, like how RCTLog works?
- Why does some logging happen on DevSupportManager and some in other classes?
- If we moved it all to it's own class, how could we access the reactContext to call the RCTLog JS module

Reviewed By: JoshuaGross

Differential Revision: D20056394

fbshipit-source-id: 32d57e300685e46da8039fc77cb22b4084acf81a
  • Loading branch information
rickhanlonii authored and facebook-github-bot committed Apr 1, 2020
1 parent ec0c65c commit 52b3105
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ rn_android_library(
react_native_target("java/com/facebook/debug/tags:tags"),
react_native_target("java/com/facebook/react/bridge:bridge"),
react_native_target("java/com/facebook/react/common:common"),
react_native_target("java/com/facebook/react/util:util"),
react_native_target("java/com/facebook/react/common/network:network"),
react_native_target("java/com/facebook/react/devsupport:interfaces"),
react_native_target("java/com/facebook/react/module/annotations:annotations"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@
import android.view.View;
import android.view.ViewGroup;
import androidx.annotation.Nullable;
import com.facebook.common.logging.FLog;
import com.facebook.fbreact.specs.NativeLogBoxSpec;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.devsupport.interfaces.DevSupportManager;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.util.RNLog;

@ReactModule(name = LogBoxModule.NAME)
public class LogBoxModule extends NativeLogBoxSpec {
Expand All @@ -40,9 +39,7 @@ public void run() {
if (mReactRootView == null && mDevSupportManager != null) {
mReactRootView = mDevSupportManager.createRootView("LogBox");
if (mReactRootView == null) {
FLog.e(
ReactConstants.TAG,
"Unable to launch logbox because react was unable to create the root view");
RNLog.e("Unable to launch logbox because react was unable to create the root view");
}
}
}
Expand All @@ -64,8 +61,7 @@ public void run() {
if (mLogBoxDialog == null && mReactRootView != null) {
Activity context = getCurrentActivity();
if (context == null || context.isFinishing()) {
FLog.e(
ReactConstants.TAG,
RNLog.e(
"Unable to launch logbox because react activity "
+ "is not available, here is the error that logbox would've displayed: ");
return;
Expand Down
2 changes: 2 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/util/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ rn_android_library(
"PUBLIC",
],
deps = [
react_native_dep("libraries/fbcore/src/main/java/com/facebook/common/logging:logging"),
react_native_dep("third-party/java/jsr-305:jsr-305"),
react_native_target("java/com/facebook/react/common:common"),
react_native_target("java/com/facebook/react/bridge:bridge"),
],
)
26 changes: 26 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/util/RCTLog.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.util;

import com.facebook.react.bridge.JavaScriptModule;

/**
* JS module interface for RCTLog
*
* <p>The RCTLog module allows for showing native logs in JavaScript.
*/
public interface RCTLog extends JavaScriptModule {

/**
* Send a log to JavaScript.
*
* @param level The level of the log.
* @param message The message to log.
*/
void logIfNoNativeHook(String level, String message);
}
125 changes: 125 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/util/RNLog.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.util;

import android.util.Log;
import com.facebook.common.logging.FLog;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.common.ReactConstants;

/** Logging wrapper for FLog with LogBox support. */
public class RNLog {
public static final int MINIMUM_LEVEL_FOR_UI = Log.WARN;

public static final int LOG = Log.VERBOSE;

public static final int TRACE = Log.DEBUG;

public static final int ADVICE = Log.INFO;

public static final int WARN = Log.WARN;

public static final int ERROR = Log.ERROR;

/**
* Log a log level message tagged as React Native to the console.
*
* @param message The message to log.
*/
public static void l(String message) {
FLog.i(ReactConstants.TAG, message);
}

/**
* Log a trace level message tagged as React Native to the console.
*
* @param message The message to log.
*/
public static void t(String message) {
FLog.i(ReactConstants.TAG, message);
}

/**
* Log a warning level message tagged as React Native to the console. This warning will not be
* shown in LogBox.
*
* @param message The message to log.
*/
public static void a(String message) {
FLog.w(ReactConstants.TAG, "(ADVICE)" + message);
}

/**
* Log a warning level message tagged as React Native to the console and display in the app.
*
* @param context The React context of the application use to display the warning.
* @param message The message to log.
*/
public static void w(ReactContext context, String message) {
RNLog.logInternal(context, message, WARN);
FLog.w(ReactConstants.TAG, message);
}

/**
* Log an error level message tagged as React Native to the console and display in the app.
*
* @param context The React context of the application use to display the error.
* @param message The message to log.
*/
public static void e(ReactContext context, String message) {
RNLog.logInternal(context, message, ERROR);
FLog.e(ReactConstants.TAG, message);
}

/**
* Log an error level message tagged as React Native to the console. This error will not be shown
* in LogBox.
*
* @param message The message to log.
*/
public static void e(String message) {
FLog.e(ReactConstants.TAG, message);
}

private static void logInternal(ReactContext context, String message, int level) {
if (level >= MINIMUM_LEVEL_FOR_UI) {
if (context != null && message != null) {
context.getJSModule(RCTLog.class).logIfNoNativeHook(levelToString(level), message);
}
}
}

private static String levelToString(int level) {
switch (level) {
case LOG:
{
return "log";
}
case TRACE:
{
return "log";
}
case ADVICE:
{
return "warn";
}
case WARN:
{
return "warn";
}
case ERROR:
{
return "error";
}
default:
{
return "none";
}
}
}
}

0 comments on commit 52b3105

Please sign in to comment.