Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture trace of error reporting thread and identify with boolean flag #355

Merged
merged 17 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f7961fb
feat: capture trace of error reporting thread and identify it with bo…
fractalwrench Aug 2, 2018
60b5934
add changelog entry
fractalwrench Aug 2, 2018
9461b25
feat: pass current thread into threadstate when notifying, rather tha…
fractalwrench Aug 7, 2018
37f4de2
docs: update javadoc for threadstate
fractalwrench Aug 7, 2018
d75f08c
rename thread -> currentThread
fractalwrench Aug 7, 2018
932397f
test: verify that the current thread is not always used in thread state
fractalwrench Aug 7, 2018
213b33b
fix: add the current thread to the thread stacktrace map if it is not…
fractalwrench Aug 9, 2018
dc34285
refactor: factor current thread test logic into separate function
fractalwrench Aug 9, 2018
3949a23
feat: use exception stacktrace when passed into threadstate construct…
fractalwrench Aug 9, 2018
2f83a7c
feat: update exceptionHandler threadstate to record exception stacktr…
fractalwrench Aug 9, 2018
a544429
test: add mazerunner scenario for thread id
fractalwrench Aug 20, 2018
3d3a5c4
build: update gemfile lock
fractalwrench Aug 20, 2018
8f5d352
test: update mazerunner scenario to use latest error reporting step s…
fractalwrench Aug 20, 2018
ddffdef
Merge pull request #362 from bugsnag/thread-id-mazerunner
fractalwrench Aug 20, 2018
36884de
Merge branch 'next' into thread-id
fractalwrench Aug 20, 2018
4ad4542
Merge branch 'next' into thread-id
fractalwrench Aug 22, 2018
4eb87e4
test: add mazerunner scenario for unhandled exception
fractalwrench Sep 25, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 6 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
20 changes: 20 additions & 0 deletions features/error_reporting_thread.feature
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding a scenario for unhandled as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good point.

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"
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
44 changes: 29 additions & 15 deletions sdk/src/androidTest/java/com/bugsnag/android/ErrorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
}

Expand Down Expand Up @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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")
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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());
}

Expand All @@ -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());
}

Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 10 additions & 5 deletions sdk/src/androidTest/java/com/bugsnag/android/NullMetadataTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
}
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion sdk/src/androidTest/java/com/bugsnag/android/ReportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Loading