Skip to content

Commit

Permalink
feat: make unhandled overridable on JVM errors
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed Dec 4, 2020
1 parent 675b574 commit 7dfab9c
Show file tree
Hide file tree
Showing 66 changed files with 453 additions and 270 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ After the next major release `bugsnag-android-ndk` will no longer be published,

* Reduce AAR size [#492](https://github.com/bugsnag/bugsnag-android/pull/492)

* Make handledState.isUnhandled() publicly readable [#496](https://github.com/bugsnag/bugsnag-android/pull/496)
* Make severityReason.isUnhandled() publicly readable [#496](https://github.com/bugsnag/bugsnag-android/pull/496)

## 4.14.2 (2019-05-21)

Expand Down
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ../maze-runner
specs:
bugsnag-maze-runner (3.2.0)
bugsnag-maze-runner (3.3.0)
appium_lib (~> 10.2)
cucumber (~> 3.1.2)
cucumber-expressions (~> 6.0.0)
Expand Down Expand Up @@ -64,7 +64,7 @@ GEM
selenium-webdriver (3.142.7)
childprocess (>= 0.5, < 4.0)
rubyzip (>= 1.2.2)
test-unit (3.3.6)
test-unit (3.3.7)
power_assert
tomlrb (1.3.0)
websocket-driver (0.7.3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public class EventPayloadTest {
public void setUp() {
ImmutableConfig config = BugsnagTestUtils.generateImmutableConfig();
RuntimeException exception = new RuntimeException("Something broke");
HandledState handledState = HandledState.newInstance(HandledState.REASON_ANR);
Event event = new Event(exception, config, handledState, NoopLogger.INSTANCE);
SeverityReason severityReason = SeverityReason.newInstance(SeverityReason.REASON_ANR);
Event event = new Event(exception, config, severityReason, NoopLogger.INSTANCE);
event.setApp(generateAppWithState());
event.setDevice(generateDeviceWithState());
eventPayload = new EventPayload("api-key", event, new Notifier());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.bugsnag.android;

import static com.bugsnag.android.HandledState.REASON_HANDLED_EXCEPTION;
import static com.bugsnag.android.SeverityReason.REASON_HANDLED_EXCEPTION;
import static com.bugsnag.android.ImmutableConfigKt.sanitiseConfiguration;

import android.app.ActivityManager;
Expand Down Expand Up @@ -614,9 +614,9 @@ public void notify(@NonNull Throwable exception) {
*/
public void notify(@NonNull Throwable exc, @Nullable OnErrorCallback onError) {
if (exc != null) {
HandledState handledState = HandledState.newInstance(REASON_HANDLED_EXCEPTION);
SeverityReason severityReason = SeverityReason.newInstance(REASON_HANDLED_EXCEPTION);
Metadata metadata = metadataState.getMetadata();
Event event = new Event(exc, immutableConfig, handledState, metadata, logger);
Event event = new Event(exc, immutableConfig, severityReason, metadata, logger);
populateAndNotifyAndroidEvent(event, onError);
} else {
logNull("notify");
Expand All @@ -629,10 +629,10 @@ public void notify(@NonNull Throwable exc, @Nullable OnErrorCallback onError) {
* Should only ever be called from the {@link ExceptionHandler}.
*/
void notifyUnhandledException(@NonNull Throwable exc, Metadata metadata,
@HandledState.SeverityReason String severityReason,
@SeverityReason.SeverityReasonType String severityReason,
@Nullable String attributeValue) {
HandledState handledState
= HandledState.newInstance(severityReason, Severity.ERROR, attributeValue);
SeverityReason handledState
= SeverityReason.newInstance(severityReason, Severity.ERROR, attributeValue);
Metadata data = Metadata.Companion.merge(metadataState.getMetadata(), metadata);
Event event = new Event(exc, immutableConfig, handledState, data, logger);
populateAndNotifyAndroidEvent(event, null);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.bugsnag.android;

import static com.bugsnag.android.HandledState.REASON_PROMISE_REJECTION;
import static com.bugsnag.android.SeverityReason.REASON_PROMISE_REJECTION;

import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
Expand Down Expand Up @@ -45,7 +45,9 @@ void deliver(@NonNull Event event) {
}
}

if (event.isUnhandled()) {
boolean unhandled = event.isUnhandled() || !event.impl.getUnhandledOverridden();

if (unhandled) {
// should only send unhandled errors if they don't terminate the process (i.e. ANRs)
String severityReasonType = event.impl.getSeverityReasonType();
boolean promiseRejection = REASON_PROMISE_REJECTION.equals(severityReasonType);
Expand Down
26 changes: 21 additions & 5 deletions bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ public class Event implements JsonStream.Streamable, MetadataAware, UserAware {

Event(@Nullable Throwable originalError,
@NonNull ImmutableConfig config,
@NonNull HandledState handledState,
@NonNull SeverityReason severityReason,
@NonNull Logger logger) {
this(originalError, config, handledState, new Metadata(), logger);
this(originalError, config, severityReason, new Metadata(), logger);
}

Event(@Nullable Throwable originalError,
@NonNull ImmutableConfig config,
@NonNull HandledState handledState,
@NonNull SeverityReason severityReason,
@NonNull Metadata metadata,
@NonNull Logger logger) {
this(new EventInternal(originalError, config, handledState, metadata), logger);
this(new EventInternal(originalError, config, severityReason, metadata), logger);
}

Event(@NonNull EventInternal impl, @NonNull Logger logger) {
Expand Down Expand Up @@ -289,9 +289,25 @@ public void toStream(@NonNull JsonStream stream) throws IOException {
/**
* Whether the event was a crash (i.e. unhandled) or handled error in which the system
* continued running.
*
* Unhandled errors count towards your stability score. If you don't want certain errors
* to count towards your stability score, you can alter this property through an
* {@link OnErrorCallback}.
*/
public boolean isUnhandled() {
return impl.isUnhandled();
return impl.getUnhandled();
}

/**
* Whether the event was a crash (i.e. unhandled) or handled error in which the system
* continued running.
*
* Unhandled errors count towards your stability score. If you don't want certain errors
* to count towards your stability score, you can alter this property through an
* {@link OnErrorCallback}.
*/
public void setUnhandled(boolean unhandled) {
impl.setUnhandled(unhandled);
}

protected boolean shouldDiscardClass() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import java.io.IOException
internal class EventInternal @JvmOverloads internal constructor(
val originalError: Throwable? = null,
config: ImmutableConfig,
private var handledState: HandledState,
private var severityReason: SeverityReason,
data: Metadata = Metadata()
) : JsonStream.Streamable, MetadataAware, UserAware {

Expand All @@ -16,23 +16,29 @@ internal class EventInternal @JvmOverloads internal constructor(
internal var session: Session? = null

var severity: Severity
get() = handledState.currentSeverity
get() = severityReason.currentSeverity
set(value) {
handledState.currentSeverity = value
severityReason.currentSeverity = value
}

var apiKey: String = config.apiKey
lateinit var app: AppWithState
lateinit var device: DeviceWithState
val isUnhandled: Boolean = handledState.isUnhandled
var breadcrumbs: MutableList<Breadcrumb> = mutableListOf()
var unhandled: Boolean
get() = severityReason.unhandled
set(value) {
severityReason.unhandled = value
}
val unhandledOverridden: Boolean
get() = severityReason.unhandledOverridden

var errors: MutableList<Error> = when (originalError) {
null -> mutableListOf()
else -> Error.createError(originalError, config.projectPackages, config.logger)
}

var threads: MutableList<Thread> = ThreadState(originalError, isUnhandled, config).threads
var threads: MutableList<Thread> = ThreadState(originalError, unhandled, config).threads
var groupingHash: String? = null
var context: String? = null

Expand All @@ -58,6 +64,7 @@ internal class EventInternal @JvmOverloads internal constructor(
return "ANR" == errorClass
}


@Throws(IOException::class)
override fun toStream(writer: JsonStream) {
// Write error basics
Expand All @@ -66,8 +73,8 @@ internal class EventInternal @JvmOverloads internal constructor(
writer.name("metaData").value(metadata)

writer.name("severity").value(severity)
writer.name("severityReason").value(handledState)
writer.name("unhandled").value(handledState.isUnhandled)
writer.name("severityReason").value(severityReason)
writer.name("unhandled").value(severityReason.unhandled)

// Write exception info
writer.name("exceptions")
Expand Down Expand Up @@ -105,12 +112,12 @@ internal class EventInternal @JvmOverloads internal constructor(
}

protected fun updateSeverityInternal(severity: Severity) {
handledState = HandledState.newInstance(handledState.severityReasonType,
severity, handledState.attributeValue)
severityReason = SeverityReason.newInstance(severityReason.severityReasonType,
severity, severityReason.attributeValue)
this.severity = severity
}

fun getSeverityReasonType(): String = handledState.severityReasonType
fun getSeverityReasonType(): String = severityReason.severityReasonType

override fun setUser(id: String?, email: String?, name: String?) {
_user = User(id, email, name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void uncaughtException(@NonNull Thread thread, @NonNull Throwable throwab
}

String severityReason = strictModeThrowable
? HandledState.REASON_STRICT_MODE : HandledState.REASON_UNHANDLED_EXCEPTION;
? SeverityReason.REASON_STRICT_MODE : SeverityReason.REASON_UNHANDLED_EXCEPTION;

if (strictModeThrowable) { // writes to disk on main thread
StrictMode.ThreadPolicy originalThreadPolicy = StrictMode.getThreadPolicy();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.bugsnag.android;

import static com.bugsnag.android.DeliveryHeadersKt.HEADER_INTERNAL_ERROR;
import static com.bugsnag.android.HandledState.REASON_UNHANDLED_EXCEPTION;
import static com.bugsnag.android.SeverityReason.REASON_UNHANDLED_EXCEPTION;

import android.annotation.SuppressLint;
import android.content.Context;
Expand Down Expand Up @@ -51,8 +51,8 @@ class InternalReportDelegate implements EventStore.Delegate {
@Override
public void onErrorIOFailure(Exception exc, File errorFile, String context) {
// send an internal error to bugsnag with no cache
HandledState handledState = HandledState.newInstance(REASON_UNHANDLED_EXCEPTION);
Event err = new Event(exc, immutableConfig, handledState, logger);
SeverityReason severityReason = SeverityReason.newInstance(REASON_UNHANDLED_EXCEPTION);
Event err = new Event(exc, immutableConfig, severityReason, logger);
err.setContext(context);

err.addMetadata(INTERNAL_DIAGNOSTICS_TAB, "canRead", errorFile.canRead());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ public boolean onError(@NonNull Event event) {
@NonNull
public static Event createEvent(@Nullable Throwable exc,
@NonNull Client client,
@NonNull HandledState handledState) {
return new Event(exc, client.getConfig(), handledState, client.logger);
@NonNull SeverityReason severityReason) {
return new Event(exc, client.getConfig(), severityReason, client.logger);
}

@NonNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

final class HandledState implements JsonStream.Streamable {

final class SeverityReason implements JsonStream.Streamable {

@StringDef({REASON_UNHANDLED_EXCEPTION, REASON_STRICT_MODE, REASON_HANDLED_EXCEPTION,
REASON_USER_SPECIFIED, REASON_CALLBACK_SPECIFIED, REASON_PROMISE_REJECTION,
REASON_LOG, REASON_SIGNAL, REASON_ANR})
@Retention(RetentionPolicy.SOURCE)
@interface SeverityReason {
@interface SeverityReasonType {
}

static final String REASON_UNHANDLED_EXCEPTION = "unhandledException";
Expand All @@ -28,7 +27,7 @@ final class HandledState implements JsonStream.Streamable {
static final String REASON_LOG = "log";
static final String REASON_ANR = "anrError";

@SeverityReason
@SeverityReasonType
private final String severityReasonType;

@Nullable
Expand All @@ -37,14 +36,15 @@ final class HandledState implements JsonStream.Streamable {
private final Severity defaultSeverity;
private Severity currentSeverity;
private boolean unhandled;
private final boolean originalUnhandled;

static HandledState newInstance(@SeverityReason String severityReasonType) {
static SeverityReason newInstance(@SeverityReasonType String severityReasonType) {
return newInstance(severityReasonType, null, null);
}

static HandledState newInstance(@SeverityReason String severityReasonType,
@Nullable Severity severity,
@Nullable String attributeValue) {
static SeverityReason newInstance(@SeverityReasonType String severityReasonType,
@Nullable Severity severity,
@Nullable String attributeValue) {

if (severityReasonType.equals(REASON_STRICT_MODE) && Intrinsics.isEmpty(attributeValue)) {
throw new IllegalArgumentException("No reason supplied for strictmode");
Expand All @@ -58,28 +58,29 @@ static HandledState newInstance(@SeverityReason String severityReasonType,
case REASON_UNHANDLED_EXCEPTION:
case REASON_PROMISE_REJECTION:
case REASON_ANR:
return new HandledState(severityReasonType, Severity.ERROR, true, null);
return new SeverityReason(severityReasonType, Severity.ERROR, true, null);
case REASON_STRICT_MODE:
return new HandledState(severityReasonType, Severity.WARNING, true, attributeValue);
return new SeverityReason(severityReasonType, Severity.WARNING, true, attributeValue);
case REASON_HANDLED_EXCEPTION:
return new HandledState(severityReasonType, Severity.WARNING, false, null);
return new SeverityReason(severityReasonType, Severity.WARNING, false, null);
case REASON_USER_SPECIFIED:
case REASON_CALLBACK_SPECIFIED:
return new HandledState(severityReasonType, severity, false, null);
return new SeverityReason(severityReasonType, severity, false, null);
case REASON_LOG:
return new HandledState(severityReasonType, severity, false, attributeValue);
return new SeverityReason(severityReasonType, severity, false, attributeValue);
default:
String msg = String.format("Invalid argument '%s' for severityReason",
severityReasonType);
throw new IllegalArgumentException(msg);
}
}

HandledState(String severityReasonType, Severity currentSeverity, boolean unhandled,
@Nullable String attributeValue) {
SeverityReason(String severityReasonType, Severity currentSeverity, boolean unhandled,
@Nullable String attributeValue) {
this.severityReasonType = severityReasonType;
this.defaultSeverity = currentSeverity;
this.unhandled = unhandled;
this.originalUnhandled = unhandled;
this.attributeValue = attributeValue;
this.currentSeverity = currentSeverity;
}
Expand All @@ -92,10 +93,18 @@ Severity getCurrentSeverity() {
return currentSeverity;
}

public boolean isUnhandled() {
boolean getUnhandled() {
return unhandled;
}

void setUnhandled(boolean unhandled) {
this.unhandled = unhandled;
}

boolean getUnhandledOverridden() {
return unhandled != originalUnhandled;
}

@Nullable
String getAttributeValue() {
return attributeValue;
Expand All @@ -111,7 +120,9 @@ String getSeverityReasonType() {

@Override
public void toStream(@NonNull JsonStream writer) throws IOException {
writer.beginObject().name("type").value(calculateSeverityReasonType());
writer.beginObject()
.name("type").value(calculateSeverityReasonType())
.name("unhandledOverridden").value(getUnhandledOverridden());

if (attributeValue != null) {
String attributeKey = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.jetbrains.annotations.NotNull;

import java.util.Collections;
import java.util.Date;
import java.util.HashMap;

Expand Down Expand Up @@ -35,7 +34,7 @@ static Event generateEvent() {
Event event = new Event(
exc,
BugsnagTestUtils.generateImmutableConfig(),
HandledState.newInstance(HandledState.REASON_HANDLED_EXCEPTION),
SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION),
NoopLogger.INSTANCE
);
event.setApp(generateAppWithState());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ class CallbackStateTest {
@Mock
lateinit var session: Session

private val handledState = HandledState.newInstance(HandledState.REASON_HANDLED_EXCEPTION)
private val handledState = SeverityReason.newInstance(
SeverityReason.REASON_HANDLED_EXCEPTION)
private val event = Event(RuntimeException(), generateImmutableConfig(), handledState, NoopLogger)
private val breadcrumb = Breadcrumb("", NoopLogger)

Expand Down
Loading

0 comments on commit 7dfab9c

Please sign in to comment.