From f12226ac005cf5b5f2f29b63b14a9b6d3057c843 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Wed, 30 Jan 2019 06:45:55 -0800 Subject: [PATCH] Just trigger for fatal logs and error logs with tag AndroidRuntime Summary: This diff updates the check which triggers crash notifications. We got a user feedback that crash reporter used to report non fatal crash too and it used to annoy users with bombardment of crash notifications. Reviewed By: passy Differential Revision: D13878272 fbshipit-source-id: 75446f08f806e8f28a6f68953c0a001fd18a7dc0 --- src/index.js | 2 +- .../__tests__/crashReporterUtility.node.js | 80 +++++-------------- .../crashReporterUtility.js | 12 +-- 3 files changed, 21 insertions(+), 73 deletions(-) rename src/{fb-stubs => utils}/__tests__/crashReporterUtility.node.js (53%) rename src/{fb-stubs => utils}/crashReporterUtility.js (54%) diff --git a/src/index.js b/src/index.js index dbd7ba1c8..0fa9ba30c 100644 --- a/src/index.js +++ b/src/index.js @@ -31,7 +31,7 @@ export { DeviceLogEntry, LogLevel, } from './devices/BaseDevice.js'; -export {shouldParseAndroidLog} from './fb-stubs/crashReporterUtility.js'; +export {shouldParseAndroidLog} from './utils/crashReporterUtility.js'; export {createTablePlugin} from './createTablePlugin.js'; export {default as DetailSidebar} from './chrome/DetailSidebar.js'; diff --git a/src/fb-stubs/__tests__/crashReporterUtility.node.js b/src/utils/__tests__/crashReporterUtility.node.js similarity index 53% rename from src/fb-stubs/__tests__/crashReporterUtility.node.js rename to src/utils/__tests__/crashReporterUtility.node.js index f259bcc8e..20363e651 100644 --- a/src/fb-stubs/__tests__/crashReporterUtility.node.js +++ b/src/utils/__tests__/crashReporterUtility.node.js @@ -16,13 +16,13 @@ function getAndroidLog( return {date, type, tag, message, app: 'testapp', pid: 0, tid: 0}; } -test('test shouldParseAndroidLog function for type error and tag has flipper mention', () => { +test('test shouldParseAndroidLog function for type error and tag is AndroidRuntime', () => { 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', + 'AndroidRuntime', + 'Possible runtime crash', ); let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); expect(shouldParseTheLog).toEqual(true); @@ -32,8 +32,8 @@ test('test shouldParseAndroidLog function for type non-error', () => { 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', + 'fb4a.activitymanager', + 'Possible debug info in activitymanager', ); let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); expect(shouldParseTheLog).toEqual(false); @@ -43,75 +43,31 @@ test('test shouldParseAndroidLog function for the older android log', () => { let log: DeviceLogEntry = getAndroidLog( new Date(referenceDate.getTime() - 10000), //This log arrives 10 secs before the refernce time 'error', - 'flipper.activitymanager', + 'fb4a.activitymanager', 'Possible error info in activitymanager', ); let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); expect(shouldParseTheLog).toEqual(false); }); +test('test shouldParseAndroidLog function for the fatal log', () => { + const referenceDate = new Date(); + let log: DeviceLogEntry = getAndroidLog( + new Date(referenceDate.getTime() + 10000), //This log arrives 10 secs after the refernce time + 'fatal', + 'arbitrary tag', + 'Possible error info in activitymanager', + ); + let shouldParseTheLog = shouldParseAndroidLog(log, referenceDate); + expect(shouldParseTheLog).toEqual(true); +}); 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', + 'Possible error info in fb4a', ); 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/utils/crashReporterUtility.js similarity index 54% rename from src/fb-stubs/crashReporterUtility.js rename to src/utils/crashReporterUtility.js index ee0ed93ed..91ffb6de2 100644 --- a/src/fb-stubs/crashReporterUtility.js +++ b/src/utils/crashReporterUtility.js @@ -11,17 +11,9 @@ 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) + ((entry.type === 'error' && entry.tag === 'AndroidRuntime') || + entry.type === 'fatal') ); }