diff --git a/CHANGELOG.md b/CHANGELOG.md index 56048c95ca..59cf391af7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 4.X.X (TBD) + +* Capture trace of error reporting thread and identify with boolean flag [#355](https://github.com/bugsnag/bugsnag-android/pull/355) + ## 4.6.1 (2018-08-21) ### Bug fixes diff --git a/Gemfile.lock b/Gemfile.lock index 100922b710..c65ec1f107 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,12 +1,13 @@ GIT remote: git@github.com:bugsnag/maze-runner - revision: f7123450d5a75b719911c6dd3baa0507e6062c2d + revision: 177f27da9966aed2cb87687fa8384d6141d2fa05 specs: bugsnag-maze-runner (1.0.0) cucumber (~> 3.1.0) cucumber-expressions (= 5.0.15) minitest (~> 5.0) rack (~> 2.0.0) + rake (~> 12.3.0) test-unit (~> 3.2.0) GEM @@ -32,17 +33,18 @@ GEM cucumber-tag_expressions (1.1.1) cucumber-wire (0.0.1) diff-lcs (1.3) - gherkin (5.0.0) + gherkin (5.1.0) method_source (0.9.0) minitest (5.11.3) multi_json (1.13.1) multi_test (0.1.2) - power_assert (1.1.1) + power_assert (1.1.3) pry (0.11.3) coderay (~> 1.1.0) method_source (~> 0.9.0) rack (2.0.5) - test-unit (3.2.7) + rake (12.3.1) + test-unit (3.2.8) power_assert PLATFORMS diff --git a/features/error_reporting_thread.feature b/features/error_reporting_thread.feature new file mode 100644 index 0000000000..72ab8ff537 --- /dev/null +++ b/features/error_reporting_thread.feature @@ -0,0 +1,20 @@ +# For handled exceptions, the thread trace should be reported in the threads array. +# For unhandled exceptions, the exception trace should be reported instead. + +Feature: Error Reporting Thread + +Scenario: Only 1 thread is flagged as the error reporting thread for handled exceptions + When I run "HandledExceptionScenario" with the defaults + Then I should receive a request + And the request is a valid for the error reporting API + And the thread with name "main" contains the error reporting flag + And the "method" of stack frame 0 equals "com.bugsnag.android.mazerunner.scenarios.Scenario.generateException" + And the payload field "events.0.threads.0.stacktrace.0.method" ends with "getThreadStackTrace" + +Scenario: Only 1 thread is flagged as the error reporting thread for unhandled exceptions + When I run "UnhandledExceptionScenario" with the defaults + Then I should receive 1 request + And the request is a valid for the error reporting API + And the thread with name "main" contains the error reporting flag + And the "method" of stack frame 0 equals "com.bugsnag.android.mazerunner.scenarios.Scenario.generateException" + And the payload field "events.0.threads.0.stacktrace.0.method" equals "com.bugsnag.android.mazerunner.scenarios.Scenario.generateException" diff --git a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index 6cba832f00..98bea56795 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -66,6 +66,7 @@ internal fun createCustomHeaderDelivery(context: Context): Delivery { internal fun writeErrorToStore(client: Client) { - val error = Error.Builder(Configuration("api-key"), RuntimeException(), null).build() + val error = Error.Builder(Configuration("api-key"), RuntimeException(), null, + Thread.currentThread(), false).build() client.errorStore.write(error) } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BeforeNotifyTest.kt b/sdk/src/androidTest/java/com/bugsnag/android/BeforeNotifyTest.kt index 2a2a82c646..279cc061ff 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BeforeNotifyTest.kt +++ b/sdk/src/androidTest/java/com/bugsnag/android/BeforeNotifyTest.kt @@ -23,7 +23,8 @@ class BeforeNotifyTest { false } - val error = Error.Builder(config, RuntimeException("Test"), null).build() + val error = Error.Builder(config, RuntimeException("Test"), null, + Thread.currentThread(), false).build() beforeNotify.run(error) assertEquals(context, error.context) } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java index d6f5656732..6b84c75f01 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java @@ -71,7 +71,8 @@ public void testWrite() throws Exception { @NonNull private Error writeErrorToStore() { - Error error = new Error.Builder(config, new RuntimeException(), null).build(); + Error error = new Error.Builder(config, new RuntimeException(), + null, Thread.currentThread(), false).build(); errorStore.write(error); return error; } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ErrorTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ErrorTest.java index b58613b5cb..f35bfa1bef 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ErrorTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ErrorTest.java @@ -41,7 +41,7 @@ public class ErrorTest { public void setUp() throws Exception { config = new Configuration("api-key"); RuntimeException exception = new RuntimeException("Example message"); - error = new Error.Builder(config, exception, null).build(); + error = new Error.Builder(config, exception, null, Thread.currentThread(), false).build(); } @After @@ -55,12 +55,14 @@ public void testShouldIgnoreClass() { // Shouldn't ignore classes not in ignoreClasses RuntimeException runtimeException = new RuntimeException("Test"); - Error error = new Error.Builder(config, runtimeException, null).build(); + Error error = new Error.Builder(config, + runtimeException, null, Thread.currentThread(), false).build(); assertFalse(error.shouldIgnoreClass()); // Should ignore errors in ignoreClasses IOException ioException = new IOException("Test"); - error = new Error.Builder(config, ioException, null).build(); + error = new Error.Builder(config, + ioException, null, Thread.currentThread(), false).build(); assertTrue(error.shouldIgnoreClass()); } @@ -91,7 +93,8 @@ public void testBasicSerialization() throws JSONException, IOException { @Test public void testHandledSerialisation() throws Exception { - Error err = new Error.Builder(config, new RuntimeException(), null) + Error err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); @@ -108,7 +111,8 @@ public void testHandledSerialisation() throws Exception { @Test public void testUnhandledSerialisation() throws Exception { - Error err = new Error.Builder(config, new RuntimeException(), null) + Error err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false) .severityReasonType(HandledState.REASON_UNHANDLED_EXCEPTION) .severity(Severity.ERROR) .build(); @@ -126,7 +130,8 @@ public void testUnhandledSerialisation() throws Exception { @Test public void testPromiseRejectionSerialisation() throws Exception { - Error err = new Error.Builder(config, new RuntimeException(), null) + Error err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false) .severityReasonType(HandledState.REASON_PROMISE_REJECTION) .severity(Severity.ERROR) .build(); @@ -144,7 +149,8 @@ public void testPromiseRejectionSerialisation() throws Exception { @Test public void testLogSerialisation() throws Exception { - Error err = new Error.Builder(config, new RuntimeException(), null) + Error err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false) .severityReasonType(HandledState.REASON_LOG) .severity(Severity.WARNING) .attributeValue("warning") @@ -179,7 +185,8 @@ public void testUserSpecifiedSerialisation() throws Exception { @Test public void testStrictModeSerialisation() throws Exception { - Error err = new Error.Builder(config, new RuntimeException(), null) + Error err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false) .severityReasonType(HandledState.REASON_STRICT_MODE) .attributeValue("Test") .build(); @@ -244,7 +251,8 @@ public void testSetSeverity() throws JSONException, IOException { @Test public void testSessionIncluded() throws Exception { Session session = generateSession(); - Error err = new Error.Builder(config, new RuntimeException(), session).build(); + Error err = new Error.Builder(config, + new RuntimeException(), session, Thread.currentThread(), false).build(); JSONObject errorJson = streamableToJson(err); assertNotNull(errorJson); @@ -264,7 +272,8 @@ public void testSessionIncluded() throws Exception { @Test(expected = JSONException.class) public void testSessionExcluded() throws Exception { - Error err = new Error.Builder(config, new RuntimeException(), null).build(); + Error err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false).build(); JSONObject errorJson = streamableToJson(err); assertNotNull(errorJson); @@ -274,10 +283,12 @@ public void testSessionExcluded() throws Exception { @Test public void checkExceptionMessageNullity() throws Exception { String msg = "Foo"; - Error err = new Error.Builder(config, new RuntimeException(msg), null).build(); + Error err = new Error.Builder(config, + new RuntimeException(msg), null, Thread.currentThread(), false).build(); assertEquals(msg, err.getExceptionMessage()); - err = new Error.Builder(config, new RuntimeException(), null).build(); + err = new Error.Builder(config, + new RuntimeException(), null, Thread.currentThread(), false).build(); assertEquals("", err.getExceptionMessage()); } @@ -298,7 +309,8 @@ public void testSendThreadsDisabled() throws Exception { public void testBugsnagExceptionName() throws Exception { BugsnagException exception = new BugsnagException("Busgang", "exceptional", new StackTraceElement[]{}); - Error err = new Error.Builder(config, exception, null).build(); + Error err = new Error.Builder(config, + exception, null, Thread.currentThread(), false).build(); assertEquals("Busgang", err.getExceptionName()); } @@ -357,7 +369,8 @@ public void testSetUser() throws Exception { @Test public void testBuilderMetaData() { Configuration config = new Configuration("api-key"); - Error.Builder builder = new Error.Builder(config, new RuntimeException("foo"), null); + Error.Builder builder = new Error.Builder(config, + new RuntimeException("foo"), null, Thread.currentThread(), false); assertNotNull(builder.metaData(new MetaData()).build()); @@ -401,7 +414,8 @@ public void testBuilderNullSession() throws Throwable { Session session = generateSession(); session.setAutoCaptured(true); - error = new Error.Builder(config, exception, session).build(); + error = new Error.Builder(config, exception, session, + Thread.currentThread(), false).build(); JSONObject errorJson = streamableToJson(error); assertFalse(errorJson.has("session")); diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java index 1bac4006e2..d712e79587 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java @@ -71,7 +71,7 @@ public void testNamedException() throws JSONException, IOException { StackTraceElement element = new StackTraceElement("Class", "method", "Class.java", 123); StackTraceElement[] frames = new StackTraceElement[]{element}; Error error = new Error.Builder(config, "RuntimeException", - "Example message", frames, null).build(); + "Example message", frames, null, Thread.currentThread()).build(); Exceptions exceptions = new Exceptions(config, error.getException()); JSONObject exceptionJson = streamableToJsonArray(exceptions).getJSONObject(0); diff --git a/sdk/src/androidTest/java/com/bugsnag/android/NullMetadataTest.java b/sdk/src/androidTest/java/com/bugsnag/android/NullMetadataTest.java index dbe7c969c5..7ec7736467 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/NullMetadataTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/NullMetadataTest.java @@ -42,20 +42,23 @@ public void tearDown() throws Exception { @Test public void testErrorDefaultMetaData() throws Exception { - Error error = new Error.Builder(config, throwable, null).build(); + Error error = new Error.Builder(config, throwable, null, + Thread.currentThread(), false).build(); validateDefaultMetadata(error.getMetaData()); } @Test public void testSecondErrorDefaultMetaData() throws Exception { Error error = new Error.Builder(config, "RuntimeException", - "Something broke", new StackTraceElement[]{}, null).build(); + "Something broke", new StackTraceElement[]{}, + null, Thread.currentThread()).build(); validateDefaultMetadata(error.getMetaData()); } @Test public void testErrorSetMetadataRef() throws Exception { - Error error = new Error.Builder(config, throwable, null).build(); + Error error = new Error.Builder(config, throwable, null, + Thread.currentThread(), false).build(); MetaData metaData = new MetaData(); metaData.addToTab(TAB_KEY, "test", "data"); error.setMetaData(metaData); @@ -64,7 +67,8 @@ public void testErrorSetMetadataRef() throws Exception { @Test public void testErrorSetNullMetadata() throws Exception { - Error error = new Error.Builder(config, throwable, null).build(); + Error error = new Error.Builder(config, throwable, null, + Thread.currentThread(), false).build(); error.setMetaData(null); validateDefaultMetadata(error.getMetaData()); } @@ -97,7 +101,8 @@ public boolean run(Error error) { return false; } }); - Error error = new Error.Builder(config, new Throwable(), null).build(); + Error error = new Error.Builder(config, new Throwable(), + null, Thread.currentThread(), false).build(); Client client = Bugsnag.getClient(); client.notify(error, DeliveryStyle.SAME_THREAD, null); } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ReportTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ReportTest.java index 1ec84e37bd..10e4a76e52 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ReportTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ReportTest.java @@ -31,7 +31,8 @@ public class ReportTest { public void setUp() throws Exception { Configuration config = new Configuration("example-api-key"); RuntimeException exception = new RuntimeException("Something broke"); - Error error = new Error.Builder(config, exception, null).build(); + Error error = new Error.Builder(config, exception, null, + Thread.currentThread(), false).build(); report = new Report("api-key", error); } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt b/sdk/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt new file mode 100644 index 0000000000..65ebf9d193 --- /dev/null +++ b/sdk/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt @@ -0,0 +1,160 @@ +package com.bugsnag.android + +import android.support.test.filters.SmallTest +import android.support.test.runner.AndroidJUnit4 +import com.bugsnag.android.BugsnagTestUtils.streamableToJsonArray +import org.json.JSONArray +import org.json.JSONObject +import org.junit.Assert.* +import org.junit.Test +import org.junit.runner.RunWith + + +@RunWith(AndroidJUnit4::class) +@SmallTest +class ThreadStateTest { + + private val configuration = Configuration("api-key") + private val threadState = ThreadState(configuration, Thread.currentThread(), Thread.getAllStackTraces(), null) + private val json = streamableToJsonArray(threadState) + + /** + * Verifies that the required values for 'thread' are serialised as an array + */ + @Test + fun testSerialisation() { + for (k in 0 until json.length()) { + val thread = json[k] as JSONObject + assertNotNull(thread.getString("id")) + assertNotNull(thread.getString("name")) + assertNotNull(thread.getString("stacktrace")) + assertEquals("android", thread.getString("type")) + } + } + + /** + * Verifies that the current thread is serialised as an object, and that only this value + * contains the errorReportingThread boolean flag + */ + @Test + fun testCurrentThread() { + verifyCurrentThreadStructure(json, Thread.currentThread().id) + } + + /** + * Verifies that a thread different from the current thread is serialised as an object, + * and that only this value contains the errorReportingThread boolean flag + */ + @Test + fun testDifferentThread() { + val otherThread = Thread.getAllStackTraces() + .filter { it.key != Thread.currentThread() } + .map { it.key } + .first() + + val state = ThreadState(configuration, otherThread, Thread.getAllStackTraces(), null) + val json = streamableToJsonArray(state) + verifyCurrentThreadStructure(json, otherThread.id) + } + + /** + * Verifies that if the current thread is missing from the available traces as reported by + * [Thread.getAllStackTraces], its stacktrace will still be serialised + */ + @Test + fun testMissingCurrentThread() { + val currentThread = Thread.currentThread() + val missingTraces = Thread.getAllStackTraces() + missingTraces.remove(currentThread) + + val state = ThreadState(configuration, currentThread, missingTraces, null) + val json = streamableToJsonArray(state) + + verifyCurrentThreadStructure(json, currentThread.id) { + assertTrue(it.getJSONArray("stacktrace").length() > 0) + } + } + + /** + * Verifies that a handled error uses [Thread] for the reporting thread stacktrace + */ + @Test + fun testHandledStacktrace() { + val currentThread = Thread.currentThread() + val allStackTraces = Thread.getAllStackTraces() + val state = ThreadState(configuration, currentThread, allStackTraces, null) + val json = streamableToJsonArray(state) + + // find the stack trace for the current thread that was passed as a parameter + val expectedTrace = allStackTraces.filter { + it.key.id == currentThread.id + }.map { it.value }.first() + + verifyCurrentThreadStructure(json, currentThread.id) { + + // the thread id + name should always be used + assertEquals(currentThread.name, it.getString("name")) + assertEquals(currentThread.id, it.getLong("id")) + + // stacktrace should come from the thread (check same length and line numbers) + val serialisedTrace = it.getJSONArray("stacktrace") + assertEquals(expectedTrace.size, serialisedTrace.length()) + + expectedTrace.forEachIndexed { index, element -> + val jsonObject = serialisedTrace.getJSONObject(index) + assertEquals(element.lineNumber, jsonObject.getInt("lineNumber")) + } + } + } + + /** + * * Verifies that an unhandled error uses [Exception] for the reporting thread stacktrace + */ + @Test + fun testUnhandledStacktrace() { + val currentThread = Thread.currentThread() + val allStackTraces = Thread.getAllStackTraces() + val exc: Throwable = RuntimeException("Whoops") + val expectedTrace = exc.stackTrace + + val state = ThreadState(configuration, currentThread, allStackTraces, exc) + val json = streamableToJsonArray(state) + + verifyCurrentThreadStructure(json, currentThread.id) { + + // the thread id + name should always be used + assertEquals(currentThread.name, it.getString("name")) + assertEquals(currentThread.id, it.getLong("id")) + + // stacktrace should come from the exception (check different length) + val serialisedTrace = it.getJSONArray("stacktrace") + assertEquals(expectedTrace.size, serialisedTrace.length()) + + expectedTrace.forEachIndexed { index, element -> + val jsonObject = serialisedTrace.getJSONObject(index) + assertEquals(element.lineNumber, jsonObject.getInt("lineNumber")) + } + } + } + + private fun verifyCurrentThreadStructure(json: JSONArray, + currentThreadId: Long, + action: ((thread: JSONObject) -> Unit)? = null) { + var currentThreadCount = 0 + + for (k in 0 until json.length()) { + val thread = json[k] as JSONObject + val threadId = thread.getLong("id") + + if (threadId == currentThreadId) { + assertTrue(thread.getBoolean("errorReportingThread")) + currentThreadCount++ + action?.invoke(thread) + } else { + assertFalse(thread.has("errorReportingThread")) + } + } + assertEquals("Expected one error reporting thread", 1, currentThreadCount) + } + +} diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 66c72330f8..896604b018 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -661,7 +661,8 @@ public void beforeRecordBreadcrumb(BeforeRecordBreadcrumb beforeRecordBreadcrumb * @param exception the exception to send to Bugsnag */ public void notify(@NonNull Throwable exception) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); notify(error, !BLOCKING); @@ -675,7 +676,8 @@ public void notify(@NonNull Throwable exception) { * additional modification */ public void notify(@NonNull Throwable exception, Callback callback) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); notify(error, DeliveryStyle.ASYNC, callback); @@ -695,7 +697,7 @@ public void notify(@NonNull String name, @NonNull StackTraceElement[] stacktrace, Callback callback) { Error error = new Error.Builder(config, name, message, stacktrace, - sessionTracker.getCurrentSession()) + sessionTracker.getCurrentSession(), Thread.currentThread()) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); notify(error, DeliveryStyle.ASYNC, callback); @@ -709,7 +711,8 @@ public void notify(@NonNull String name, * Severity.WARNING or Severity.INFO */ public void notify(@NonNull Throwable exception, Severity severity) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .severity(severity) .build(); notify(error, !BLOCKING); @@ -725,7 +728,8 @@ public void notify(@NonNull Throwable exception, Severity severity) { @Deprecated public void notify(@NonNull Throwable exception, @NonNull MetaData metaData) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .metaData(metaData) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); @@ -744,7 +748,8 @@ public void notify(@NonNull Throwable exception, @Deprecated public void notify(@NonNull Throwable exception, Severity severity, @NonNull MetaData metaData) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .metaData(metaData) .severity(severity) .build(); @@ -768,7 +773,7 @@ public void notify(@NonNull String name, @NonNull String message, @NonNull StackTraceElement[] stacktrace, Severity severity, @NonNull MetaData metaData) { Error error = new Error.Builder(config, name, message, - stacktrace, sessionTracker.getCurrentSession()) + stacktrace, sessionTracker.getCurrentSession(), Thread.currentThread()) .severity(severity) .metaData(metaData) .build(); @@ -796,7 +801,7 @@ public void notify(@NonNull String name, Severity severity, @NonNull MetaData metaData) { Error error = new Error.Builder(config, name, message, - stacktrace, sessionTracker.getCurrentSession()) + stacktrace, sessionTracker.getCurrentSession(), Thread.currentThread()) .severity(severity) .metaData(metaData) .build(); @@ -905,7 +910,8 @@ public void run() { * @param exception the exception to send to Bugsnag */ public void notifyBlocking(@NonNull Throwable exception) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); notify(error, BLOCKING); @@ -919,7 +925,8 @@ public void notifyBlocking(@NonNull Throwable exception) { * additional modification */ public void notifyBlocking(@NonNull Throwable exception, Callback callback) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); notify(error, DeliveryStyle.SAME_THREAD, callback); @@ -939,7 +946,7 @@ public void notifyBlocking(@NonNull String name, @NonNull StackTraceElement[] stacktrace, Callback callback) { Error error = new Error.Builder(config, name, message, - stacktrace, sessionTracker.getCurrentSession()) + stacktrace, sessionTracker.getCurrentSession(), Thread.currentThread()) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .build(); notify(error, DeliveryStyle.SAME_THREAD, callback); @@ -955,7 +962,8 @@ public void notifyBlocking(@NonNull String name, @Deprecated public void notifyBlocking(@NonNull Throwable exception, @NonNull MetaData metaData) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION) .metaData(metaData) .build(); @@ -974,7 +982,8 @@ public void notifyBlocking(@NonNull Throwable exception, @Deprecated public void notifyBlocking(@NonNull Throwable exception, Severity severity, @NonNull MetaData metaData) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession(), + Thread.currentThread(), false) .metaData(metaData) .severity(severity) .build(); @@ -1000,7 +1009,7 @@ public void notifyBlocking(@NonNull String name, Severity severity, @NonNull MetaData metaData) { Error error = new Error.Builder(config, name, message, - stacktrace, sessionTracker.getCurrentSession()) + stacktrace, sessionTracker.getCurrentSession(), Thread.currentThread()) .severity(severity) .metaData(metaData) .build(); @@ -1028,7 +1037,7 @@ public void notifyBlocking(@NonNull String name, Severity severity, @NonNull MetaData metaData) { Error error = new Error.Builder(config, name, message, - stacktrace, sessionTracker.getCurrentSession()) + stacktrace, sessionTracker.getCurrentSession(), Thread.currentThread()) .severity(severity) .metaData(metaData) .build(); @@ -1044,7 +1053,8 @@ public void notifyBlocking(@NonNull String name, * Severity.WARNING or Severity.INFO */ public void notifyBlocking(@NonNull Throwable exception, Severity severity) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, + sessionTracker.getCurrentSession(), Thread.currentThread(), false) .severity(severity) .build(); notify(error, BLOCKING); @@ -1072,7 +1082,8 @@ public void internalClientNotify(@NonNull Throwable exception, Logger.info(msg); @SuppressWarnings("WrongConstant") - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + Error error = new Error.Builder(config, exception, + sessionTracker.getCurrentSession(), Thread.currentThread(), false) .severity(Severity.fromString(severity)) .severityReasonType(severityReason) .attributeValue(logLevel) @@ -1231,8 +1242,9 @@ void deliver(@NonNull Report report, @NonNull Error error) { */ void cacheAndNotify(@NonNull Throwable exception, Severity severity, MetaData metaData, @HandledState.SeverityReason String severityReason, - @Nullable String attributeValue) { - Error error = new Error.Builder(config, exception, sessionTracker.getCurrentSession()) + @Nullable String attributeValue, Thread thread) { + Error error = new Error.Builder(config, exception, + sessionTracker.getCurrentSession(), thread, true) .severity(severity) .metaData(metaData) .severityReasonType(severityReason) diff --git a/sdk/src/main/java/com/bugsnag/android/Error.java b/sdk/src/main/java/com/bugsnag/android/Error.java index 7fd37d761f..e968559495 100644 --- a/sdk/src/main/java/com/bugsnag/android/Error.java +++ b/sdk/src/main/java/com/bugsnag/android/Error.java @@ -399,8 +399,13 @@ static class Builder { @HandledState.SeverityReason private String severityReasonType; - Builder(@NonNull Configuration config, @NonNull Throwable exception, Session session) { - this.threadState = new ThreadState(config); + Builder(@NonNull Configuration config, + @NonNull Throwable exception, + @Nullable Session session, + @NonNull Thread thread, + boolean unhandled) { + Throwable exc = unhandled ? exception : null; + this.threadState = new ThreadState(config, thread, Thread.getAllStackTraces(), exc); this.config = config; this.exception = exception; this.severityReasonType = HandledState.REASON_USER_SPECIFIED; // default @@ -414,8 +419,9 @@ static class Builder { } Builder(@NonNull Configuration config, @NonNull String name, - @NonNull String message, @NonNull StackTraceElement[] frames, Session session) { - this(config, new BugsnagException(name, message, frames), session); + @NonNull String message, @NonNull StackTraceElement[] frames, + Session session, Thread thread) { + this(config, new BugsnagException(name, message, frames), session, thread, false); } Builder severityReasonType(@HandledState.SeverityReason String severityReasonType) { diff --git a/sdk/src/main/java/com/bugsnag/android/ExceptionHandler.java b/sdk/src/main/java/com/bugsnag/android/ExceptionHandler.java index caad55593e..79e2f79b34 100644 --- a/sdk/src/main/java/com/bugsnag/android/ExceptionHandler.java +++ b/sdk/src/main/java/com/bugsnag/android/ExceptionHandler.java @@ -77,12 +77,12 @@ public void uncaughtException(@NonNull Thread thread, @NonNull Throwable throwab StrictMode.setThreadPolicy(StrictMode.ThreadPolicy.LAX); client.cacheAndNotify(throwable, Severity.ERROR, - metaData, severityReason, violationDesc); + metaData, severityReason, violationDesc, thread); StrictMode.setThreadPolicy(originalThreadPolicy); } else { client.cacheAndNotify(throwable, Severity.ERROR, - metaData, severityReason, violationDesc); + metaData, severityReason, violationDesc, thread); } } diff --git a/sdk/src/main/java/com/bugsnag/android/ThreadState.java b/sdk/src/main/java/com/bugsnag/android/ThreadState.java index 2e13daf717..4ae7ba317d 100644 --- a/sdk/src/main/java/com/bugsnag/android/ThreadState.java +++ b/sdk/src/main/java/com/bugsnag/android/ThreadState.java @@ -1,11 +1,11 @@ package com.bugsnag.android; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import java.io.IOException; import java.util.Arrays; import java.util.Comparator; -import java.util.Iterator; import java.util.Map; import java.util.Set; @@ -15,34 +15,39 @@ class ThreadState implements JsonStream.Streamable { private static final String THREAD_TYPE = "android"; - final Configuration config; + private final Configuration config; private final Thread[] threads; private final Map stackTraces; + private final long currentThreadId; - ThreadState(Configuration config) { + public ThreadState(@NonNull Configuration config, + @NonNull Thread currentThread, + @NonNull Map allStackTraces, + @Nullable Throwable exc) { this.config = config; - stackTraces = Thread.getAllStackTraces(); - threads = sanitiseThreads(Thread.currentThread().getId(), stackTraces); + stackTraces = allStackTraces; + + // API 24/25 don't record the currentThread, add it in manually + // https://issuetracker.google.com/issues/64122757 + if (!stackTraces.containsKey(currentThread)) { + stackTraces.put(currentThread, currentThread.getStackTrace()); + } + if (exc != null) { // unhandled errors use the exception trace + stackTraces.put(currentThread, exc.getStackTrace()); + } + + currentThreadId = currentThread.getId(); + threads = sanitiseThreads(stackTraces); } /** - * Returns an array of threads excluding the current thread, sorted by thread id + * Returns an array of threads including the current thread, sorted by thread id * - * @param currentThreadId the current thread id - * @param liveThreads all live threads + * @param liveThreads all live threads */ - private Thread[] sanitiseThreads(long currentThreadId, - Map liveThreads) { + private Thread[] sanitiseThreads(Map liveThreads) { Set threadSet = liveThreads.keySet(); - // remove current thread - for (Iterator iterator = threadSet.iterator(); iterator.hasNext(); ) { - Thread thread = iterator.next(); - if (thread.getId() == currentThreadId) { - iterator.remove(); - } - } - Thread[] threads = threadSet.toArray(new Thread[threadSet.size()]); Arrays.sort(threads, new Comparator() { public int compare(@NonNull Thread lhs, @NonNull Thread rhs) { @@ -63,6 +68,10 @@ public void toStream(@NonNull JsonStream writer) throws IOException { StackTraceElement[] stacktrace = stackTraces.get(thread); writer.name("stacktrace").value(new Stacktrace(config, stacktrace)); + + if (currentThreadId == thread.getId()) { + writer.name("errorReportingThread").value(true); + } writer.endObject(); } writer.endArray();