Fix the callstack sent from error reporting runnable

Summary:
The crash logged from `ErrorReporterRunnable` wasn't detailed. For example look at the following video.
Also for some reason the `FlipperConnection` interface doesn't have an argument for crash name. Thus I changed the js side of the plugin to accept `crash.name` to be undefined.

{F155526537}

Reviewed By: passy

Differential Revision: D14852561

fbshipit-source-id: 6daf9847535b4508fa312b4f940b014911aae2e5
This commit is contained in:
Pritesh Nandgaonkar
2019-04-15 05:41:28 -07:00
committed by Facebook Github Bot
parent 9e4a9607b5
commit 5169d318de
7 changed files with 56 additions and 15 deletions

View File

@@ -153,10 +153,13 @@ class JFlipperConnectionImpl : public jni::HybridClass<JFlipperConnectionImpl, J
static void registerNatives() { static void registerNatives() {
registerHybrid({ registerHybrid({
makeNativeMethod("sendObject", JFlipperConnectionImpl::sendObject), makeNativeMethod("sendObject", JFlipperConnectionImpl::sendObject),
makeNativeMethod("sendArray", JFlipperConnectionImpl::sendArray), makeNativeMethod("sendArray", JFlipperConnectionImpl::sendArray),
makeNativeMethod("reportError", JFlipperConnectionImpl::reportError), makeNativeMethod("reportError", JFlipperConnectionImpl::reportError),
makeNativeMethod("receive", JFlipperConnectionImpl::receive), makeNativeMethod(
"reportErrorWithMetadata",
JFlipperConnectionImpl::reportErrorWithMetadata),
makeNativeMethod("receive", JFlipperConnectionImpl::receive),
}); });
} }
@@ -168,8 +171,15 @@ class JFlipperConnectionImpl : public jni::HybridClass<JFlipperConnectionImpl, J
_connection->send(std::move(method), json ? folly::parseJson(json->toJsonString()) : folly::dynamic::object()); _connection->send(std::move(method), json ? folly::parseJson(json->toJsonString()) : folly::dynamic::object());
} }
void reportErrorWithMetadata(
const std::string reason,
const std::string stackTrace) {
_connection->error(reason, stackTrace);
}
void reportError(jni::alias_ref<jni::JThrowable> throwable) { void reportError(jni::alias_ref<jni::JThrowable> throwable) {
_connection->error(throwable->toString(), throwable->getStackTrace()->toString()); _connection->error(
throwable->toString(), throwable->getStackTrace()->toString());
} }
void receive(const std::string method, jni::alias_ref<JFlipperReceiver> receiver) { void receive(const std::string method, jni::alias_ref<JFlipperReceiver> receiver) {

View File

@@ -44,6 +44,9 @@ class FlipperConnectionImpl implements FlipperConnection {
public native void sendArray(String method, FlipperArray params); public native void sendArray(String method, FlipperArray params);
@Override
public native void reportErrorWithMetadata(String reason, String stackTrace);
@Override @Override
public native void reportError(Throwable throwable); public native void reportError(Throwable throwable);

View File

@@ -7,6 +7,9 @@
*/ */
package com.facebook.flipper.core; package com.facebook.flipper.core;
import java.io.PrintWriter;
import java.io.StringWriter;
public abstract class ErrorReportingRunnable implements Runnable { public abstract class ErrorReportingRunnable implements Runnable {
private final FlipperConnection mConnection; private final FlipperConnection mConnection;
@@ -21,7 +24,11 @@ public abstract class ErrorReportingRunnable implements Runnable {
runOrThrow(); runOrThrow();
} catch (Exception e) { } catch (Exception e) {
if (mConnection != null) { if (mConnection != null) {
mConnection.reportError(e); StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw);
e.printStackTrace(pw);
String sStackTrace = sw.toString(); // stack trace as a string
mConnection.reportErrorWithMetadata(e.toString(), sStackTrace);
} }
} finally { } finally {
doFinally(); doFinally();

View File

@@ -26,6 +26,9 @@ public interface FlipperConnection {
*/ */
void send(String method, FlipperArray params); void send(String method, FlipperArray params);
/** Report client error with reason and stacktrace as an argument */
void reportErrorWithMetadata(String reason, String stackTrace);
/** Report client error */ /** Report client error */
void reportError(Throwable throwable); void reportError(Throwable throwable);

View File

@@ -47,6 +47,11 @@ public class FlipperConnectionMock implements FlipperConnection {
paramList.add(params); paramList.add(params);
} }
@Override
public void reportErrorWithMetadata(String reason, String stackTrace) {
errors.add(new Throwable(reason));
}
@Override @Override
public void reportError(Throwable throwable) { public void reportError(Throwable throwable) {
errors.add(throwable); errors.add(throwable);

View File

@@ -318,8 +318,11 @@ public class InspectorFlipperPluginTest {
new FlipperObject.Builder().put("ids", new FlipperArray.Builder().put("test")).build(), new FlipperObject.Builder().put("ids", new FlipperArray.Builder().put("test")).build(),
responder); responder);
assertThat(connection.errors.size(), equalTo(1));
assertThat( assertThat(
connection.errors, CoreMatchers.hasItem(hasThrowableWithMessage("Unexpected null value"))); connection.errors,
CoreMatchers.hasItem(
hasThrowableWithMessage("java.lang.RuntimeException: Unexpected null value")));
} }
private class TestNode { private class TestNode {

View File

@@ -532,15 +532,25 @@ export default class CrashReporterPlugin extends FlipperDevicePlugin<
static getActiveNotifications = ( static getActiveNotifications = (
persistedState: PersistedState, persistedState: PersistedState,
): Array<Notification> => { ): Array<Notification> => {
return persistedState.crashes.map((crash: Crash) => { const filteredCrashes = persistedState.crashes.filter(crash => {
let ignore = !crash.name && !crash.reason;
if (ignore) {
console.error('Ignored the notification for the crash', crash);
}
return ignore;
});
return filteredCrashes.map((crash: Crash) => {
const id = crash.notificationID; const id = crash.notificationID;
const title = `CRASH: ${truncate(crash.name, 50)} Reason: ${truncate( let name: string = crash.name || crash.reason;
crash.reason, let title: string = 'CRASH: ' + truncate(name, 50);
50, title = `${
)}`; name == crash.reason
const callstack = CrashReporterPlugin.trimCallStackIfPossible( ? title
crash.callstack, : title + 'Reason: ' + truncate(crash.reason, 50)
); }`;
const callstack = crash.callstack
? CrashReporterPlugin.trimCallStackIfPossible(crash.callstack)
: 'No callstack available';
const msg = `Callstack: ${truncate(callstack, 200)}`; const msg = `Callstack: ${truncate(callstack, 200)}`;
return { return {
id, id,