Skip to content

Commit

Permalink
feat(delivery): added attemptDeliveryOnCrash to Configuration and…
Browse files Browse the repository at this point in the history
… `ImmutableConfig` and allow a 3 second timeout during crash to attempt delivery
  • Loading branch information
lemnik committed Sep 30, 2022
1 parent 284509e commit e8554c5
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 6 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## TBD

### Enhancements

* Setting `Configuration.attemptDeliveryOnCrash` will cause Bugsnag to attempt error delivery during some crashes.
Use of this feature is discouraged, see the method JavaDoc for more information.
[]()

## 5.26.0 (2022-08-18)

### Enhancements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ internal class ConfigInternal(
var projectPackages: Set<String> = emptySet()
var persistenceDirectory: File? = null

var attemptDeliveryOnCrash: Boolean = false

val notifier: Notifier = Notifier()

protected val plugins = HashSet<Plugin>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import androidx.annotation.VisibleForTesting;

import java.io.File;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -1112,6 +1111,39 @@ public void addPlugin(@NonNull Plugin plugin) {
}
}

/**
* Whether Bugsnag should try to send crashing errors prior to app termination.
*
* Delivery will only be attempted for uncaught Java / Kotlin exceptions or errors, and
* while in progress will block the crashing thread for up to 3 seconds.
*
* Delivery on crash should be considered unreliable due to the necessary short timeout and
* potential for generating "errors on errors".
*
* Use of this feature is discouraged because it:
* - may cause Application Not Responding (ANR) errors on-top of existing crashes
* - will result in duplicate errors in your Dashboard when errors are not detected as sent
* before termination
* - may prevent other error handlers from detecting or reporting a crash
*
* By default this value is {@code false}.
*
* @param attemptDeliveryOnCrash {@code true} if Bugsnag should try to send crashing errors
* prior to app termination
*/
public void setAttemptDeliveryOnCrash(boolean attemptDeliveryOnCrash) {
impl.setAttemptDeliveryOnCrash(attemptDeliveryOnCrash);
}

/**
* Whether Bugsnag should try to send crashing errors prior to app termination.
*
* @see #setAttemptDeliveryOnCrash(boolean)
*/
public boolean isAttemptDeliveryOnCrash() {
return impl.getAttemptDeliveryOnCrash();
}

Set<Plugin> getPlugins() {
return impl.getPlugins();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;

import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Future;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.TimeUnit;

class DeliveryDelegate extends BaseObservable {

@VisibleForTesting
static long DELIVERY_TIMEOUT = 3000L;

final Logger logger;
private final EventStore eventStore;
private final ImmutableConfig immutableConfig;
Expand Down Expand Up @@ -55,7 +56,11 @@ void deliver(@NonNull Event event) {
String severityReasonType = event.getImpl().getSeverityReasonType();
boolean promiseRejection = REASON_PROMISE_REJECTION.equals(severityReasonType);
boolean anr = event.getImpl().isAnr(event);
cacheEvent(event, anr || promiseRejection);
if (immutableConfig.getAttemptDeliveryOnCrash()) {
cacheAndSendSynchronously(event);
} else {
cacheEvent(event, anr || promiseRejection);
}
} else if (callbackState.runOnSendTasks(event, logger)) {
// Build the eventPayload
String apiKey = event.getApiKey();
Expand Down Expand Up @@ -107,6 +112,24 @@ DeliveryStatus deliverPayloadInternal(@NonNull EventPayload payload, @NonNull Ev
return deliveryStatus;
}

private void cacheAndSendSynchronously(@NonNull Event event) {
long cutoffTime = System.currentTimeMillis() + DELIVERY_TIMEOUT;
Future<String> task = eventStore.writeAndDeliver(event);

long timeout = cutoffTime - System.currentTimeMillis();
if (task != null && timeout > 0) {
try {
task.get(timeout, TimeUnit.MILLISECONDS);
} catch (Exception ex) {
logger.w("failed to immediately deliver event", ex);
}

if (!task.isDone()) {
task.cancel(true);
}
}
}

private void cacheEvent(@NonNull Event event, boolean attemptSend) {
eventStore.write(event);
if (attemptSend) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ data class ImmutableConfig(
val maxReportedThreads: Int,
val persistenceDirectory: Lazy<File>,
val sendLaunchCrashesSynchronously: Boolean,
val attemptDeliveryOnCrash: Boolean,

// results cached here to avoid unnecessary lookups in Client.
val packageInfo: PackageInfo?,
Expand Down Expand Up @@ -167,6 +168,7 @@ internal fun convertToImmutableConfig(
telemetry = config.telemetry.toSet(),
persistenceDirectory = persistenceDir,
sendLaunchCrashesSynchronously = config.sendLaunchCrashesSynchronously,
attemptDeliveryOnCrash = config.isAttemptDeliveryOnCrash,
packageInfo = packageInfo,
appInfo = appInfo,
redactedKeys = config.redactedKeys.toSet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public File invoke() {
}
}),
true,
true,
null,
null,
Collections.singleton("password")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<ID>ThrowingExceptionsWithoutMessageOrCause:TrimmedStacktraceScenario.kt$TrimmedStacktraceScenario$RuntimeException()</ID>
<ID>TooGenericExceptionThrown:CrashHandlerScenario.kt$CrashHandlerScenario$throw RuntimeException("CrashHandlerScenario")</ID>
<ID>TooGenericExceptionThrown:CustomHttpClientFlushScenario.kt$CustomHttpClientFlushScenario$throw RuntimeException("ReportCacheScenario")</ID>
<ID>TooGenericExceptionThrown:DeliverOnCrashScenario.kt$DeliverOnCrashScenario$throw RuntimeException("DeliverOnCrashScenario")</ID>
<ID>TooGenericExceptionThrown:DisableAutoDetectErrorsScenario.kt$DisableAutoDetectErrorsScenario$throw RuntimeException("Should never appear")</ID>
<ID>TooGenericExceptionThrown:FeatureFlagScenario.kt$FeatureFlagScenario$throw RuntimeException("FeatureFlagScenario unhandled")</ID>
<ID>TooGenericExceptionThrown:OnSendCallbackScenario.kt$OnSendCallbackScenario$throw RuntimeException("Unhandled Error")</ID>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.bugsnag.android.mazerunner.scenarios

import android.content.Context
import com.bugsnag.android.Configuration

private const val TIMEOUT = 15_000L

internal class DeliverOnCrashScenario(
config: Configuration,
context: Context,
eventMetadata: String
) : Scenario(config, context, eventMetadata) {

init {
config.isAttemptDeliveryOnCrash = true
val deliveryDelegate = Class.forName("DeliveryDelegate")
deliveryDelegate.getDeclaredField("DELIVERY_TIMEOUT").apply {
isAccessible = true
set(null, TIMEOUT)
}
}

override fun startScenario() {
super.startScenario()
throw RuntimeException("DeliverOnCrashScenario")
}
}
8 changes: 8 additions & 0 deletions features/full_tests/crash_handler.feature
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,11 @@ Feature: Reporting with other exception handlers installed
And the exception "errorClass" equals "java.lang.RuntimeException"
And the exception "message" equals "CrashHandlerScenario"
And the event "metaData.customHandler.invoked" is true

Scenario: Delivers crashes synchronously when configured
When I run "DeliverOnCrashScenario"
And I wait to receive an error
Then the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier
And the error payload field "events" is an array with 1 elements
And the exception "errorClass" equals "java.lang.RuntimeException"
And the exception "message" equals "DeliverOnCrashScenario"

0 comments on commit e8554c5

Please sign in to comment.