diff --git a/CHANGELOG.md b/CHANGELOG.md index 20c31d2203..4addfe3432 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## 4.4.0 (2018-05-17) + +### Features + +Deprecation notice: + +SessionTrackingApiClient and ErrorApiClient are now deprecated in favour of the Delivery interface. +If you configure a custom HTTP client with Bugsnag, it is recommended that you migrate over to this new API. +Further information is available [in the docs.](https://docs.bugsnag.com/platforms/android/sdk/configuration-options/). + +* Expose Delivery API interface for configuring custom HTTP clients +[#299](https://github.com/bugsnag/bugsnag-android/pull/299) + +### Enhancements + +* Use buffered streams for IO (perf improvement) +[#307](https://github.com/bugsnag/bugsnag-android/pull/307) + + ## 4.3.4 (2018-05-02) ### Bug fixes diff --git a/Gemfile.lock b/Gemfile.lock index 09e9cb520c..100922b710 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,9 +1,10 @@ GIT remote: git@github.com:bugsnag/maze-runner - revision: 65bdcb7a135ef0a5864d03c454f7a1654f850237 + revision: f7123450d5a75b719911c6dd3baa0507e6062c2d specs: bugsnag-maze-runner (1.0.0) cucumber (~> 3.1.0) + cucumber-expressions (= 5.0.15) minitest (~> 5.0) rack (~> 2.0.0) test-unit (~> 3.2.0) @@ -11,7 +12,7 @@ GIT GEM remote: https://rubygems.org/ specs: - backports (3.11.1) + backports (3.11.3) builder (3.2.3) coderay (1.1.2) cucumber (3.1.0) @@ -27,7 +28,7 @@ GEM backports (>= 3.8.0) cucumber-tag_expressions (~> 1.1.0) gherkin (>= 5.0.0) - cucumber-expressions (5.0.13) + cucumber-expressions (5.0.15) cucumber-tag_expressions (1.1.1) cucumber-wire (0.0.1) diff-lcs (1.3) @@ -40,7 +41,7 @@ GEM pry (0.11.3) coderay (~> 1.1.0) method_source (~> 0.9.0) - rack (2.0.4) + rack (2.0.5) test-unit (3.2.7) power_assert diff --git a/features/custom_client.feature b/features/custom_client.feature new file mode 100644 index 0000000000..86e3ae9160 --- /dev/null +++ b/features/custom_client.feature @@ -0,0 +1,32 @@ +Feature: Using custom API clients for reporting errors + +Scenario: Set a custom session client and flush a stored session + When I run "CustomClientSessionFlushScenario" with the defaults + When I force stop the "com.bugsnag.android.mazerunner" Android app + And I set environment variable "EVENT_TYPE" to "CustomClientSessionFlushScenario" + And I set environment variable "EVENT_METADATA" to "DeliverSessions" + And I start the "com.bugsnag.android.mazerunner" Android app using the "com.bugsnag.android.mazerunner.MainActivity" activity + Then I should receive 2 requests + And the "Custom-Client" header equals "Hello World" for request 0 + And the "Custom-Client" header equals "Hello World" for request 1 + +Scenario: Set a custom error API client and notify an error + When I run "CustomClientErrorScenario" with the defaults + Then I should receive 1 request + And the "Custom-Client" header equals "Hello World" + And the request is a valid for the error reporting API + +Scenario: Set a custom session API client and start a session + When I run "CustomClientSessionScenario" with the defaults + Then I should receive 1 request + And the "Custom-Client" header equals "Hello World" + And the request is a valid for the session tracking API + +Scenario: Set a custom error client and flush a stored error + When I run "CustomClientErrorFlushScenario" with the defaults + When I force stop the "com.bugsnag.android.mazerunner" Android app + And I set environment variable "EVENT_TYPE" to "CustomClientErrorFlushScenario" + And I set environment variable "EVENT_METADATA" to "DeliverReports" + And I start the "com.bugsnag.android.mazerunner" Android app using the "com.bugsnag.android.mazerunner.MainActivity" activity + Then I should receive 1 request + And the "Custom-Client" header equals "Hello World" diff --git a/gradle.properties b/gradle.properties index 9bb7aee5fc..005870bddd 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ org.gradle.jvmargs=-Xmx1536m # This option should only be used with decoupled projects. More details, visit # http://www.gradle.org/docs/current/userguide/multi_project_builds.html#sec:decoupled_projects # org.gradle.parallel=true -VERSION_NAME=4.3.4 +VERSION_NAME=4.4.0 GROUP=com.bugsnag POM_SCM_URL=https://github.com/bugsnag/bugsnag-android POM_SCM_CONNECTION=scm:git@github.com:bugsnag/bugsnag-android.git diff --git a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index f83fd5768f..6cba832f00 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -2,7 +2,6 @@ package com.bugsnag.android import android.content.Context import android.net.ConnectivityManager -import com.bugsnag.android.Bugsnag.client /** * Accesses the session tracker and flushes all stored sessions @@ -11,31 +10,62 @@ internal fun flushAllSessions() { Bugsnag.getClient().sessionTracker.flushStoredSessions() } -internal fun flushErrorStoreAsync(client: Client, apiClient: ErrorReportApiClient) { - client.errorStore.flushAsync(apiClient) +internal fun flushErrorStoreAsync(client: Client) { + client.errorStore.flushAsync() } -internal fun flushErrorStoreOnLaunch(client: Client, apiClient: ErrorReportApiClient) { - client.errorStore.flushOnLaunch(apiClient) +internal fun flushErrorStoreOnLaunch(client: Client) { + client.errorStore.flushOnLaunch() } /** - * Creates an error API client with a 500ms delay, emulating poor network connectivity + * Creates a delivery API client with a 500ms delay, emulating poor network connectivity */ -internal fun createSlowErrorApiClient(context: Context): ErrorReportApiClient { +internal fun createSlowDelivery(context: Context): Delivery { val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? - val defaultHttpClient = DefaultHttpClient(cm) - - return ErrorReportApiClient({ url: String?, - report: Report?, - headers: MutableMap? -> - Thread.sleep(500) - defaultHttpClient.postReport(url, report, headers) - }) + val delivery = DefaultDelivery(cm) + + return object : Delivery { + override fun deliver(payload: SessionTrackingPayload?, config: Configuration?) { + Thread.sleep(500) + delivery.deliver(payload, config) + } + + override fun deliver(report: Report?, config: Configuration?) { + Thread.sleep(500) + delivery.deliver(report, config) + } + } +} + +internal fun createDefaultDelivery(context: Context): DefaultDelivery { + val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? + return DefaultDelivery(cm) +} + +internal fun createCustomHeaderDelivery(context: Context): Delivery { + return object : Delivery { + val delivery: DefaultDelivery = createDefaultDelivery(context) + + override fun deliver(payload: SessionTrackingPayload?, config: Configuration?) { + deliver(config?.sessionEndpoint, payload, config?.sessionApiHeaders) + } + + override fun deliver(report: Report?, config: Configuration?) { + deliver(config?.endpoint, report, config?.errorApiHeaders) + } + + fun deliver(endpoint: String?, + streamable: JsonStream.Streamable?, + headers: MutableMap?) { + headers!!["Custom-Client"] = "Hello World" + delivery.deliver(endpoint, streamable, headers) + } + } } + internal fun writeErrorToStore(client: Client) { val error = Error.Builder(Configuration("api-key"), RuntimeException(), null).build() client.errorStore.write(error) } - diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorConnectivityScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorConnectivityScenario.kt index acaa178216..7ca0ff67b3 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorConnectivityScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorConnectivityScenario.kt @@ -11,13 +11,13 @@ internal class AsyncErrorConnectivityScenario(config: Configuration, context: Context) : Scenario(config, context) { override fun run() { + val delivery = createSlowDelivery(context) + config.delivery = delivery super.run() - val apiClient = createSlowErrorApiClient(context) - Bugsnag.setErrorReportApiClient(apiClient) writeErrorToStore(Bugsnag.getClient()) - flushErrorStoreAsync(Bugsnag.getClient(), apiClient) - flushErrorStoreOnLaunch(Bugsnag.getClient(), apiClient) + flushErrorStoreAsync(Bugsnag.getClient()) + flushErrorStoreOnLaunch(Bugsnag.getClient()) Thread.sleep(50) } diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorDoubleFlushScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorDoubleFlushScenario.kt index 65b98b0090..4a0a66a230 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorDoubleFlushScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorDoubleFlushScenario.kt @@ -11,13 +11,12 @@ internal class AsyncErrorDoubleFlushScenario(config: Configuration, context: Context) : Scenario(config, context) { override fun run() { + config.delivery = createSlowDelivery(context) super.run() - val apiClient = createSlowErrorApiClient(context) - Bugsnag.setErrorReportApiClient(apiClient) writeErrorToStore(Bugsnag.getClient()) - flushErrorStoreAsync(Bugsnag.getClient(), apiClient) - flushErrorStoreAsync(Bugsnag.getClient(), apiClient) + flushErrorStoreAsync(Bugsnag.getClient()) + flushErrorStoreAsync(Bugsnag.getClient()) Thread.sleep(50) } diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorLaunchScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorLaunchScenario.kt index 87e5c8cf87..5be180e82d 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorLaunchScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/AsyncErrorLaunchScenario.kt @@ -11,13 +11,12 @@ internal class AsyncErrorLaunchScenario(config: Configuration, context: Context) : Scenario(config, context) { override fun run() { + config.delivery = createSlowDelivery(context) super.run() - val apiClient = createSlowErrorApiClient(context) - Bugsnag.setErrorReportApiClient(apiClient) writeErrorToStore(Bugsnag.getClient()) - flushErrorStoreOnLaunch(Bugsnag.getClient(), apiClient) - flushErrorStoreAsync(Bugsnag.getClient(), apiClient) + flushErrorStoreOnLaunch(Bugsnag.getClient()) + flushErrorStoreAsync(Bugsnag.getClient()) Thread.sleep(50) } diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt new file mode 100644 index 0000000000..f58d75c0b1 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt @@ -0,0 +1,28 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.createCustomHeaderDelivery + +/** + * Sends an unhandled exception which is cached on disk to Bugsnag, then sent on a separate launch, + * using a custom API client which modifies the request. + */ +internal class CustomClientErrorFlushScenario(config: Configuration, + context: Context) : Scenario(config, context) { + + override fun run() { + if ("DeliverReports" == eventMetaData) { + config.delivery = createCustomHeaderDelivery(context) + } + super.run() + + if ("DeliverReports" != eventMetaData) { + disableAllDelivery() + throw RuntimeException("ReportCacheScenario") + } + + } + +} diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.kt new file mode 100644 index 0000000000..639319a340 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.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.createCustomHeaderDelivery + +/** + * Sends a handled exception to Bugsnag using a custom API client which modifies the request. + */ +internal class CustomClientErrorScenario(config: Configuration, + context: Context) : Scenario(config, context) { + + override fun run() { + config.delivery = createCustomHeaderDelivery(context) + super.run() + Bugsnag.notify(RuntimeException("Hello")) + } + +} diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt new file mode 100644 index 0000000000..f57b48a781 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt @@ -0,0 +1,29 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.createCustomHeaderDelivery +import com.bugsnag.android.createDefaultDelivery + +/** + * Sends a session which is cached on disk to Bugsnag, then sent on a separate launch, + * using a custom API client which modifies the request. + */ +internal class CustomClientSessionFlushScenario(config: Configuration, + context: Context) : Scenario(config, context) { + + override fun run() { + super.run() + + if ("DeliverSessions" == eventMetaData) { + // simulate activity lifecycle callback occurring before api client can be set + Bugsnag.startSession() + config.delivery = createCustomHeaderDelivery(context) + } else { + disableAllDelivery() + Bugsnag.startSession() + } + } + +} diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.kt new file mode 100644 index 0000000000..954e3143be --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.kt @@ -0,0 +1,21 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.createCustomHeaderDelivery +import com.bugsnag.android.createDefaultDelivery + +/** + * Sends a session using a custom API client which modifies the request. + */ +internal class CustomClientSessionScenario(config: Configuration, + context: Context) : Scenario(config, context) { + + override fun run() { + config.delivery = createCustomHeaderDelivery(context) + super.run() + Bugsnag.startSession() + } + +} diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt index 1a04295df0..163f8c043e 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt @@ -1,9 +1,7 @@ package com.bugsnag.android.mazerunner.scenarios import android.content.Context -import com.bugsnag.android.Bugsnag -import com.bugsnag.android.Configuration -import com.bugsnag.android.NetworkException +import com.bugsnag.android.* abstract internal class Scenario(protected val config: Configuration, protected val context: Context) { @@ -19,18 +17,33 @@ abstract internal class Scenario(protected val config: Configuration, * Sets a NOP implementation for the Session Tracking API, preventing delivery */ protected fun disableSessionDelivery() { - Bugsnag.setSessionTrackingApiClient({ _, _, _ -> - throw NetworkException("Session Delivery NOP", RuntimeException("NOP")) - }) + val baseDelivery = Bugsnag.getClient().config.delivery + Bugsnag.getClient().config.delivery = object: Delivery { + override fun deliver(payload: SessionTrackingPayload?, config: Configuration?) { + throw DeliveryFailureException("Session Delivery NOP", RuntimeException("NOP")) + } + + override fun deliver(report: Report?, config: Configuration?) { + baseDelivery.deliver(report, config) + } + } } /** * Sets a NOP implementation for the Error Tracking API, preventing delivery */ protected fun disableReportDelivery() { - Bugsnag.setErrorReportApiClient({ _, _, _ -> - throw NetworkException("Error Delivery NOP", RuntimeException("NOP")) - }) + val baseDelivery = Bugsnag.getClient().config.delivery + Bugsnag.getClient().config.delivery = object: Delivery { + override fun deliver(payload: SessionTrackingPayload?, config: Configuration?) { + baseDelivery.deliver(payload, config) + } + + override fun deliver(report: Report?, config: Configuration?) { + throw DeliveryFailureException("Session Delivery NOP", RuntimeException("NOP")) + } + } + } /** diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BreadcrumbLifecycleCrashTest.java b/sdk/src/androidTest/java/com/bugsnag/android/BreadcrumbLifecycleCrashTest.java index 3223134e9f..f0f5cd6625 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BreadcrumbLifecycleCrashTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BreadcrumbLifecycleCrashTest.java @@ -22,11 +22,10 @@ public class BreadcrumbLifecycleCrashTest { */ @Before public void setUp() throws Exception { - Configuration configuration = new Configuration("api-key"); + Configuration configuration = BugsnagTestUtils.generateConfiguration(); Context context = InstrumentationRegistry.getContext(); SessionStore sessionStore = new SessionStore(configuration, context); - SessionTrackingApiClient apiClient = BugsnagTestUtils.generateSessionTrackingApiClient(); - sessionTracker = new SessionTracker(configuration, null, sessionStore, apiClient); + sessionTracker = new SessionTracker(configuration, null, sessionStore); } @Test diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index e4f38fe8aa..59eaaa1356 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -42,8 +42,7 @@ static SharedPreferences getSharedPrefs(Context context) { static Client generateClient() { Client client = new Client(InstrumentationRegistry.getContext(), "api-key"); - client.setErrorReportApiClient(generateErrorReportApiClient()); - client.setSessionTrackingApiClient(generateSessionTrackingApiClient()); + client.config.setDelivery(generateDelivery()); return client; } @@ -52,12 +51,14 @@ static Session generateSession() { } static Configuration generateConfiguration() { - return new Configuration("test"); + Configuration configuration = new Configuration("test"); + configuration.setDelivery(generateDelivery()); + return configuration; } static SessionTracker generateSessionTracker() { return new SessionTracker(generateConfiguration(), BugsnagTestUtils.generateClient(), - generateSessionStore(), generateSessionTrackingApiClient()); + generateSessionStore()); } @NonNull @@ -89,4 +90,19 @@ public void postReport(String urlString, } }; } + + public static Delivery generateDelivery() { + return new Delivery() { + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) + throws DeliveryFailureException {} + + @Override + public void deliver(Report report, + Configuration config) + throws DeliveryFailureException {} + + }; + } } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java index 875a1d571c..8e1696009d 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import android.content.Context; import android.support.test.InstrumentationRegistry; @@ -86,4 +87,29 @@ public void testSetSendThreads() throws Exception { assertFalse(config.getSendThreads()); } + @Test + public void testDefaultClientDelivery() { + assertFalse(client.config.getDelivery() instanceof DeliveryCompat); + } + + @Test + public void testCustomDeliveryOverride() { + Context context = InstrumentationRegistry.getContext(); + config = BugsnagTestUtils.generateConfiguration(); + Delivery customDelivery = new Delivery() { + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) + throws DeliveryFailureException {} + + @Override + public void deliver(Report report, + Configuration config) + throws DeliveryFailureException {} + + }; + config.setDelivery(customDelivery); + client = new Client(context, config); + assertEquals(customDelivery, client.config.getDelivery()); + } } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java index fb98304857..a1db8c19aa 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java @@ -219,25 +219,6 @@ public void testFullManifestConfig() { assertEquals(true, newConfig.shouldAutoCaptureSessions()); } - @Test - public void testSessionTrackerApiClient() throws Exception { - Client client = new Client(InstrumentationRegistry.getContext(), "api-key"); - assertTrue(client.sessionTracker.getApiClient() instanceof DefaultHttpClient); - - SessionTrackingApiClient customClient = new SessionTrackingApiClient() { - @Override - public void postSessionTrackingPayload(String urlString, - SessionTrackingPayload payload, - Map headers) - throws NetworkException, BadResponseException { - - } - }; - client.setSessionTrackingApiClient(customClient); - assertFalse(client.sessionTracker.getApiClient() instanceof DefaultHttpClient); - assertEquals(customClient, client.sessionTracker.getApiClient()); - } - @Test public void testMaxBreadcrumbs() { Client client = generateClient(); @@ -282,4 +263,10 @@ public void testClientClearTab() { client.clearTab("drink"); assertTrue(client.getMetaData().getTab("drink").isEmpty()); } + + @Test(expected = IllegalArgumentException.class) + public void testApiClientNullValidation() { + generateClient().setSessionTrackingApiClient(null); + } + } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java index ddf1c0ad18..2971146858 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java @@ -4,6 +4,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import android.support.test.filters.SmallTest; @@ -164,4 +165,20 @@ public void testOverrideCodeBundleId() throws Exception { config.setCodeBundleId("abc123"); assertEquals("abc123", config.getCodeBundleId()); } + + @Test + public void testSetDelivery() { + Configuration configuration = new Configuration("api-key"); + assertNull(configuration.getDelivery()); + Delivery delivery = BugsnagTestUtils.generateDelivery(); + configuration.setDelivery(delivery); + + assertFalse(configuration.getDelivery() instanceof DeliveryCompat); + assertEquals(delivery, configuration.getDelivery()); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetNullDelivery() { + config.setDelivery(null); + } } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java new file mode 100644 index 0000000000..919e280967 --- /dev/null +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -0,0 +1,102 @@ +package com.bugsnag.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Before; +import org.junit.Test; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +public class DeliveryCompatTest { + + private final Configuration config = BugsnagTestUtils.generateConfiguration(); + private DeliveryCompat deliveryCompat; + + private AtomicInteger customCount; + + /** + * Generates a Delivery instance that increments a counter on each request + * + * @throws Exception if setup failed + */ + @Before + public void setUp() throws Exception { + customCount = new AtomicInteger(); + deliveryCompat = new DeliveryCompat(); + } + + @Test + public void deliverReport() throws Exception { + Report report = null; + deliveryCompat.deliver(report, config); + + deliveryCompat.errorReportApiClient = new ErrorReportApiClient() { + @Override + public void postReport(String urlString, + Report report, + Map headers) + throws NetworkException, BadResponseException { + customCount.incrementAndGet(); + } + }; + + deliveryCompat.deliver(report, config); + assertEquals(1, customCount.get()); + } + + @Test + public void deliverSession() throws Exception { + SessionTrackingPayload payload = null; + deliveryCompat.deliver(payload, config); + + deliveryCompat.sessionTrackingApiClient = new SessionTrackingApiClient() { + @Override + public void postSessionTrackingPayload(String urlString, + SessionTrackingPayload payload, + Map headers) + throws NetworkException, BadResponseException { + customCount.incrementAndGet(); + } + }; + + deliveryCompat.deliver(payload, config); + assertEquals(1, customCount.get()); + } + + @Test + public void testClientCompat() { + Client client = BugsnagTestUtils.generateClient(); + Delivery delivery = client.config.getDelivery(); + assertFalse(delivery instanceof DeliveryCompat); + + ErrorReportApiClient errorReportApiClient = BugsnagTestUtils.generateErrorReportApiClient(); + client.setErrorReportApiClient(errorReportApiClient); + + assertTrue(client.config.getDelivery() instanceof DeliveryCompat); + + DeliveryCompat compat = (DeliveryCompat) client.config.getDelivery(); + assertEquals(errorReportApiClient, compat.errorReportApiClient); + assertNull(compat.sessionTrackingApiClient); + + SessionTrackingApiClient sessionClient + = BugsnagTestUtils.generateSessionTrackingApiClient(); + client.setSessionTrackingApiClient(sessionClient); + + assertEquals(errorReportApiClient, compat.errorReportApiClient); + assertEquals(sessionClient, compat.sessionTrackingApiClient); + } + + @Test(expected = DeliveryFailureException.class) + public void testExceptionConversion() throws Exception { + deliveryCompat.handleException(new NetworkException("", null)); + } + + @Test + public void testSwallowExceptionConversion() throws Exception { // no exception thrown + deliveryCompat.handleException(new BadResponseException("", 0)); + } +} diff --git a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java new file mode 100644 index 0000000000..9866644d08 --- /dev/null +++ b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java @@ -0,0 +1,33 @@ +package com.bugsnag.android; + +import static org.junit.Assert.assertEquals; + +import android.support.test.filters.MediumTest; +import android.support.test.runner.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; + +@RunWith(AndroidJUnit4.class) +@MediumTest +public class JsonWriterBugsnagTest { + + @Test(expected = IOException.class) + public void testClose() throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + OutputStreamWriter writer = new OutputStreamWriter(baos); + JsonWriter jsonWriter = new JsonWriter(writer); + + jsonWriter.beginObject().endObject(); + jsonWriter.flush(); + assertEquals("{}", new String(baos.toByteArray(), "UTF-8")); + + jsonWriter.close(); + writer.write(5); // can't write to a closed stream, throws IOException + } + +} diff --git a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java index 21cb6103d7..4ffb643e20 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java @@ -1,19 +1,3 @@ -/* - * Copyright (C) 2010 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - package com.bugsnag.android; import static org.junit.Assert.assertEquals; @@ -30,6 +14,22 @@ import java.math.BigDecimal; import java.math.BigInteger; +/* + * Copyright (C) 2010 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + @RunWith(AndroidJUnit4.class) @MediumTest public final class JsonWriterTest { diff --git a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java index 1eeb756fa1..4e6c7bba42 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java @@ -31,8 +31,9 @@ public class SessionTrackerTest { @Before public void setUp() throws Exception { configuration = new Configuration("test"); - sessionTracker = new SessionTracker(configuration, generateClient(), generateSessionStore(), - generateSessionTrackingApiClient()); + configuration.setDelivery(BugsnagTestUtils.generateDelivery()); + sessionTracker + = new SessionTracker(configuration, generateClient(), generateSessionStore()); configuration.setAutoCaptureSessions(true); user = new User(); } @@ -125,7 +126,7 @@ public void testBasicInForeground() throws Exception { public void testInForegroundDuration() throws Exception { long now = System.currentTimeMillis(); sessionTracker = new SessionTracker(configuration, generateClient(), - 0, generateSessionStore(), generateSessionTrackingApiClient()); + 0, generateSessionStore()); sessionTracker.updateForegroundTracker(ACTIVITY_NAME, false, now); assertEquals(0, sessionTracker.getDurationInForegroundMs(now)); @@ -146,7 +147,7 @@ public void testInForegroundDuration() throws Exception { @Test public void testZeroSessionTimeout() throws Exception { sessionTracker = new SessionTracker(configuration, generateClient(), - 0, generateSessionStore(), generateSessionTrackingApiClient()); + 0, generateSessionStore()); long now = System.currentTimeMillis(); sessionTracker.updateForegroundTracker(ACTIVITY_NAME, true, now); @@ -161,7 +162,7 @@ public void testZeroSessionTimeout() throws Exception { @Test public void testSessionTimeout() throws Exception { sessionTracker = new SessionTracker(configuration, generateClient(), - 100, generateSessionStore(), generateSessionTrackingApiClient()); + 100, generateSessionStore()); long now = System.currentTimeMillis(); sessionTracker.updateForegroundTracker(ACTIVITY_NAME, true, now); diff --git a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java index 9b6d1e0f71..db0fad2378 100644 --- a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java +++ b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java @@ -1,11 +1,13 @@ package com.bugsnag.android; - import java.util.Locale; +/** + * @deprecated use {@link DeliveryFailureException} instead + */ +@Deprecated public class BadResponseException extends Exception { - public BadResponseException(String url, int responseCode) { - super(String.format(Locale.US, - "Got non-200 response code (%d) from %s", responseCode, url)); + public BadResponseException(String msg, int responseCode) { + super(String.format(Locale.US, "%s (%d)", msg, responseCode)); } } diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index 59e1ea8e05..17b8d4966c 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -298,7 +298,10 @@ public static void setUserName(final String name) { * https://docs.bugsnag.com/api/error-reporting/ * * @param errorReportApiClient the custom HTTP client implementation + * + * @deprecated use {@link Configuration#setDelivery(Delivery)} instead */ + @Deprecated public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { getClient().setErrorReportApiClient(errorReportApiClient); } @@ -312,11 +315,15 @@ public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorRe * the Bugsnag API. * * @param apiClient the custom HTTP client implementation + * + * @deprecated use {@link Configuration#setDelivery(Delivery)} instead */ + @Deprecated public static void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { getClient().setSessionTrackingApiClient(apiClient); } + /** * Add a "before notify" callback, to execute code before every * report to Bugsnag. diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 56fa7dc87b..788568719a 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -59,7 +59,6 @@ public class Client extends Observable implements Observer { static final String MF_AUTO_CAPTURE_SESSIONS = BUGSNAG_NAMESPACE + ".AUTO_CAPTURE_SESSIONS"; - @NonNull protected final Configuration config; private final Context appContext; @@ -77,8 +76,6 @@ public class Client extends Observable implements Observer { private final EventReceiver eventReceiver; final SessionTracker sessionTracker; - private ErrorReportApiClient errorReportApiClient; - private SessionTrackingApiClient sessionTrackingApiClient; /** * Initialize a Bugsnag client @@ -127,12 +124,14 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat ConnectivityManager cm = (ConnectivityManager) appContext.getSystemService(Context.CONNECTIVITY_SERVICE); - DefaultHttpClient defaultHttpClient = new DefaultHttpClient(cm); - errorReportApiClient = defaultHttpClient; - sessionTrackingApiClient = defaultHttpClient; + + //noinspection ConstantConditions + if (configuration.getDelivery() == null) { + configuration.setDelivery(new DefaultDelivery(cm)); + } sessionTracker = - new SessionTracker(configuration, this, sessionStore, sessionTrackingApiClient); + new SessionTracker(configuration, this, sessionStore); eventReceiver = new EventReceiver(this); // Set up and collect constant app and device diagnostics @@ -165,8 +164,6 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat + "breadcrumbs on API Levels below 14."); } - errorReportApiClient = new DefaultHttpClient(cm); - // populate from manifest (in the case where the constructor was called directly by the // User or no UUID was supplied) if (config.getBuildUUID() == null) { @@ -206,12 +203,13 @@ public void run() { config.addObserver(this); - // Flush any on-disk errors - errorStore.flushOnLaunch(errorReportApiClient); - boolean isNotProduction = !AppData.RELEASE_STAGE_PRODUCTION.equals( AppData.guessReleaseStage(appContext)); Logger.setEnabled(isNotProduction); + + + // Flush any on-disk errors + errorStore.flushOnLaunch(); } private class ConnectivityChangeReceiver extends BroadcastReceiver { @@ -223,7 +221,7 @@ public void onReceive(Context context, Intent intent) { boolean retryReports = networkInfo != null && networkInfo.isConnectedOrConnecting(); if (retryReports) { - errorStore.flushAsync(errorReportApiClient); + errorStore.flushAsync(); } } } @@ -631,21 +629,36 @@ void setUserName(String name, boolean notify) { } } + DeliveryCompat getAndSetDeliveryCompat() { + Delivery current = config.getDelivery(); + + if (current instanceof DeliveryCompat) { + return (DeliveryCompat)current; + } else { + DeliveryCompat compat = new DeliveryCompat(); + config.setDelivery(compat); + return compat; + } + } + @SuppressWarnings("ConstantConditions") + @Deprecated void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { if (errorReportApiClient == null) { throw new IllegalArgumentException("ErrorReportApiClient cannot be null."); } - this.errorReportApiClient = errorReportApiClient; + DeliveryCompat compat = getAndSetDeliveryCompat(); + compat.errorReportApiClient = errorReportApiClient; } @SuppressWarnings("ConstantConditions") + @Deprecated void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { if (apiClient == null) { throw new IllegalArgumentException("SessionTrackingApiClient cannot be null."); } - this.sessionTrackingApiClient = apiClient; - sessionTracker.setApiClient(apiClient); + DeliveryCompat compat = getAndSetDeliveryCompat(); + compat.sessionTrackingApiClient = apiClient; } /** @@ -914,7 +927,7 @@ public void run() { break; case ASYNC_WITH_CACHE: errorStore.write(error); - errorStore.flushAsync(errorReportApiClient); + errorStore.flushAsync(); break; default: break; @@ -1237,16 +1250,12 @@ public void disableExceptionHandler() { void deliver(@NonNull Report report, @NonNull Error error) { try { - errorReportApiClient.postReport(config.getEndpoint(), report, - config.getErrorApiHeaders()); + config.getDelivery().deliver(report, config); Logger.info("Sent 1 new error to Bugsnag"); - } catch (NetworkException exception) { - Logger.info("Could not send error(s) to Bugsnag, saving to disk to send later"); - - // Save error to disk for later sending + } catch (DeliveryFailureException exception) { + Logger.warn("Could not send error(s) to Bugsnag," + + " saving to disk to send later", exception); errorStore.write(error); - } catch (BadResponseException exception) { - Logger.info("Bad response when sending data to Bugsnag"); } catch (Exception exception) { Logger.warn("Problem sending error to Bugsnag", exception); } diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 2e84476482..e7be4effbb 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -8,7 +8,6 @@ import java.util.Collection; import java.util.Date; import java.util.HashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Observable; @@ -56,6 +55,8 @@ public class Configuration extends Observable implements Observer { private String codeBundleId; private String notifierType; + private Delivery delivery; + /** * Construct a new Bugsnag configuration object * @@ -494,7 +495,42 @@ String getNotifierType() { return notifierType; } - Map getErrorApiHeaders() { + /** + * Retrieves the delivery used to make HTTP requests to Bugsnag. + * + * @return the current delivery + */ + @NonNull + public Delivery getDelivery() { + return delivery; + } + + /** + * Sets the delivery used to make HTTP requests to Bugsnag. A default implementation is + * provided, but you may wish to use your own implementation if you have requirements such + * as pinning SSL certificates, for example. + *

