From 1c352255884411414d4fba0d4f62818f8f3c9f13 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 30 Apr 2021 11:30:51 +0100 Subject: [PATCH 1/4] test: add test for breadcrumb timestamp --- .../test/java/com/bugsnag/android/BreadcrumbFacadeTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbFacadeTest.java index 5b967a8f59..6dc00640fd 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbFacadeTest.java @@ -75,4 +75,9 @@ public void metadataValid() { public void dateValid() { assertEquals(new Date(0).getTime(), crumb.getTimestamp().getTime()); } + + @Test + public void stringDateValid() { + assertEquals("1970-01-01T00:00:00.000Z", crumb.getStringTimestamp()); + } } From 458a9b5e3308e34914fc99a21c60aa92163133c0 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 30 Apr 2021 15:35:10 +0100 Subject: [PATCH 2/4] feat: support enabling/disabling automatic error detection for unity --- .../main/java/com/bugsnag/android/Client.java | 42 ++++++++--- .../com/bugsnag/android/ExceptionHandler.java | 7 ++ .../com/bugsnag/android/LibraryLoader.java | 8 +- .../com/bugsnag/android/NativeInterface.java | 16 +++- .../java/com/bugsnag/android/PluginClient.kt | 74 +++++++++++++++---- .../com/bugsnag/android/ClientFacadeTest.java | 50 ++++++++++++- .../bugsnag/android/ExceptionHandlerTest.kt | 6 ++ .../com/bugsnag/android/LibraryLoaderTest.kt | 3 + .../bugsnag/android/NativeInterfaceApiTest.kt | 12 +++ .../com/bugsnag/android/PluginClientTest.kt | 9 +++ .../java/com/bugsnag/android/AnrPlugin.kt | 58 +++++++++------ .../java/com/bugsnag/android/NdkPlugin.kt | 33 +++++++-- 12 files changed, 257 insertions(+), 61 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index 8a836bec33..9e637ba7e3 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -78,7 +78,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware { final Logger logger; final DeliveryDelegate deliveryDelegate; - final ClientObservable clientObservable = new ClientObservable(); + final ClientObservable clientObservable; private PluginClient pluginClient; final Notifier notifier = new Notifier(); @@ -88,6 +88,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware { final LastRunInfoStore lastRunInfoStore; final LaunchCrashTracker launchCrashTracker; final BackgroundTaskService bgTaskService = new BackgroundTaskService(); + private ExceptionHandler exceptionHandler; /** * Initialize a Bugsnag client @@ -137,6 +138,7 @@ public Unit invoke(Boolean hasConnection, String networkState) { immutableConfig = sanitiseConfiguration(appContext, configuration, connectivity); logger = immutableConfig.getLogger(); warnIfNotAppContext(androidContext); + clientObservable = new ClientObservable(); // Set up breadcrumbs callbackState = configuration.impl.callbackState.copy(); @@ -209,8 +211,9 @@ public Unit invoke(String activity, Map metadata) { immutableConfig, breadcrumbState, notifier, bgTaskService); // Install a default exception handler with this client + exceptionHandler = new ExceptionHandler(this, logger); if (immutableConfig.getEnabledErrorTypes().getUnhandledExceptions()) { - new ExceptionHandler(this, logger); + exceptionHandler.install(); } // register a receiver for automatic breadcrumbs @@ -245,6 +248,7 @@ public Unit invoke(String activity, Map metadata) { ContextState contextState, CallbackState callbackState, UserState userState, + ClientObservable clientObservable, Context appContext, @NonNull DeviceDataCollector deviceDataCollector, @NonNull AppDataCollector appDataCollector, @@ -267,6 +271,7 @@ public Unit invoke(String activity, Map metadata) { this.contextState = contextState; this.callbackState = callbackState; this.userState = userState; + this.clientObservable = clientObservable; this.appContext = appContext; this.deviceDataCollector = deviceDataCollector; this.appDataCollector = appDataCollector; @@ -365,6 +370,17 @@ void registerObserver(Observer observer) { launchCrashTracker.addObserver(observer); } + void unregisterObserver(Observer observer) { + metadataState.deleteObserver(observer); + breadcrumbState.deleteObserver(observer); + sessionTracker.deleteObserver(observer); + clientObservable.deleteObserver(observer); + userState.deleteObserver(observer); + contextState.deleteObserver(observer); + deliveryDelegate.deleteObserver(observer); + launchCrashTracker.deleteObserver(observer); + } + /** * Sends initial state values for Metadata/User/Context to any registered observers. */ @@ -985,13 +1001,7 @@ Logger getLogger() { @SuppressWarnings("rawtypes") @Nullable Plugin getPlugin(@NonNull Class clz) { - Set plugins = pluginClient.getPlugins(); - for (Plugin plugin : plugins) { - if (plugin.getClass().equals(clz)) { - return plugin; - } - } - return null; + return pluginClient.findPlugin(clz); } Notifier getNotifier() { @@ -1001,4 +1011,18 @@ Notifier getNotifier() { MetadataState getMetadataState() { return metadataState; } + + void setAutoNotify(boolean autoNotify) { + pluginClient.setAutoNotify(this, autoNotify); + + if (autoNotify) { + exceptionHandler.install(); + } else { + exceptionHandler.uninstall(); + } + } + + void setAutoDetectAnrs(boolean autoDetectAnrs) { + pluginClient.setAutoDetectAnrs(this, autoDetectAnrs); + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ExceptionHandler.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/ExceptionHandler.java index a10ee3f263..4e5f9beab9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ExceptionHandler.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ExceptionHandler.java @@ -23,9 +23,16 @@ class ExceptionHandler implements UncaughtExceptionHandler { this.client = client; this.logger = logger; this.originalHandler = Thread.getDefaultUncaughtExceptionHandler(); + } + + void install() { Thread.setDefaultUncaughtExceptionHandler(this); } + void uninstall() { + Thread.setDefaultUncaughtExceptionHandler(originalHandler); + } + @Override public void uncaughtException(@NonNull Thread thread, @NonNull Throwable throwable) { boolean strictModeThrowable = strictModeHandler.isStrictModeThrowable(throwable); diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/LibraryLoader.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/LibraryLoader.java index f8dcb51a71..850a2f82c8 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/LibraryLoader.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/LibraryLoader.java @@ -4,7 +4,8 @@ class LibraryLoader { - private AtomicBoolean attemptedLoad = new AtomicBoolean(); + private final AtomicBoolean attemptedLoad = new AtomicBoolean(); + private boolean loaded = false; /** * Attempts to load a native library, returning false if the load was unsuccessful. @@ -21,6 +22,7 @@ boolean loadLibrary(String name, Client client, OnErrorCallback callback) { if (!attemptedLoad.getAndSet(true)) { try { System.loadLibrary(name); + loaded = true; return true; } catch (UnsatisfiedLinkError error) { client.notify(error, callback); @@ -28,4 +30,8 @@ boolean loadLibrary(String name, Client client, OnErrorCallback callback) { } return false; } + + boolean isLoaded() { + return loaded; + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java index ffd2f30221..30e857b601 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java @@ -405,11 +405,23 @@ public static Logger getLogger() { return getClient().getConfig().getLogger(); } + /** + * Switches automatic error detection on/off after Bugsnag has initialized. + * This is required to support legacy functionality in Unity. + * + * @param autoNotify whether errors should be automatically detected. + */ public static void setAutoNotify(boolean autoNotify) { - // TODO implement me + getClient().setAutoNotify(autoNotify); } + /** + * Switches automatic ANR detection on/off after Bugsnag has initialized. + * This is required to support legacy functionality in Unity. + * + * @param autoDetectAnrs whether ANRs should be automatically detected. + */ public static void setAutoDetectAnrs(boolean autoDetectAnrs) { - // TODO implement me + getClient().setAutoDetectAnrs(autoDetectAnrs); } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/PluginClient.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/PluginClient.kt index 8d3671ee42..d4b4f93127 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/PluginClient.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/PluginClient.kt @@ -2,11 +2,21 @@ package com.bugsnag.android internal class PluginClient( userPlugins: Set, - immutableConfig: ImmutableConfig, + private val immutableConfig: ImmutableConfig, private val logger: Logger ) { - protected val plugins: Set + companion object { + private const val NDK_PLUGIN = "com.bugsnag.android.NdkPlugin" + private const val ANR_PLUGIN = "com.bugsnag.android.AnrPlugin" + private const val RN_PLUGIN = "com.bugsnag.android.BugsnagReactNativePlugin" + } + + private val plugins: Set + + private val ndkPlugin = instantiatePlugin(NDK_PLUGIN) + private val anrPlugin = instantiatePlugin(ANR_PLUGIN) + private val rnPlugin = instantiatePlugin(RN_PLUGIN) init { val set = mutableSetOf() @@ -14,13 +24,9 @@ internal class PluginClient( // instantiate ANR + NDK plugins by reflection as bugsnag-android-core has no // direct dependency on the artefacts - if (immutableConfig.enabledErrorTypes.ndkCrashes) { - instantiatePlugin("com.bugsnag.android.NdkPlugin")?.let { set.add(it) } - } - if (immutableConfig.enabledErrorTypes.anrs) { - instantiatePlugin("com.bugsnag.android.AnrPlugin")?.let { set.add(it) } - } - instantiatePlugin("com.bugsnag.android.BugsnagReactNativePlugin")?.let { set.add(it) } + ndkPlugin?.let(set::add) + anrPlugin?.let(set::add) + rnPlugin?.let(set::add) plugins = set.toSet() } @@ -37,11 +43,51 @@ internal class PluginClient( } } - fun loadPlugins(client: Client) = plugins.forEach { - try { - it.load(client) - } catch (exc: Throwable) { - logger.e("Failed to load plugin $it, continuing with initialisation.", exc) + fun loadPlugins(client: Client) { + plugins.forEach { plugin -> + try { + loadPluginInternal(plugin, client) + } catch (exc: Throwable) { + logger.e("Failed to load plugin $plugin, continuing with initialisation.", exc) + } + } + } + + fun setAutoNotify(client: Client, autoNotify: Boolean) { + setAutoDetectAnrs(client, autoNotify) + + if (autoNotify) { + ndkPlugin?.load(client) + } else { + ndkPlugin?.unload() + } + } + + fun setAutoDetectAnrs(client: Client, autoDetectAnrs: Boolean) { + if (autoDetectAnrs) { + anrPlugin?.load(client) + } else { + anrPlugin?.unload() + } + } + + fun findPlugin(clz: Class<*>): Plugin? = plugins.find { it.javaClass == clz } + + private fun loadPluginInternal(plugin: Plugin, client: Client) { + val name = plugin.javaClass.name + val errorTypes = immutableConfig.enabledErrorTypes + + // only initialize NDK/ANR plugins if automatic detection enabled + if (name == NDK_PLUGIN) { + if (errorTypes.ndkCrashes) { + plugin.load(client) + } + } else if (name == ANR_PLUGIN) { + if (errorTypes.anrs) { + plugin.load(client) + } + } else { + plugin.load(client) } } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java index b4e54fc241..e9d4bed457 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java @@ -8,7 +8,6 @@ import static org.mockito.Mockito.when; import android.content.Context; -import android.content.SharedPreferences; import android.os.storage.StorageManager; import androidx.annotation.NonNull; @@ -23,6 +22,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Observable; +import java.util.Observer; @SuppressWarnings("ConstantConditions") @RunWith(MockitoJUnitRunner.class) @@ -43,6 +44,9 @@ public class ClientFacadeTest { @Mock UserState userState; + @Mock + ClientObservable clientObservable; + @Mock Context appContext; @@ -73,9 +77,6 @@ public class ClientFacadeTest { @Mock SessionLifecycleCallback sessionLifecycleCallback; - @Mock - SharedPreferences sharedPrefs; - @Mock Connectivity connectivity; @@ -112,6 +113,7 @@ public void setUp() { contextState, callbackState, userState, + clientObservable, appContext, deviceDataCollector, appDataCollector, @@ -482,4 +484,44 @@ public void markLaunchCompleted() { client.markLaunchCompleted(); verify(launchCrashTracker, times(1)).markLaunchCompleted(); } + + + @Test + public void registerObserver() { + Observer observer = new Observer() { + @Override + public void update(Observable observable, Object arg) { + } + }; + client.registerObserver(observer); + + verify(metadataState, times(1)).addObserver(observer); + verify(breadcrumbState, times(1)).addObserver(observer); + verify(sessionTracker, times(1)).addObserver(observer); + verify(clientObservable, times(1)).addObserver(observer); + verify(userState, times(1)).addObserver(observer); + verify(contextState, times(1)).addObserver(observer); + verify(deliveryDelegate, times(1)).addObserver(observer); + verify(launchCrashTracker, times(1)).addObserver(observer); + } + + @Test + public void unregisterObserver() { + Observer observer = new Observer() { + @Override + public void update(Observable observable, Object arg) { + } + }; + client.unregisterObserver(observer); + + verify(metadataState, times(1)).deleteObserver(observer); + verify(breadcrumbState, times(1)).deleteObserver(observer); + verify(sessionTracker, times(1)).deleteObserver(observer); + verify(clientObservable, times(1)).deleteObserver(observer); + verify(userState, times(1)).deleteObserver(observer); + verify(contextState, times(1)).deleteObserver(observer); + verify(deliveryDelegate, times(1)).deleteObserver(observer); + verify(launchCrashTracker, times(1)).deleteObserver(observer); + } + } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ExceptionHandlerTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ExceptionHandlerTest.kt index db41e4cced..1ba84aa619 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ExceptionHandlerTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ExceptionHandlerTest.kt @@ -35,7 +35,13 @@ internal class ExceptionHandlerTest { @Test fun handlerInstalled() { val exceptionHandler = ExceptionHandler(client, NoopLogger) + assertSame(originalHandler, Thread.getDefaultUncaughtExceptionHandler()) + + exceptionHandler.install() assertSame(exceptionHandler, Thread.getDefaultUncaughtExceptionHandler()) + + exceptionHandler.uninstall() + assertSame(originalHandler, Thread.getDefaultUncaughtExceptionHandler()) } @Test diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/LibraryLoaderTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/LibraryLoaderTest.kt index 970f4d60f4..8b3d8dbb97 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/LibraryLoaderTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/LibraryLoaderTest.kt @@ -20,6 +20,7 @@ class LibraryLoaderTest { val libraryLoader = LibraryLoader() val loaded = libraryLoader.loadLibrary("foo", client) { true } assertFalse(loaded) + assertFalse(libraryLoader.isLoaded) verify(client, times(1)).notify(any(), any()) } @@ -28,10 +29,12 @@ class LibraryLoaderTest { val libraryLoader = LibraryLoader() var loaded = libraryLoader.loadLibrary("foo", client) { true } assertFalse(loaded) + assertFalse(libraryLoader.isLoaded) // duplicate calls only invoke System.loadLibrary once loaded = libraryLoader.loadLibrary("foo", client) { true } assertFalse(loaded) + assertFalse(libraryLoader.isLoaded) verify(client, times(1)).notify(any(), any()) } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/NativeInterfaceApiTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/NativeInterfaceApiTest.kt index 68d736e535..8a63a710d8 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/NativeInterfaceApiTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/NativeInterfaceApiTest.kt @@ -223,4 +223,16 @@ internal class NativeInterfaceApiTest { NativeInterface.notify("SIGPIPE", "SIGSEGV 11", Severity.ERROR, arrayOf()) verify(client, times(1)).notify(any(), any()) } + + @Test + fun autoDetectAnrs() { + NativeInterface.setAutoDetectAnrs(true) + verify(client, times(1)).setAutoDetectAnrs(true) + } + + @Test + fun autoNotify() { + NativeInterface.setAutoNotify(true) + verify(client, times(1)).setAutoNotify(true) + } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/PluginClientTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/PluginClientTest.kt index 6ebce1c8fd..44ffeb1618 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/PluginClientTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/PluginClientTest.kt @@ -1,5 +1,7 @@ package com.bugsnag.android +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock @@ -24,4 +26,11 @@ class PluginClientTest { pluginClient.loadPlugins(client) Mockito.verify(plugin, times(1)).load(client) } + + @Test + fun findPlugin() { + val pluginClient = PluginClient(setOf(plugin), config, NoopLogger) + assertNull(pluginClient.findPlugin(String::class.java)) + assertEquals(plugin, pluginClient.findPlugin(plugin::class.java)) + } } 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 9858689943..2091b2ba49 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 @@ -2,6 +2,7 @@ package com.bugsnag.android import android.os.Handler import android.os.Looper +import java.util.concurrent.atomic.AtomicBoolean internal class AnrPlugin : Plugin { @@ -20,7 +21,8 @@ internal class AnrPlugin : Plugin { } } - private val loader = LibraryLoader() + private val libraryLoader = LibraryLoader() + private val oneTimeSetupPerformed = AtomicBoolean(false) private lateinit var client: Client private val collector = AnrDetailsCollector() @@ -37,31 +39,14 @@ internal class AnrPlugin : Plugin { } override fun load(client: Client) { - val loaded = loader.loadLibrary("bugsnag-plugin-android-anr", client) { - val error = it.errors[0] - error.errorClass = "AnrLinkError" - error.errorMessage = LOAD_ERR_MSG - true - } + this.client = client - if (loaded) { - @Suppress("UNCHECKED_CAST") - val clz = loadClass("com.bugsnag.android.NdkPlugin") as Class? - if (clz != null) { - val ndkPlugin = client.getPlugin(clz) - if (ndkPlugin != null) { - val method = ndkPlugin.javaClass.getMethod("getUnwindStackFunction") - @Suppress("UNCHECKED_CAST") - val function = method.invoke(ndkPlugin) as Long - setUnwindFunction(function) - } - } - - // this must be run from the main thread as the SIGQUIT is sent to the main thread, - // and if the handler is installed on a background thread instead we receive no signal + if (!oneTimeSetupPerformed.getAndSet(true)) { + performOneTimeSetup(client) + } + if (libraryLoader.isLoaded) { Handler(Looper.getMainLooper()).post( Runnable { - this.client = client enableAnrReporting() client.logger.i("Initialised ANR Plugin") } @@ -71,7 +56,32 @@ internal class AnrPlugin : Plugin { } } - override fun unload() = disableAnrReporting() + private fun performOneTimeSetup(client: Client) { + libraryLoader.loadLibrary("bugsnag-plugin-android-anr", client) { + val error = it.errors[0] + error.errorClass = "AnrLinkError" + error.errorMessage = LOAD_ERR_MSG + true + } + @Suppress("UNCHECKED_CAST") + val clz = loadClass("com.bugsnag.android.NdkPlugin") as Class? + if (clz != null) { + val ndkPlugin = client.getPlugin(clz) + if (ndkPlugin != null) { + val method = ndkPlugin.javaClass.getMethod("getUnwindStackFunction") + + @Suppress("UNCHECKED_CAST") + val function = method.invoke(ndkPlugin) as Long + setUnwindFunction(function) + } + } + } + + override fun unload() { + if (libraryLoader.isLoaded) { + disableAnrReporting() + } + } /** * Notifies bugsnag that an ANR has occurred, by generating an Error report and populating it diff --git a/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/NdkPlugin.kt b/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/NdkPlugin.kt index 4c8434db40..918ddacc54 100644 --- a/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/NdkPlugin.kt +++ b/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/NdkPlugin.kt @@ -1,6 +1,7 @@ package com.bugsnag.android import com.bugsnag.android.ndk.NativeBridge +import java.util.concurrent.atomic.AtomicBoolean internal class NdkPlugin : Plugin { @@ -9,12 +10,14 @@ internal class NdkPlugin : Plugin { "not report NDK errors. See https://docs.bugsnag.com/platforms/android/ndk-link-errors" } - private val loader = LibraryLoader() + private val libraryLoader = LibraryLoader() + private val oneTimeSetupPerformed = AtomicBoolean(false) private external fun enableCrashReporting() private external fun disableCrashReporting() private var nativeBridge: NativeBridge? = null + private var client: Client? = null private fun initNativeBridge(client: Client): NativeBridge { val nativeBridge = NativeBridge() @@ -24,23 +27,39 @@ internal class NdkPlugin : Plugin { } override fun load(client: Client) { - val loaded = loader.loadLibrary("bugsnag-ndk", client) { + this.client = client + + if (!oneTimeSetupPerformed.getAndSet(true)) { + performOneTimeSetup(client) + } + if (libraryLoader.isLoaded) { + enableCrashReporting() + client.logger.i("Initialised NDK Plugin") + } + } + + private fun performOneTimeSetup(client: Client) { + libraryLoader.loadLibrary("bugsnag-ndk", client) { val error = it.errors[0] error.errorClass = "NdkLinkError" error.errorMessage = LOAD_ERR_MSG true } - - if (loaded) { + if (libraryLoader.isLoaded) { nativeBridge = initNativeBridge(client) - enableCrashReporting() - client.logger.i("Initialised NDK Plugin") } else { client.logger.e(LOAD_ERR_MSG) } } - override fun unload() = disableCrashReporting() + override fun unload() { + if (libraryLoader.isLoaded) { + disableCrashReporting() + nativeBridge?.let { bridge -> + client?.unregisterObserver(bridge) + } + } + } fun getUnwindStackFunction(): Long { val bridge = nativeBridge From a27c2af4fea910797e0519e3b4cb2f4d20c0b028 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 4 May 2021 11:30:42 +0100 Subject: [PATCH 3/4] test: add E2E tests for autoNotify/autoDetectAnrs --- .../src/main/cpp/cxx-scenarios.cpp | 10 +++ .../UnhandledNdkAutoNotifyFalseScenario.kt | 30 +++++++++ .../UnhandledNdkAutoNotifyTrueScenario.kt | 26 ++++++++ .../jvm-scenarios/detekt-baseline.xml | 2 + .../com/bugsnag/android/TestHarnessHooks.kt | 8 +++ .../scenarios/AutoDetectAnrsFalseScenario.kt | 37 +++++++++++ .../scenarios/AutoDetectAnrsTrueScenario.kt | 33 ++++++++++ .../HandledJvmAutoNotifyFalseScenario.kt | 19 ++++++ .../UnhandledJvmAutoNotifyFalseScenario.kt | 24 +++++++ .../UnhandledJvmAutoNotifyTrueScenario.kt | 20 ++++++ .../full_tests/batch_1/auto_notify.feature | 66 +++++++++++++++++++ 11 files changed, 275 insertions(+) create mode 100644 features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyFalseScenario.kt create mode 100644 features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyTrueScenario.kt create mode 100644 features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsFalseScenario.kt create mode 100644 features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsTrueScenario.kt create mode 100644 features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledJvmAutoNotifyFalseScenario.kt create mode 100644 features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyFalseScenario.kt create mode 100644 features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyTrueScenario.kt create mode 100644 features/full_tests/batch_1/auto_notify.feature diff --git a/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp b/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp index d2f8574060..020393b2ea 100644 --- a/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp +++ b/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp @@ -353,4 +353,14 @@ Java_com_bugsnag_android_mazerunner_scenarios_CXXMarkLaunchCompletedScenario_cra jobject thiz) { abort(); } + +JNIEXPORT void JNICALL +Java_com_bugsnag_android_mazerunner_scenarios_UnhandledNdkAutoNotifyTrueScenario_crash(JNIEnv *env) { + abort(); +} + +JNIEXPORT void JNICALL +Java_com_bugsnag_android_mazerunner_scenarios_UnhandledNdkAutoNotifyFalseScenario_crash(JNIEnv *env) { + abort(); +} } diff --git a/features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyFalseScenario.kt b/features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyFalseScenario.kt new file mode 100644 index 0000000000..72befbf5ea --- /dev/null +++ b/features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyFalseScenario.kt @@ -0,0 +1,30 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.mazerunner.getZeroEventsLogMessages +import com.bugsnag.android.setAutoNotify + +class UnhandledNdkAutoNotifyFalseScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + init { + System.loadLibrary("cxx-scenarios") + } + + external fun crash() + + override fun startScenario() { + super.startScenario() + setAutoNotify(Bugsnag.getClient(), false) + crash() + } + + override fun getInterceptedLogMessages(): List { + return getZeroEventsLogMessages(startBugsnagOnly) + } +} diff --git a/features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyTrueScenario.kt b/features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyTrueScenario.kt new file mode 100644 index 0000000000..dd96cdc219 --- /dev/null +++ b/features/fixtures/mazerunner/cxx-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledNdkAutoNotifyTrueScenario.kt @@ -0,0 +1,26 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.setAutoNotify + +class UnhandledNdkAutoNotifyTrueScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + init { + System.loadLibrary("cxx-scenarios") + } + + external fun crash() + + override fun startScenario() { + super.startScenario() + setAutoNotify(Bugsnag.getClient(), false) + setAutoNotify(Bugsnag.getClient(), true) + crash() + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml b/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml index 60cddc13d2..e63cd6f4c7 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml +++ b/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml @@ -4,6 +4,8 @@ MagicNumber:AnrHelper.kt$1000 MagicNumber:AnrHelper.kt$<no name provided>$60000 + MagicNumber:AutoDetectAnrsFalseScenario.kt$AutoDetectAnrsFalseScenario$100000 + MagicNumber:AutoDetectAnrsTrueScenario.kt$AutoDetectAnrsTrueScenario$100000 MagicNumber:BugsnagInitScenario.kt$BugsnagInitScenario$25 MagicNumber:HandledKotlinSmokeScenario.kt$HandledKotlinSmokeScenario$999 MagicNumber:JvmAnrSleepScenario.kt$JvmAnrSleepScenario$100000 diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index 5dfe6f851d..b2a5b205d3 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -88,3 +88,11 @@ fun generateEvent(client: Client): Event { event.device = generateDeviceWithState() return event } + +fun setAutoNotify(client: Client, enabled: Boolean) { + client.setAutoNotify(enabled) +} + +fun setAutoDetectAnrs(client: Client, enabled: Boolean) { + client.setAutoDetectAnrs(enabled) +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsFalseScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsFalseScenario.kt new file mode 100644 index 0000000000..5b6b5d0859 --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsFalseScenario.kt @@ -0,0 +1,37 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import android.os.Handler +import android.os.Looper +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.mazerunner.getZeroEventsLogMessages +import com.bugsnag.android.setAutoDetectAnrs + +internal class AutoDetectAnrsFalseScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + init { + config.enabledErrorTypes.anrs = true + } + + override fun startScenario() { + super.startScenario() + setAutoDetectAnrs(Bugsnag.getClient(), false) + + val main = Handler(Looper.getMainLooper()) + main.postDelayed( + Runnable { + Thread.sleep(100000) + }, + 1 + ) // A moment of delay so there is something to 'tap' onscreen + } + + override fun getInterceptedLogMessages(): List { + return getZeroEventsLogMessages(startBugsnagOnly) + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsTrueScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsTrueScenario.kt new file mode 100644 index 0000000000..fbebf2532b --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/AutoDetectAnrsTrueScenario.kt @@ -0,0 +1,33 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import android.os.Handler +import android.os.Looper +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.setAutoDetectAnrs + +internal class AutoDetectAnrsTrueScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + init { + config.enabledErrorTypes.anrs = true + } + + override fun startScenario() { + super.startScenario() + setAutoDetectAnrs(Bugsnag.getClient(), false) + setAutoDetectAnrs(Bugsnag.getClient(), true) + + val main = Handler(Looper.getMainLooper()) + main.postDelayed( + Runnable { + Thread.sleep(100000) + }, + 1 + ) // A moment of delay so there is something to 'tap' onscreen + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledJvmAutoNotifyFalseScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledJvmAutoNotifyFalseScenario.kt new file mode 100644 index 0000000000..2700b3790f --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledJvmAutoNotifyFalseScenario.kt @@ -0,0 +1,19 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.setAutoNotify + +class HandledJvmAutoNotifyFalseScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + override fun startScenario() { + super.startScenario() + setAutoNotify(Bugsnag.getClient(), false) + Bugsnag.notify(generateException()) + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyFalseScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyFalseScenario.kt new file mode 100644 index 0000000000..dd7c9e5941 --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyFalseScenario.kt @@ -0,0 +1,24 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.mazerunner.getZeroEventsLogMessages +import com.bugsnag.android.setAutoNotify + +class UnhandledJvmAutoNotifyFalseScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + override fun startScenario() { + super.startScenario() + setAutoNotify(Bugsnag.getClient(), false) + throw generateException() + } + + override fun getInterceptedLogMessages(): List { + return getZeroEventsLogMessages(startBugsnagOnly) + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyTrueScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyTrueScenario.kt new file mode 100644 index 0000000000..195a7626c8 --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledJvmAutoNotifyTrueScenario.kt @@ -0,0 +1,20 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.setAutoNotify + +class UnhandledJvmAutoNotifyTrueScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + override fun startScenario() { + super.startScenario() + setAutoNotify(Bugsnag.getClient(), false) + setAutoNotify(Bugsnag.getClient(), true) + throw generateException() + } +} diff --git a/features/full_tests/batch_1/auto_notify.feature b/features/full_tests/batch_1/auto_notify.feature new file mode 100644 index 0000000000..8c98eb217b --- /dev/null +++ b/features/full_tests/batch_1/auto_notify.feature @@ -0,0 +1,66 @@ +Feature: Switching automatic error detection on/off for Unity + +Scenario: Handled JVM exceptions are captured with autoNotify=false + When I run "HandledJvmAutoNotifyFalseScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "HandledJvmAutoNotifyFalseScenario" + +Scenario: JVM exception not captured with autoNotify=false + When I run "UnhandledJvmAutoNotifyFalseScenario" and relaunch the app + And I configure Bugsnag for "UnhandledJvmAutoNotifyFalseScenario" + Then Bugsnag confirms it has no errors to send + +Scenario: NDK signal not captured with autoNotify=false + When I run "UnhandledNdkAutoNotifyFalseScenario" and relaunch the app + And I configure Bugsnag for "UnhandledNdkAutoNotifyFalseScenario" + Then Bugsnag confirms it has no errors to send + +@skip_android_8_1 +Scenario: ANR not captured with autoDetectAnrs=false + When I run "AutoDetectAnrsFalseScenario" and relaunch the app + And I configure Bugsnag for "AutoDetectAnrsFalseScenario" + Then Bugsnag confirms it has no errors to send + +Scenario: JVM exception captured with autoNotify reenabled + When I run "UnhandledJvmAutoNotifyTrueScenario" and relaunch the app + And I configure Bugsnag for "UnhandledJvmAutoNotifyTrueScenario" + Then I wait to receive an error + And 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 "UnhandledJvmAutoNotifyTrueScenario" + And the exception "type" equals "android" + And the event "unhandled" is true + And the event "severity" equals "error" + +Scenario: NDK signal captured with autoNotify reenabled + When I run "UnhandledNdkAutoNotifyTrueScenario" and relaunch the app + And I configure Bugsnag for "UnhandledNdkAutoNotifyTrueScenario" + Then I wait to receive an error + And 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 "message" equals "Abort program" + And the exception "type" equals "c" + And the event "unhandled" is true + And the event "severity" equals "error" + And the event "severityReason.type" equals "signal" + And the event "severityReason.unhandledOverridden" is false + +@skip_android_8_1 +Scenario: ANR captured with autoDetectAnrs reenabled + When I run "AutoDetectAnrsTrueScenario" + And I wait for 2 seconds + And I tap the screen 3 times + And I wait for 4 seconds + And I clear any error dialogue + 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 "ANR" + And the exception "message" starts with " Input dispatching timed out" + And the exception "type" equals "android" + And the event "unhandled" is true + And the event "severity" equals "error" + And the event "severityReason.type" equals "anrError" + And the event "severityReason.unhandledOverridden" is false From c08e2a4de0a7d62104a16a80911e203e6b48a56c Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 4 May 2021 11:32:16 +0100 Subject: [PATCH 4/4] docs: add changelog --- CHANGELOG.md | 7 +++++++ .../src/main/java/com/bugsnag/android/Client.java | 6 ++++-- .../test/java/com/bugsnag/android/ClientFacadeTest.java | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca7e003999..aea7ff6416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## TBD + +### Enhancements + +* Unity: add methods for setting autoNotify and autoDetectAnrs + [#1233](https://github.com/bugsnag/bugsnag-android/pull/1233) + ## 5.9.1 (2021-04-22) ### Bug fixes diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index 9e637ba7e3..c5db63c184 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -88,7 +88,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware { final LastRunInfoStore lastRunInfoStore; final LaunchCrashTracker launchCrashTracker; final BackgroundTaskService bgTaskService = new BackgroundTaskService(); - private ExceptionHandler exceptionHandler; + private final ExceptionHandler exceptionHandler; /** * Initialize a Bugsnag client @@ -264,7 +264,8 @@ public Unit invoke(String activity, Map metadata) { Logger logger, DeliveryDelegate deliveryDelegate, LastRunInfoStore lastRunInfoStore, - LaunchCrashTracker launchCrashTracker + LaunchCrashTracker launchCrashTracker, + ExceptionHandler exceptionHandler ) { this.immutableConfig = immutableConfig; this.metadataState = metadataState; @@ -289,6 +290,7 @@ public Unit invoke(String activity, Map metadata) { this.lastRunInfoStore = lastRunInfoStore; this.launchCrashTracker = launchCrashTracker; this.lastRunInfo = null; + this.exceptionHandler = exceptionHandler; } private LastRunInfo loadLastRunInfo() { diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java index e9d4bed457..052bea11b0 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java @@ -98,6 +98,9 @@ public class ClientFacadeTest { @Mock LaunchCrashTracker launchCrashTracker; + @Mock + ExceptionHandler exceptionHandler; + private Client client; private InterceptingLogger logger; @@ -129,7 +132,8 @@ public void setUp() { logger, deliveryDelegate, lastRunInfoStore, - launchCrashTracker + launchCrashTracker, + exceptionHandler ); // required fields for generating an event