From 5673b0a327be78e5b06ac5b04d6499e265473843 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 6 Aug 2019 17:24:46 +0100 Subject: [PATCH] continue with anr message improvements --- .../java/com/bugsnag/android/ErrorNameTest.kt | 48 ++++++++++ .../java/com/bugsnag/android/ErrorTest.java | 10 --- .../com/bugsnag/android/ExceptionsTest.java | 22 +---- .../com/bugsnag/android/BugsnagException.java | 33 ++++++- .../main/java/com/bugsnag/android/Error.java | 35 +++++--- .../java/com/bugsnag/android/Exceptions.java | 6 +- .../android/AnrDetailsCollectorTest.kt | 87 +++++++++++++++++++ .../bugsnag/android/AnrDetailsCollector.kt | 17 +++- .../java/com/bugsnag/android/AnrPlugin.kt | 9 +- 9 files changed, 211 insertions(+), 56 deletions(-) create mode 100644 bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorNameTest.kt create mode 100644 bugsnag-plugin-android-anr/src/androidTest/java/com/bugsnag/android/AnrDetailsCollectorTest.kt diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorNameTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorNameTest.kt new file mode 100644 index 0000000000..fb33ad73f5 --- /dev/null +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorNameTest.kt @@ -0,0 +1,48 @@ +package com.bugsnag.android + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +class ErrorNameTest { + + private lateinit var error: Error + + @Before + fun setUp() { + val config = Configuration("api-key") + val exception = RuntimeException("Example message", RuntimeException("Another")) + error = Error.Builder(config, exception, null, Thread.currentThread(), false).build() + } + + @Test + fun exceptionTypeConverted() { + assertTrue(error.exception is BugsnagException) + assertTrue(error.exceptions.exception is BugsnagException) + assertTrue(error.exceptions.exception.cause is RuntimeException) + } + + @Test + fun defaultExceptionName() { + assertEquals("java.lang.RuntimeException", error.exceptionName) + } + + @Test + fun defaultExceptionMessage() { + assertEquals("Example message", error.exceptionMessage) + } + + @Test + fun overrideExceptionName() { + error.exceptionName = "Foo" + assertEquals("Foo", error.exceptionName) + } + + @Test + fun overrideExceptionMessage() { + error.exceptionMessage = "Some custom message" + assertEquals("Some custom message", error.exceptionMessage) + } + +} diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorTest.java b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorTest.java index 3629157593..a51b5788af 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorTest.java +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ErrorTest.java @@ -78,16 +78,6 @@ public void testShouldIgnoreClass() { assertTrue(error.shouldIgnoreClass()); } - @Test - public void testGetExceptionName() { - assertEquals("java.lang.RuntimeException", error.getExceptionName()); - } - - @Test - public void testGetExceptionMessage() { - assertEquals("Example message", error.getExceptionMessage()); - } - @Test public void testBasicSerialization() throws JSONException, IOException { error.setAppData(client.getAppData().getAppData()); diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java index 1bb281e41f..cfd59e6880 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java @@ -31,7 +31,8 @@ public void setUp() throws Exception { @Test public void testBasicException() throws JSONException, IOException { - Exceptions exceptions = new Exceptions(config, new RuntimeException("oops")); + RuntimeException oops = new RuntimeException("oops"); + Exceptions exceptions = new Exceptions(config, new BugsnagException(oops)); JSONArray exceptionsJson = streamableToJsonArray(exceptions); assertEquals(1, exceptionsJson.length()); @@ -45,7 +46,7 @@ public void testBasicException() throws JSONException, IOException { @Test public void testCauseException() throws JSONException, IOException { Throwable ex = new RuntimeException("oops", new Exception("cause")); - Exceptions exceptions = new Exceptions(config, ex); + Exceptions exceptions = new Exceptions(config, new BugsnagException(ex)); JSONArray exceptionsJson = streamableToJsonArray(exceptions); assertEquals(2, exceptionsJson.length()); @@ -68,7 +69,7 @@ public void testNamedException() throws JSONException, IOException { Error error = new Error.Builder(config, "RuntimeException", "Example message", frames, BugsnagTestUtils.generateSessionTracker(), Thread.currentThread()).build(); - Exceptions exceptions = new Exceptions(config, error.getException()); + Exceptions exceptions = new Exceptions(config, new BugsnagException(error.getException())); JSONObject exceptionJson = streamableToJsonArray(exceptions).getJSONObject(0); assertEquals("RuntimeException", exceptionJson.get("errorClass")); @@ -79,19 +80,4 @@ public void testNamedException() throws JSONException, IOException { assertEquals("Class.java", stackframeJson.get("file")); assertEquals(123, stackframeJson.get("lineNumber")); } - - @Test - public void testCustomExceptionSerialization() throws JSONException, IOException { - Exceptions exceptions = new Exceptions(config, new CustomException("Failed serialization")); - - JSONObject exceptionJson = streamableToJsonArray(exceptions).getJSONObject(0); - assertEquals("CustomizedException", exceptionJson.get("errorClass")); - assertEquals("Failed serialization", exceptionJson.get("message")); - - JSONObject stackframeJson = exceptionJson.getJSONArray("stacktrace").getJSONObject(0); - assertEquals("MyFile.run", stackframeJson.get("method")); - assertEquals("MyFile.java", stackframeJson.get("file")); - assertEquals(408, stackframeJson.get("lineNumber")); - assertEquals(18, stackframeJson.get("offset")); - } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagException.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagException.java index bc1cd51241..f56684238a 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagException.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagException.java @@ -12,7 +12,8 @@ public class BugsnagException extends Throwable { /** * The name of the exception (used instead of the exception class) */ - private final String name; + private String name; + private String message; private String type; @@ -27,11 +28,24 @@ public BugsnagException(@NonNull String name, @NonNull String message, @NonNull StackTraceElement[] frames) { super(message); - - super.setStackTrace(frames); + setStackTrace(frames); this.name = name; } + BugsnagException(@NonNull Throwable exc) { + super(exc.getMessage()); + + if (exc instanceof BugsnagException) { + this.message = ((BugsnagException) exc).getMessage(); + this.name = ((BugsnagException) exc).getName(); + this.type = ((BugsnagException) exc).getType(); + } else { + this.name = exc.getClass().getName(); + } + setStackTrace(exc.getStackTrace()); + initCause(exc.getCause()); + } + /** * @return The name of the exception (used instead of the exception class) */ @@ -40,6 +54,19 @@ public String getName() { return name; } + public void setName(@NonNull String name) { + this.name = name; + } + + @NonNull + public String getMessage() { + return message != null ? message : super.getMessage(); + } + + public void setMessage(String message) { + this.message = message; + } + String getType() { return type; } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java index 622a688efe..fa04526422 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java @@ -44,18 +44,23 @@ public class Error implements JsonStream.Streamable { private String[] projectPackages; private final Exceptions exceptions; private Breadcrumbs breadcrumbs; - private final Throwable exception; + private final BugsnagException exception; private final HandledState handledState; private final Session session; private final ThreadState threadState; private boolean incomplete = false; - Error(@NonNull Configuration config, @NonNull Throwable exception, + Error(@NonNull Configuration config, @NonNull Throwable exc, HandledState handledState, @NonNull Severity severity, Session session, ThreadState threadState) { this.threadState = threadState; this.config = config; - this.exception = exception; + + if (exc instanceof BugsnagException) { + this.exception = (BugsnagException) exc; + } else { + this.exception = new BugsnagException(exc); + } this.handledState = handledState; this.severity = severity; this.session = session; @@ -311,11 +316,14 @@ public void setMetaData(@NonNull MetaData metaData) { */ @NonNull public String getExceptionName() { - if (exception instanceof BugsnagException) { - return ((BugsnagException) exception).getName(); - } else { - return exception.getClass().getName(); - } + return exception.getName(); + } + + /** + * Sets the class name from the exception contained in this Error report. + */ + public void setExceptionName(@NonNull String exceptionName) { + exception.setName(exceptionName); } /** @@ -323,8 +331,15 @@ public String getExceptionName() { */ @NonNull public String getExceptionMessage() { - String localizedMessage = exception.getLocalizedMessage(); - return localizedMessage != null ? localizedMessage : ""; + String msg = exception.getMessage(); + return msg != null ? msg : ""; + } + + /** + * Sets the message from the exception contained in this Error report. + */ + public void setExceptionMessage(@NonNull String exceptionMessage) { + exception.setMessage(exceptionMessage); } /** diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Exceptions.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Exceptions.java index 692f51bc20..5987bd2071 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Exceptions.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Exceptions.java @@ -8,11 +8,11 @@ * Unwrap and serialize exception information and any "cause" exceptions. */ class Exceptions implements JsonStream.Streamable { - private final Throwable exception; + private final BugsnagException exception; private String exceptionType; private String[] projectPackages; - Exceptions(Configuration config, Throwable exception) { + Exceptions(Configuration config, BugsnagException exception) { this.exception = exception; exceptionType = Configuration.DEFAULT_EXCEPTION_TYPE; projectPackages = config.getProjectPackages(); @@ -39,7 +39,7 @@ public void toStream(@NonNull JsonStream writer) throws IOException { writer.endArray(); } - Throwable getException() { + BugsnagException getException() { return exception; } diff --git a/bugsnag-plugin-android-anr/src/androidTest/java/com/bugsnag/android/AnrDetailsCollectorTest.kt b/bugsnag-plugin-android-anr/src/androidTest/java/com/bugsnag/android/AnrDetailsCollectorTest.kt new file mode 100644 index 0000000000..48a1446c45 --- /dev/null +++ b/bugsnag-plugin-android-anr/src/androidTest/java/com/bugsnag/android/AnrDetailsCollectorTest.kt @@ -0,0 +1,87 @@ +package com.bugsnag.android + +import android.app.ActivityManager +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito +import org.mockito.junit.MockitoJUnitRunner + +@RunWith(MockitoJUnitRunner::class) +class AnrDetailsCollectorTest { + + private companion object { + private const val PID_EXAMPLE = 5902 + } + + private val collector = AnrDetailsCollector() + private val stateInfo = ActivityManager.ProcessErrorStateInfo() + private lateinit var error: Error + + @Mock + lateinit var am: ActivityManager + + @Before + fun setUp() { + stateInfo.pid = PID_EXAMPLE + stateInfo.tag = "com.bugsnag.android.example/.ExampleActivity" + stateInfo.shortMsg = "ANR Input dispatching timed out" + stateInfo.longMsg = "ANR in com.bugsnag.android.example" + + error = Error.Builder( + Configuration("f"), + RuntimeException(), + null, + Thread.currentThread(), + true + ).build() + } + + @Test + fun exceptionReturnsNull() { + Mockito.`when`(am.processesInErrorState).thenThrow(RuntimeException()) + assertNull(collector.captureProcessErrorState(am, 0)) + } + + @Test + fun emptyListReturnsNull() { + Mockito.`when`(am.processesInErrorState).thenReturn(listOf()) + assertNull(collector.captureProcessErrorState(am, 0)) + } + + @Test + fun differentPidReturnsNull() { + Mockito.`when`(am.processesInErrorState).thenReturn(listOf(stateInfo)) + val captureProcessErrorState = collector.captureProcessErrorState(am, PID_EXAMPLE) + assertEquals(stateInfo, captureProcessErrorState) + } + + @Test + fun samePidReturnsObj() { + val second = ActivityManager.ProcessErrorStateInfo() + Mockito.`when`(am.processesInErrorState).thenReturn(listOf(stateInfo, second)) + val captureProcessErrorState = collector.captureProcessErrorState(am, PID_EXAMPLE) + assertEquals(stateInfo, captureProcessErrorState) + } + + @Test + fun errMsgAltered() { + collector.mutateError(error, stateInfo) + assertEquals(stateInfo.shortMsg, error.exceptionName) + } + + @Test + fun contextAltered() { + collector.mutateError(error, stateInfo) + assertEquals(stateInfo.tag, error.context) + } + + @Test + fun anrDetailsAltered() { + collector.mutateError(error, stateInfo) + assertEquals(stateInfo.longMsg, error.exceptionMessage) + } +} diff --git a/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrDetailsCollector.kt b/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrDetailsCollector.kt index 5f2a76544a..0e1c257f76 100644 --- a/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrDetailsCollector.kt +++ b/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrDetailsCollector.kt @@ -6,8 +6,6 @@ import android.content.Context import android.os.Process import android.support.annotation.VisibleForTesting -// TODO unit tests - internal class AnrDetailsCollector { fun collectAnrDetails(ctx: Context): ProcessErrorStateInfo? { @@ -23,7 +21,18 @@ internal class AnrDetailsCollector { */ @VisibleForTesting internal fun captureProcessErrorState(am: ActivityManager, pid: Int): ProcessErrorStateInfo? { - val processes = am.processesInErrorState ?: emptyList() - return processes.firstOrNull { it.pid == pid } + return try { + val processes = am.processesInErrorState ?: emptyList() + processes.firstOrNull { it.pid == pid } + } catch (exc: RuntimeException) { + null + } + } + + internal fun mutateError(error: Error, anrState: ProcessErrorStateInfo) { + // TODO truncate these values? + error.context = anrState.tag + error.exceptionName = anrState.shortMsg + error.exceptionMessage = anrState.longMsg } } diff --git a/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrPlugin.kt b/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrPlugin.kt index daa2ead66b..c1e3c0c43d 100644 --- a/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrPlugin.kt +++ b/bugsnag-plugin-android-anr/src/main/java/com/bugsnag/android/AnrPlugin.kt @@ -47,14 +47,7 @@ internal class AnrPlugin : BugsnagPlugin { Logger.warn("Looping in attempt to capture ANR details") Async.run(this) } else { - Logger.warn("Captured ANR details: $anrDetails") - - // TODO determine how to populate error report - val metaData = error.metaData - metaData.addToTab("ANR", "Message", anrDetails.shortMsg) - metaData.addToTab("ANR", "Component", anrDetails.tag) - metaData.addToTab("ANR", "Details", anrDetails.longMsg) - + collector.mutateError(error, anrDetails) client.notify(error, DeliveryStyle.ASYNC_WITH_CACHE, null) } } catch (exc: InterruptedException) {