+ * Any custom implementation must be capable of sending + * Error Reports + * and Sessions as + * documented at https://docs.bugsnag.com/api/ + * + * @param delivery the custom HTTP client implementation + */ + public void setDelivery(@NonNull Delivery delivery) { + //noinspection ConstantConditions + if (delivery == null) { + throw new IllegalArgumentException("Delivery cannot be null"); + } + this.delivery = delivery; + } + + /** + * Supplies the headers which must be used in any request sent to the Error Reporting API. + * + * @return the HTTP headers + */ + public Map getErrorApiHeaders() { Map map = new HashMap<>(); map.put(HEADER_API_PAYLOAD_VERSION, "4.0"); map.put(HEADER_API_KEY, apiKey); @@ -502,7 +538,12 @@ Map getErrorApiHeaders() { return map; } - Map getSessionApiHeaders() { + /** + * Supplies the headers which must be used in any request sent to the Session Tracking API. + * + * @return the HTTP headers + */ + public Map getSessionApiHeaders() { Map map = new HashMap<>(); map.put(HEADER_API_PAYLOAD_VERSION, "1.0"); map.put(HEADER_API_KEY, apiKey); diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java similarity index 50% rename from sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java rename to sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index 348b65010b..3c65791d7c 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -3,55 +3,53 @@ import android.net.ConnectivityManager; import android.net.NetworkInfo; +import java.io.BufferedWriter; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.net.HttpURLConnection; import java.net.URL; +import java.nio.charset.Charset; import java.util.Map; -class DefaultHttpClient implements ErrorReportApiClient, SessionTrackingApiClient { +class DefaultDelivery implements Delivery { private final ConnectivityManager connectivityManager; - DefaultHttpClient(ConnectivityManager connectivityManager) { + DefaultDelivery(ConnectivityManager connectivityManager) { this.connectivityManager = connectivityManager; } @Override - public void postReport(String urlString, - Report report, - Map headers) - throws NetworkException, BadResponseException { + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { + String endpoint = config.getSessionEndpoint(); + int status = deliver(endpoint, payload, config.getSessionApiHeaders()); - int status = makeRequest(urlString, report, headers); - - if (status / 100 != 2) { - throw new BadResponseException(urlString, status); + if (status != 202) { + Logger.warn("Session API request failed with status " + status, null); } else { - Logger.info("Completed error API request"); + Logger.info("Completed session tracking request"); } } @Override - public void postSessionTrackingPayload(String urlString, - SessionTrackingPayload payload, - Map headers) - throws NetworkException, BadResponseException { - - int status = makeRequest(urlString, payload, headers); + public void deliver(Report report, + Configuration config) throws DeliveryFailureException { + String endpoint = config.getEndpoint(); + int status = deliver(endpoint, report, config.getErrorApiHeaders()); - if (status != 202) { - throw new BadResponseException(urlString, status); + if (status / 100 != 2) { + Logger.warn("Error API request failed with status " + status, null); } else { - Logger.info("Completed session tracking request"); + Logger.info("Completed error API request"); } } - private int makeRequest(String urlString, - JsonStream.Streamable streamable, - Map headers) throws NetworkException { - checkHasNetworkConnection(urlString); + int deliver(String urlString, + JsonStream.Streamable streamable, + Map headers) throws DeliveryFailureException { + checkHasNetworkConnection(); HttpURLConnection conn = null; try { @@ -65,35 +63,33 @@ private int makeRequest(String urlString, conn.addRequestProperty(entry.getKey(), entry.getValue()); } - OutputStream out = null; + JsonStream stream = null; try { - out = conn.getOutputStream(); - JsonStream stream = new JsonStream(new OutputStreamWriter(out)); + OutputStream out = conn.getOutputStream(); + Charset charset = Charset.forName("UTF-8"); + BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, charset)); + stream = new JsonStream(writer); streamable.toStream(stream); - stream.close(); } finally { - IOUtils.closeQuietly(out); + IOUtils.closeQuietly(stream); } - // End the request, get the response code return conn.getResponseCode(); } catch (IOException exception) { - throw new NetworkException(urlString, exception); + throw new DeliveryFailureException("IOException encountered in request", exception); } finally { IOUtils.close(conn); } } - private void checkHasNetworkConnection(String urlString) throws NetworkException { + private void checkHasNetworkConnection() throws DeliveryFailureException { NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo(); // conserve device battery by avoiding radio use if (!(activeNetworkInfo != null && activeNetworkInfo.isConnectedOrConnecting())) { - RuntimeException rex = new RuntimeException("No network connection available"); - throw new NetworkException(urlString, rex); + throw new DeliveryFailureException("No network connection available", null); } } - } diff --git a/sdk/src/main/java/com/bugsnag/android/Delivery.java b/sdk/src/main/java/com/bugsnag/android/Delivery.java new file mode 100644 index 0000000000..2bb75990c2 --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -0,0 +1,63 @@ +package com.bugsnag.android; + + +/** + * Implementations of this interface deliver Error Reports and Sessions captured to the Bugsnag API. + *

