From 978b14c3d31962f3f46248e1d49c64bed9438b79 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Fri, 25 Jan 2019 09:50:08 -0800 Subject: [PATCH] Append and show errors which are caused due to jni/native crashes Summary: Before this diff, the crash reporteer plugin for android just used to show messages which were error and had tag as "AndroidRuntime". This diff fixes this and instead accepts multiple different tags which might be related to crashes. The issue with triggering the notification for all kind of messages would be that, the user might get many. For the jni errors, the stack trace of it are shown as multiple entry instead of one entry in logcat. So to combine all the stack trace into one callstack message, this diff adds a timer of 10ms and combines the messages which occur in that timespan and then trigger the notification. Reviewed By: danielbuechele Differential Revision: D13772505 fbshipit-source-id: fec6f5a7f9f46948c9f9dc5b2a7b92690913c8aa --- .../__tests__/crashReporterUtility.node.js | 117 ++++++++++++++++++ src/fb-stubs/crashReporterUtility.js | 27 ++++ src/index.js | 7 +- .../testCrashReporterPlugin.electron.js | 1 + src/plugins/crash_reporter/index.js | 36 ++++-- 5 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 src/fb-stubs/__tests__/crashReporterUtility.node.js create mode 100644 src/fb-stubs/crashReporterUtility.js diff --git a/src/fb-stubs/__tests__/crashReporterUtility.node.js b/src/fb-stubs/__tests__/crashReporterUtility.node.js new file mode 100644 index 000000000..f259bcc8e --- /dev/null +++ b/src/fb-stubs/__tests__/crashReporterUtility.node.js @@ -0,0 +1,117 @@ +/** + * Copyright 2018-present Facebook. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * @format + */ +import {shouldParseAndroidLog} from '../crashReporterUtility.js'; +import type {DeviceLogEntry, LogLevel} from 'flipper'; + +function getAndroidLog( + date: Date, + type: LogLevel, + tag: string, + message: string, +): DeviceLogEntry { + return {date, type, tag, message, app: 'testapp', pid: 0, tid: 0}; +} + +test('test shouldParseAndroidLog function for type error and tag has flipper mention', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'flipper.activitymanager', + 'Possible error', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); +test('test shouldParseAndroidLog function for type non-error', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'debug', + 'flipper.activitymanager', + 'Possible debug info in flipper', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(false); +}); +test('test shouldParseAndroidLog function for the older android log', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() - 10000), //This log arrives 10 secs before the refernce time + 'error', + 'flipper.activitymanager', + 'Possible error info in activitymanager', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(false); +}); +test('test shouldParseAndroidLog function for the error log which does not staisfy our tags check', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'arbitrary tag', + 'Possible error info in activitymanager', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(false); +}); +test('test shouldParseAndroidLog function for the error log which does not staisfy our tags check but has mention of flipper', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'arbitrary tag', + 'Possible error info in flipper', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); +test('test shouldParseAndroidLog function for the error log which has flipper mentioned in the tag', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'arbitrary flipper tag', + 'Possible error info in facebook', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); +test('test shouldParseAndroidLog function for the error log which has tag libfbjni', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'libfbjni', + 'Possible error info in libfbjni', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); +test('test shouldParseAndroidLog function for the error log which has tag art', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'art', + 'Possible error info in wakizashi', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); +test('test shouldParseAndroidLog function for the error log which has tag AndroidRuntime', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'error', + 'AndroidRuntime', + 'Possible error info in app', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); diff --git a/src/fb-stubs/crashReporterUtility.js b/src/fb-stubs/crashReporterUtility.js new file mode 100644 index 000000000..ee0ed93ed --- /dev/null +++ b/src/fb-stubs/crashReporterUtility.js @@ -0,0 +1,27 @@ +/** + * Copyright 2018-present Facebook. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * @format + * @flow + */ +import type {DeviceLogEntry} from '../devices/BaseDevice.js'; + +export function shouldParseAndroidLog( + entry: DeviceLogEntry, + date: Date, +): boolean { + const tagsCheck = + entry.tag === 'art' || // This tag is for jni errors from sample app + entry.tag === 'AndroidRuntime' || // This tag is for runtime java errors + entry.tag === 'libfbjni' || // Related to fbjni errors from flipper + entry.tag.includes('flipper'); + + const messagesCheck = entry.message.includes('flipper'); + + return ( + entry.date.getTime() - date.getTime() > 0 && // The log should have arrived after the device has been registered + entry.type === 'error' && + (tagsCheck || messagesCheck) + ); +} diff --git a/src/index.js b/src/index.js index 2358ac295..dbd7ba1c8 100644 --- a/src/index.js +++ b/src/index.js @@ -26,7 +26,12 @@ export type {Store} from './reducers/index.js'; export { default as SidebarExtensions, } from './fb-stubs/LayoutInspectorSidebarExtensions.js'; -export {DeviceLogListener, DeviceLogEntry} from './devices/BaseDevice.js'; +export { + DeviceLogListener, + DeviceLogEntry, + LogLevel, +} from './devices/BaseDevice.js'; +export {shouldParseAndroidLog} from './fb-stubs/crashReporterUtility.js'; export {createTablePlugin} from './createTablePlugin.js'; export {default as DetailSidebar} from './chrome/DetailSidebar.js'; diff --git a/src/plugins/crash_reporter/__tests__/testCrashReporterPlugin.electron.js b/src/plugins/crash_reporter/__tests__/testCrashReporterPlugin.electron.js index 7e6f9f893..6f640d50d 100644 --- a/src/plugins/crash_reporter/__tests__/testCrashReporterPlugin.electron.js +++ b/src/plugins/crash_reporter/__tests__/testCrashReporterPlugin.electron.js @@ -40,6 +40,7 @@ function getCrash( name: name, }; } + beforeEach(() => { setNotificationID(0); // Resets notificationID to 0 setDefaultPersistedState({crashes: []}); // Resets defaultpersistedstate diff --git a/src/plugins/crash_reporter/index.js b/src/plugins/crash_reporter/index.js index 00c5b5e6b..293603631 100644 --- a/src/plugins/crash_reporter/index.js +++ b/src/plugins/crash_reporter/index.js @@ -20,6 +20,7 @@ import { getPluginKey, getPersistedState, BaseDevice, + shouldParseAndroidLog, } from 'flipper'; import fs from 'fs'; import os from 'os'; @@ -342,17 +343,32 @@ export default class CrashReporterPlugin extends FlipperDevicePlugin< newPluginState: ?PersistedState, ) => void, ) { + let androidLog: string = ''; + let androidLogUnderProcess = false; + let timer = null; baseDevice.addLogListener((entry: DeviceLogEntry) => { - if ( - entry.type === 'error' && - entry.tag === 'AndroidRuntime' && - entry.date.getTime() - date.getTime() > 0 - ) { - parseCrashLogAndUpdateState( - store, - entry.message, - setPersistedState, - ); + if (shouldParseAndroidLog(entry, referenceDate)) { + if (androidLogUnderProcess) { + androidLog += '\n' + entry.message; + androidLog = androidLog.trim(); + if (timer) { + clearTimeout(timer); + } + } else { + androidLog = entry.message; + androidLogUnderProcess = true; + } + timer = setTimeout(() => { + if (androidLog.length > 0) { + parseCrashLogAndUpdateState( + store, + androidLog, + setPersistedState, + ); + } + androidLogUnderProcess = false; + androidLog = ''; + }, 50); } }); })(store, referenceDate, setPersistedState);