+ * A default {@link Delivery} implementation is provided as part of Bugsnag initialization, + * but you may wish to use your own implementation if you have requirements such + * as pinning SSL certificates, for example. + *

+ * Any custom implementation must be capable of sending + * + * Error Reports and Sessions as + * documented at https://docs.bugsnag.com/api/ + * + * @see DefaultDelivery + */ +public interface Delivery { + + /** + * Posts an array of sessions to the Bugsnag Session Tracking API. + *

+ * This request must be delivered to the endpoint at {@link Configuration#getSessionEndpoint()}, + * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. + *

+ * If the request was not successful and you wish to try again later, throw + * {@link DeliveryFailureException}. The notifier will cache the payload and initiate delivery + * at a future time. For example, if you are unable to obtain a network connection, it would + * make sense to attempt delivery later on, whereas if the status code was 400, it would not. + *

+ * See + * https://docs.bugsnag.com/api/sessions/ + * + * @param payload The session tracking payload + * @param config The configuration by which this request will be sent + * @throws DeliveryFailureException when delivery is not successful and + * the report should be stored for future attempts. + */ + void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException; + + /** + * Posts an Error Report to the Bugsnag Error Reporting API. + *

+ * This request must be delivered to the endpoint at {@link Configuration#getSessionEndpoint()}, + * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. + *

+ * If the request was not successful and you wish to try again later, throw + * {@link DeliveryFailureException}. The notifier will cache the payload and initiate delivery + * at a future time. For example, if you are unable to obtain a network connection, it would + * make sense to attempt delivery later on, whereas if the status code was 400, it would not. + *

+ * See + * https://docs.bugsnag.com/api/error-reporting/ + * + * @param report The error report + * @param config The configuration by which this request will be sent + * @throws DeliveryFailureException when delivery is not successful and + * the report should be stored for future attempts. + */ + void deliver(Report report, + Configuration config) throws DeliveryFailureException; +} diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java new file mode 100644 index 0000000000..a277f94c1b --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -0,0 +1,46 @@ +package com.bugsnag.android; + +/** + * A compatibility implementation of {@link Delivery} which wraps {@link ErrorReportApiClient} and + * {@link SessionTrackingApiClient}. This class allows for backwards compatibility for users still + * utilising the old API, and should be removed in the next major version. + */ +class DeliveryCompat implements Delivery { + + volatile ErrorReportApiClient errorReportApiClient; + volatile SessionTrackingApiClient sessionTrackingApiClient; + + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { + if (sessionTrackingApiClient != null) { + try { + sessionTrackingApiClient.postSessionTrackingPayload(config.getSessionEndpoint(), + payload, config.getSessionApiHeaders()); + } catch (Throwable throwable) { + handleException(throwable); + } + } + } + + @Override + public void deliver(Report report, Configuration config) throws DeliveryFailureException { + if (errorReportApiClient != null) { + try { + errorReportApiClient.postReport(config.getEndpoint(), + report, config.getErrorApiHeaders()); + } catch (Throwable throwable) { + handleException(throwable); + } + } + } + + void handleException(Throwable throwable) throws DeliveryFailureException { + if (throwable instanceof NetworkException) { + throw new DeliveryFailureException(throwable.getMessage(), throwable); + } else { + Logger.warn("Ignoring Exception, this API is deprecated", throwable); + } + } + +} diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java b/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java new file mode 100644 index 0000000000..38172cb46b --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java @@ -0,0 +1,17 @@ +package com.bugsnag.android; + +/** + * This should be thrown if delivery of a request was not successful and you wish to try again + * later. The notifier will cache the payload and initiate delivery at a future time. + * + * @see Delivery + */ +public class DeliveryFailureException extends Exception { + public DeliveryFailureException(String message) { + super(message); + } + + public DeliveryFailureException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java index a67b05c43b..1c21bbb4eb 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java @@ -7,9 +7,10 @@ * place of the default implementation, by calling * {@link Bugsnag#setErrorReportApiClient(ErrorReportApiClient)} * - * @see DefaultHttpClient + * @deprecated use {@link Delivery} to send error reports */ @SuppressWarnings("WeakerAccess") +@Deprecated public interface ErrorReportApiClient { /** diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java index c6f32711e4..7afc6a7ec3 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java @@ -50,11 +50,11 @@ public int compare(File lhs, File rhs) { "/bugsnag-errors/", 128, ERROR_REPORT_COMPARATOR); } - void flushOnLaunch(final ErrorReportApiClient errorReportApiClient) { + void flushOnLaunch() { final List crashReports = findLaunchCrashReports(); if (crashReports.isEmpty() || config.getLaunchCrashThresholdMs() == 0) { - flushAsync(errorReportApiClient); // if disabled or no startup crash, flush async + flushAsync(); // if disabled or no startup crash, flush async } else { // Block the main thread for a 2 second interval as the app may crash very soon. @@ -66,7 +66,7 @@ void flushOnLaunch(final ErrorReportApiClient errorReportApiClient) { Async.run(new Runnable() { @Override public void run() { - flushReports(crashReports, errorReportApiClient); + flushReports(crashReports); flushOnLaunchCompleted = true; } }); @@ -88,7 +88,7 @@ public void run() { /** * Flush any on-disk errors to Bugsnag */ - void flushAsync(final ErrorReportApiClient errorReportApiClient) { + void flushAsync() { if (storeDirectory == null) { return; } @@ -97,7 +97,7 @@ void flushAsync(final ErrorReportApiClient errorReportApiClient) { Async.run(new Runnable() { @Override public void run() { - flushReports(findStoredFiles(), errorReportApiClient); + flushReports(findStoredFiles()); } }); } catch (RejectedExecutionException exception) { @@ -105,15 +105,14 @@ public void run() { } } - private void flushReports(Collection storedReports, - ErrorReportApiClient apiClient) { + private void flushReports(Collection storedReports) { if (!storedReports.isEmpty() && semaphore.tryAcquire(1)) { try { Logger.info(String.format(Locale.US, "Sending %d saved error(s) to Bugsnag", storedReports.size())); for (File errorFile : storedReports) { - flushErrorReport(errorFile, apiClient); + flushErrorReport(errorFile); } } finally { semaphore.release(1); @@ -121,18 +120,17 @@ private void flushReports(Collection storedReports, } } - private void flushErrorReport(File errorFile, ErrorReportApiClient errorReportApiClient) { + private void flushErrorReport(File errorFile) { try { Report report = new Report(config.getApiKey(), errorFile); - errorReportApiClient.postReport(config.getEndpoint(), report, - config.getErrorApiHeaders()); + config.getDelivery().deliver(report, config); deleteStoredFiles(Collections.singleton(errorFile)); Logger.info("Deleting sent error file " + errorFile.getName()); - } catch (NetworkException exception) { + } catch (DeliveryFailureException exception) { cancelQueuedFiles(Collections.singleton(errorFile)); Logger.warn("Could not send previously saved error(s)" - + " to Bugsnag, will try again later", exception); + + " to Bugsnag, will try again later", exception); } catch (Exception exception) { deleteStoredFiles(Collections.singleton(errorFile)); Logger.warn("Problem sending unsent error from disk", exception); diff --git a/sdk/src/main/java/com/bugsnag/android/FileStore.java b/sdk/src/main/java/com/bugsnag/android/FileStore.java index 8aedc53bfd..4e26717b34 100644 --- a/sdk/src/main/java/com/bugsnag/android/FileStore.java +++ b/sdk/src/main/java/com/bugsnag/android/FileStore.java @@ -4,8 +4,11 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import java.io.BufferedWriter; import java.io.File; +import java.io.FileOutputStream; import java.io.FileWriter; +import java.io.OutputStreamWriter; import java.io.Writer; import java.util.ArrayList; import java.util.Arrays; @@ -83,23 +86,21 @@ String write(@NonNull T streamable) { String filename = getFilename(streamable); - Writer out = null; + JsonStream stream = null; lock.lock(); try { - out = new FileWriter(filename); - - JsonStream stream = new JsonStream(out); + FileOutputStream fos = new FileOutputStream(filename); + Writer out = new BufferedWriter(new OutputStreamWriter(fos, "UTF-8")); + stream = new JsonStream(out); stream.value(streamable); - stream.close(); - Logger.info(String.format("Saved unsent payload to disk (%s) ", filename)); return filename; } catch (Exception exception) { Logger.warn(String.format("Couldn't save unsent payload to disk (%s) ", filename), exception); } finally { - IOUtils.closeQuietly(out); + IOUtils.closeQuietly(stream); lock.unlock(); } return null; diff --git a/sdk/src/main/java/com/bugsnag/android/JsonStream.java b/sdk/src/main/java/com/bugsnag/android/JsonStream.java index 21147a9c1d..64a7049543 100644 --- a/sdk/src/main/java/com/bugsnag/android/JsonStream.java +++ b/sdk/src/main/java/com/bugsnag/android/JsonStream.java @@ -3,9 +3,12 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import java.io.BufferedReader; import java.io.File; -import java.io.FileReader; +import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStreamReader; +import java.io.Reader; import java.io.Writer; public class JsonStream extends JsonWriter { @@ -57,9 +60,10 @@ public void value(@NonNull File file) throws IOException { beforeValue(false); // add comma if in array // Copy the file contents onto the stream - FileReader input = null; + Reader input = null; try { - input = new FileReader(file); + FileInputStream fis = new FileInputStream(file); + input = new BufferedReader(new InputStreamReader(fis, "UTF-8")); IOUtils.copy(input, out); } finally { IOUtils.closeQuietly(input); diff --git a/sdk/src/main/java/com/bugsnag/android/NetworkException.java b/sdk/src/main/java/com/bugsnag/android/NetworkException.java index f473b15810..6023a0fc58 100644 --- a/sdk/src/main/java/com/bugsnag/android/NetworkException.java +++ b/sdk/src/main/java/com/bugsnag/android/NetworkException.java @@ -2,9 +2,12 @@ import java.io.IOException; +/** + * @deprecated use {@link DeliveryFailureException} instead + */ +@Deprecated public class NetworkException extends IOException { - public NetworkException(String url, Exception ex) { - super(String.format("Network error when posting to %s", url)); - initCause(ex); + public NetworkException(String msg, Throwable cause) { + super(msg, cause); } } diff --git a/sdk/src/main/java/com/bugsnag/android/Notifier.java b/sdk/src/main/java/com/bugsnag/android/Notifier.java index 3b9c6318b0..8b424cb507 100644 --- a/sdk/src/main/java/com/bugsnag/android/Notifier.java +++ b/sdk/src/main/java/com/bugsnag/android/Notifier.java @@ -9,7 +9,7 @@ */ public class Notifier implements JsonStream.Streamable { static final String NOTIFIER_NAME = "Android Bugsnag Notifier"; - static final String NOTIFIER_VERSION = "4.3.4"; + static final String NOTIFIER_VERSION = "4.4.0"; static final String NOTIFIER_URL = "https://bugsnag.com"; private String name; private String version; diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index b9d797be07..0348b81173 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -30,7 +30,6 @@ class SessionTracker implements Application.ActivityLifecycleCallbacks { private final long timeoutMs; private final Client client; private final SessionStore sessionStore; - private SessionTrackingApiClient apiClient; // This most recent time an Activity was stopped. private AtomicLong activityLastStoppedAtMs = new AtomicLong(0); @@ -40,18 +39,16 @@ class SessionTracker implements Application.ActivityLifecycleCallbacks { private AtomicReference currentSession = new AtomicReference<>(); private Semaphore flushingRequest = new Semaphore(1); - SessionTracker(Configuration configuration, Client client, SessionStore sessionStore, - SessionTrackingApiClient apiClient) { - this(configuration, client, DEFAULT_TIMEOUT_MS, sessionStore, apiClient); + SessionTracker(Configuration configuration, Client client, SessionStore sessionStore) { + this(configuration, client, DEFAULT_TIMEOUT_MS, sessionStore); } SessionTracker(Configuration configuration, Client client, long timeoutMs, - SessionStore sessionStore, SessionTrackingApiClient apiClient) { + SessionStore sessionStore) { this.configuration = configuration; this.client = client; this.timeoutMs = timeoutMs; this.sessionStore = sessionStore; - this.apiClient = apiClient; } /** @@ -69,14 +66,6 @@ void startNewSession(@NonNull Date date, @Nullable User user, boolean autoCaptur trackSessionIfNeeded(session); } - void setApiClient(SessionTrackingApiClient apiClient) { - this.apiClient = apiClient; - } - - SessionTrackingApiClient getApiClient() { - return apiClient; - } - /** * Determines whether or not a session should be tracked. If this is true, the session will be * stored and sent to the Bugsnag API, otherwise no action will occur in this method. @@ -101,13 +90,12 @@ public void run() { new SessionTrackingPayload(session, client.appData); try { - apiClient.postSessionTrackingPayload(endpoint, payload, - configuration.getSessionApiHeaders()); - } catch (NetworkException exception) { // store for later sending - Logger.info("Failed to post session payload"); + configuration.getDelivery().deliver(payload, configuration); + } catch (DeliveryFailureException exception) { // store for later sending + Logger.warn("Storing session payload for future delivery", exception); sessionStore.write(session); - } catch (BadResponseException exception) { // drop bad data - Logger.warn("Invalid session tracking payload", exception); + } catch (Exception exception) { + Logger.warn("Dropping invalid session tracking payload", exception); } } }); @@ -168,15 +156,14 @@ void flushStoredSessions() { //FUTURE:SM Reduce duplication here and above try { - final String endpoint = configuration.getSessionEndpoint(); - apiClient.postSessionTrackingPayload(endpoint, payload, - configuration.getSessionApiHeaders()); + configuration.getDelivery().deliver(payload, configuration); sessionStore.deleteStoredFiles(storedFiles); - } catch (NetworkException exception) { // store for later sending + } catch (DeliveryFailureException exception) { sessionStore.cancelQueuedFiles(storedFiles); - Logger.info("Failed to post stored session payload"); - } catch (BadResponseException exception) { // drop bad data - Logger.warn("Invalid session tracking payload", exception); + Logger.warn("Leaving session payload for future delivery", exception); + } catch (Exception exception) { + // drop bad data + Logger.warn("Deleting invalid session tracking payload", exception); sessionStore.deleteStoredFiles(storedFiles); } } diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java index f82f05288f..cabd2d6b23 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java @@ -7,8 +7,9 @@ * of this client can be used in place of the default implementation, by calling * {@link Bugsnag#setSessionTrackingApiClient(SessionTrackingApiClient)} * - * @see DefaultHttpClient + * @deprecated use {@link Delivery} to send sessions */ +@Deprecated public interface SessionTrackingApiClient { /**