From 6395139a0779122b530c9de4b8a475150bdbe722 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 24 Apr 2018 14:00:49 +0100 Subject: [PATCH 01/43] test: add custom error client flush scenario adds a mazeruner scenario that ensures a custom error api client is always used when flushing stored error reports on launch --- Gemfile.lock | 9 +++--- features/custom_client.feature | 10 ++++++ .../com/bugsnag/android/TestHarnessHooks.kt | 5 +++ .../CustomClientErrorFlushScenario.kt | 31 +++++++++++++++++++ .../main/java/com/bugsnag/android/Client.java | 20 +++++++++--- 5 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 features/custom_client.feature create mode 100644 mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt 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..1024554710 --- /dev/null +++ b/features/custom_client.feature @@ -0,0 +1,10 @@ +Feature: Using custom API clients for reporting errors + +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/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index f83fd5768f..eaee3f6e7b 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -34,6 +34,11 @@ internal fun createSlowErrorApiClient(context: Context): ErrorReportApiClient { }) } +internal fun createDefaultHttpClient(context: Context): ErrorReportApiClient { + val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? + return DefaultHttpClient(cm) +} + 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/CustomClientErrorFlushScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt new file mode 100644 index 0000000000..045a6936b4 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt @@ -0,0 +1,31 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import android.util.Log +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.createDefaultHttpClient + +/** + * 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() { + super.run() + + if ("DeliverReports" == eventMetaData) { + Bugsnag.setErrorReportApiClient { urlString, report, headers -> + headers["Custom-Client"] = "Hello World" + createDefaultHttpClient(context).postReport(urlString, report, headers) + } + } else { + disableAllDelivery() + throw RuntimeException("ReportCacheScenario") + } + + } + +} diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index d9c18f8262..2b751ccf7d 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -59,6 +59,7 @@ public class Client extends Observable implements Observer { static final String MF_AUTO_CAPTURE_SESSIONS = BUGSNAG_NAMESPACE + ".AUTO_CAPTURE_SESSIONS"; + private volatile boolean clientInitialised = false; @NonNull protected final Configuration config; @@ -206,12 +207,23 @@ 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 + Async.run(new Runnable() { + @Override + public void run() { + try { + Thread.sleep(10); // allow users to set custom API clients + clientInitialised = true; + errorStore.flushOnLaunch(errorReportApiClient); + } catch (InterruptedException exception) { + Logger.warn("Failed to flush errors on launch", exception); + } + } + }); } private class ConnectivityChangeReceiver extends BroadcastReceiver { @@ -222,7 +234,7 @@ public void onReceive(Context context, Intent intent) { NetworkInfo networkInfo = cm.getActiveNetworkInfo(); boolean retryReports = networkInfo != null && networkInfo.isConnectedOrConnecting(); - if (retryReports) { + if (retryReports && clientInitialised) { errorStore.flushAsync(errorReportApiClient); } } From a2a06ef332c8d6b72fc934718ab30d5bd5ba6f4a Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 24 Apr 2018 15:48:49 +0100 Subject: [PATCH 02/43] test: manual session/notify scenarios with custom api client adds mazerunner scenarios for a manual session start, and notify, with a custom api client for both --- features/custom_client.feature | 13 ++++++++++ .../com/bugsnag/android/TestHarnessHooks.kt | 9 +++++-- .../CustomClientErrorFlushScenario.kt | 5 ++-- .../scenarios/CustomClientErrorScenario.kt | 24 +++++++++++++++++ .../scenarios/CustomClientSessionScenario.kt | 26 +++++++++++++++++++ 5 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.kt create mode 100644 mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.kt diff --git a/features/custom_client.feature b/features/custom_client.feature index 1024554710..afecaba1dd 100644 --- a/features/custom_client.feature +++ b/features/custom_client.feature @@ -1,5 +1,17 @@ Feature: Using custom API clients for reporting errors +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 @@ -8,3 +20,4 @@ Scenario: Set a custom error client and flush a stored error 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/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index eaee3f6e7b..fab29e34ed 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -3,6 +3,7 @@ package com.bugsnag.android import android.content.Context import android.net.ConnectivityManager import com.bugsnag.android.Bugsnag.client +import java.util.* /** * Accesses the session tracker and flushes all stored sessions @@ -34,7 +35,12 @@ internal fun createSlowErrorApiClient(context: Context): ErrorReportApiClient { }) } -internal fun createDefaultHttpClient(context: Context): ErrorReportApiClient { +internal fun createDefaultErrorClient(context: Context): ErrorReportApiClient { + val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? + return DefaultHttpClient(cm) +} + +internal fun createDefaultSessionClient(context: Context): SessionTrackingApiClient { val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? return DefaultHttpClient(cm) } @@ -43,4 +49,3 @@ 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/CustomClientErrorFlushScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt index 045a6936b4..f3f718a403 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt @@ -1,10 +1,9 @@ package com.bugsnag.android.mazerunner.scenarios import android.content.Context -import android.util.Log import com.bugsnag.android.Bugsnag import com.bugsnag.android.Configuration -import com.bugsnag.android.createDefaultHttpClient +import com.bugsnag.android.createDefaultErrorClient /** * Sends an unhandled exception which is cached on disk to Bugsnag, then sent on a separate launch, @@ -19,7 +18,7 @@ internal class CustomClientErrorFlushScenario(config: Configuration, if ("DeliverReports" == eventMetaData) { Bugsnag.setErrorReportApiClient { urlString, report, headers -> headers["Custom-Client"] = "Hello World" - createDefaultHttpClient(context).postReport(urlString, report, headers) + createDefaultErrorClient(context).postReport(urlString, report, headers) } } else { disableAllDelivery() 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..1817553824 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.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.createDefaultErrorClient + +/** + * 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() { + super.run() + + Bugsnag.setErrorReportApiClient { urlString, report, headers -> + headers["Custom-Client"] = "Hello World" + createDefaultErrorClient(context).postReport(urlString, report, headers) + } + Bugsnag.notify(RuntimeException("Hello")) + } + +} 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..24593e358c --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.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.createDefaultSessionClient + +/** + * 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() { + super.run() + + Bugsnag.setSessionTrackingApiClient { urlString, report, headers -> + headers["Custom-Client"] = "Hello World" + val sessionClient = createDefaultSessionClient(context) + sessionClient.postSessionTrackingPayload(urlString, report, headers) + } + + Bugsnag.startSession() + } + +} From 99c72febbedfa3e78cfc59d3be6634f9e5c6df14 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 24 Apr 2018 16:25:00 +0100 Subject: [PATCH 03/43] test: session flush custom client scenario adds a mazerunner scenario with a custom api client flushing a stored session --- features/custom_client.feature | 11 ++++++- .../CustomClientSessionFlushScenario.kt | 33 +++++++++++++++++++ .../main/java/com/bugsnag/android/Client.java | 4 +-- .../com/bugsnag/android/SessionTracker.java | 6 ++++ 4 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt diff --git a/features/custom_client.feature b/features/custom_client.feature index afecaba1dd..2c24c09cef 100644 --- a/features/custom_client.feature +++ b/features/custom_client.feature @@ -1,5 +1,15 @@ 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 @@ -20,4 +30,3 @@ Scenario: Set a custom error client and flush a stored error 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/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..bd936019b6 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt @@ -0,0 +1,33 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.createDefaultSessionClient + +/** + * 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() + + Bugsnag.setSessionTrackingApiClient { urlString, report, headers -> + headers["Custom-Client"] = "Hello World" + val sessionClient = createDefaultSessionClient(context) + sessionClient.postSessionTrackingPayload(urlString, report, headers) + } + } else { + disableAllDelivery() + Bugsnag.startSession() + } + } + +} diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 2b751ccf7d..5ca4c331ad 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -59,7 +59,7 @@ public class Client extends Observable implements Observer { static final String MF_AUTO_CAPTURE_SESSIONS = BUGSNAG_NAMESPACE + ".AUTO_CAPTURE_SESSIONS"; - private volatile boolean clientInitialised = false; + volatile boolean clientInitialised = false; @NonNull protected final Configuration config; @@ -217,13 +217,13 @@ public void run() { public void run() { try { Thread.sleep(10); // allow users to set custom API clients - clientInitialised = true; errorStore.flushOnLaunch(errorReportApiClient); } catch (InterruptedException exception) { Logger.warn("Failed to flush errors on launch", exception); } } }); + clientInitialised = true; } private class ConnectivityChangeReceiver extends BroadcastReceiver { diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index a4ce7b3859..2be4d30500 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -94,6 +94,12 @@ private void trackSessionIfNeeded(final Session session) { Async.run(new Runnable() { @Override public void run() { + + if (!client.clientInitialised) { // store + sessionStore.write(session); + return; + } + //FUTURE:SM It would be good to optimise this flushStoredSessions(); From 2ecbc3537917f249f291e2d18079bdc468aafa53 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 24 Apr 2018 16:25:48 +0100 Subject: [PATCH 04/43] style: improved indent of feature --- features/custom_client.feature | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/features/custom_client.feature b/features/custom_client.feature index 2c24c09cef..86e3ae9160 100644 --- a/features/custom_client.feature +++ b/features/custom_client.feature @@ -1,14 +1,14 @@ 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 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 From 7be488eef9a9061ca60b93d4c1700b5b3f136037 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 1 May 2018 11:21:43 +0100 Subject: [PATCH 05/43] perf: use buffered streams for IO FileReader and FileWriter are not buffered, meaning they are slow. This change wraps the stream in a buffered reader/writer which in an (unscientific) unit test of the ErrorStore, reduced the write of an error from ~230ms to ~115ms. This also allows us to specify the charset encoding explicitly. --- .../java/com/bugsnag/android/DefaultHttpClient.java | 6 +++++- sdk/src/main/java/com/bugsnag/android/FileStore.java | 6 +++++- sdk/src/main/java/com/bugsnag/android/JsonStream.java | 10 +++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java index 348b65010b..aa77868358 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java @@ -3,11 +3,13 @@ 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 { @@ -69,7 +71,9 @@ private int makeRequest(String urlString, try { out = conn.getOutputStream(); - JsonStream stream = new JsonStream(new OutputStreamWriter(out)); + Charset charset = Charset.forName("UTF-8"); + BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, charset)); + JsonStream stream = new JsonStream(writer); streamable.toStream(stream); stream.close(); } finally { diff --git a/sdk/src/main/java/com/bugsnag/android/FileStore.java b/sdk/src/main/java/com/bugsnag/android/FileStore.java index 0593c2bb9c..13f78fd6fe 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; @@ -69,7 +72,8 @@ String write(@NonNull T streamable) { Writer out = null; try { - out = new FileWriter(filename); + FileOutputStream fos = new FileOutputStream(filename); + out = new BufferedWriter(new OutputStreamWriter(fos, "UTF-8")); JsonStream stream = new JsonStream(out); stream.value(streamable); diff --git a/sdk/src/main/java/com/bugsnag/android/JsonStream.java b/sdk/src/main/java/com/bugsnag/android/JsonStream.java index d440f939c1..6cd345e7ef 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 { @@ -53,9 +56,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); From eb3501f866f00aec69243804fb3a06bd6c5df760 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 14:00:44 +0100 Subject: [PATCH 06/43] revert previous implementation of client error flushing on launch --- .../main/java/com/bugsnag/android/Client.java | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 5ca4c331ad..de8700704e 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -59,8 +59,6 @@ public class Client extends Observable implements Observer { static final String MF_AUTO_CAPTURE_SESSIONS = BUGSNAG_NAMESPACE + ".AUTO_CAPTURE_SESSIONS"; - volatile boolean clientInitialised = false; - @NonNull protected final Configuration config; private final Context appContext; @@ -211,19 +209,9 @@ public void run() { AppData.guessReleaseStage(appContext)); Logger.setEnabled(isNotProduction); + // Flush any on-disk errors - Async.run(new Runnable() { - @Override - public void run() { - try { - Thread.sleep(10); // allow users to set custom API clients - errorStore.flushOnLaunch(errorReportApiClient); - } catch (InterruptedException exception) { - Logger.warn("Failed to flush errors on launch", exception); - } - } - }); - clientInitialised = true; + errorStore.flushOnLaunch(errorReportApiClient); } private class ConnectivityChangeReceiver extends BroadcastReceiver { @@ -234,7 +222,7 @@ public void onReceive(Context context, Intent intent) { NetworkInfo networkInfo = cm.getActiveNetworkInfo(); boolean retryReports = networkInfo != null && networkInfo.isConnectedOrConnecting(); - if (retryReports && clientInitialised) { + if (retryReports) { errorStore.flushAsync(errorReportApiClient); } } From 7d5974ec50b7ecb72187b5ac5eb1c2d8d1af0db8 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 14:01:44 +0100 Subject: [PATCH 07/43] remove client initialised check from session tracker --- sdk/src/main/java/com/bugsnag/android/SessionTracker.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index 2be4d30500..a4ce7b3859 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -94,12 +94,6 @@ private void trackSessionIfNeeded(final Session session) { Async.run(new Runnable() { @Override public void run() { - - if (!client.clientInitialised) { // store - sessionStore.write(session); - return; - } - //FUTURE:SM It would be good to optimise this flushStoredSessions(); From a0928c073ab9429d7c20e7349b787d36178ffa12 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 14:17:06 +0100 Subject: [PATCH 08/43] feat: add skeleton for new delivery api --- .../com/bugsnag/android/Configuration.java | 13 ++++++++++-- .../com/bugsnag/android/DefaultDelivery.java | 21 +++++++++++++++++++ .../java/com/bugsnag/android/Delivery.java | 10 +++++++++ .../android/DeliveryFailureException.java | 21 +++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java create mode 100644 sdk/src/main/java/com/bugsnag/android/Delivery.java create mode 100644 sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 2e84476482..76be364d68 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -55,6 +55,7 @@ public class Configuration extends Observable implements Observer { = new ConcurrentLinkedQueue<>(); private String codeBundleId; private String notifierType; + private Delivery delivery; /** * Construct a new Bugsnag configuration object @@ -494,7 +495,15 @@ String getNotifierType() { return notifierType; } - Map getErrorApiHeaders() { + public Delivery getDelivery() { + return delivery; + } + + public void setDelivery(Delivery delivery) { + this.delivery = delivery; + } + + public Map getErrorApiHeaders() { Map map = new HashMap<>(); map.put(HEADER_API_PAYLOAD_VERSION, "4.0"); map.put(HEADER_API_KEY, apiKey); @@ -502,7 +511,7 @@ Map getErrorApiHeaders() { return map; } - Map getSessionApiHeaders() { + 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/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java new file mode 100644 index 0000000000..d0205ba118 --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -0,0 +1,21 @@ +package com.bugsnag.android; + +class DefaultDelivery implements Delivery { + + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { + + } + + @Override + public void deliver(Report report, + Configuration config) throws DeliveryFailureException { + + } + + private void deliver(JsonStream.Streamable streamable, + Configuration config) throws DeliveryFailureException { + + } +} 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..d05769f561 --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -0,0 +1,10 @@ +package com.bugsnag.android; + +interface Delivery { + + void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException; + + void deliver(Report report, + Configuration config) throws DeliveryFailureException; +} 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..6997eaab22 --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java @@ -0,0 +1,21 @@ +package com.bugsnag.android; + +public class DeliveryFailureException extends Exception { + + enum Reason { + CONNECTIVITY, + BAD_REQUEST + } + + final Reason reason; + + public DeliveryFailureException(Reason reason, String msg) { + super(msg); + this.reason = reason; + } + + public DeliveryFailureException(Reason reason, String msg, Throwable throwable) { + super(msg, throwable); + this.reason = reason; + } +} From 9bc26e90557fb3a1afb8459efd3e4d98ba8f148f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 14:45:45 +0100 Subject: [PATCH 09/43] feat: setup the default delivery when creating a client --- .../java/com/bugsnag/android/BugsnagTestUtils.java | 14 ++++++++++++++ .../java/com/bugsnag/android/ClientConfigTest.java | 8 ++++++++ .../com/bugsnag/android/ConfigurationTest.java | 10 ++++++++++ sdk/src/main/java/com/bugsnag/android/Client.java | 3 +++ .../java/com/bugsnag/android/Configuration.java | 2 +- 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index e4f38fe8aa..27ff0ef743 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -89,4 +89,18 @@ 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..39a38bea6d 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -3,6 +3,9 @@ import static org.junit.Assert.assertArrayEquals; 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.content.Context; import android.support.test.InstrumentationRegistry; @@ -86,4 +89,9 @@ public void testSetSendThreads() throws Exception { assertFalse(config.getSendThreads()); } + @Test + public void testClientSetsDelivery() { + assertTrue(client.getConfig().getDelivery() instanceof DefaultDelivery); + } + } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java index ddf1c0ad18..7226e0ef5e 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,13 @@ 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); + assertEquals(delivery, configuration.getDelivery()); + } } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index c620678f6c..144042c946 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -126,6 +126,9 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat ConnectivityManager cm = (ConnectivityManager) appContext.getSystemService(Context.CONNECTIVITY_SERVICE); + + configuration.setDelivery(new DefaultDelivery()); + DefaultHttpClient defaultHttpClient = new DefaultHttpClient(cm); errorReportApiClient = defaultHttpClient; sessionTrackingApiClient = defaultHttpClient; diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 76be364d68..bab1333d6d 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -55,7 +55,7 @@ public class Configuration extends Observable implements Observer { = new ConcurrentLinkedQueue<>(); private String codeBundleId; private String notifierType; - private Delivery delivery; + private volatile Delivery delivery; /** * Construct a new Bugsnag configuration object From 88e003cfdb5279cefc1fccb8d68f3ce7f0e1d95d Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 15:10:11 +0100 Subject: [PATCH 10/43] feat: implement defaultdelivery class (same as defaulthttpclient) --- .../com/bugsnag/android/Configuration.java | 2 +- .../com/bugsnag/android/DefaultDelivery.java | 83 ++++++++++++++++++- .../android/DeliveryFailureException.java | 2 +- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index bab1333d6d..76be364d68 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -55,7 +55,7 @@ public class Configuration extends Observable implements Observer { = new ConcurrentLinkedQueue<>(); private String codeBundleId; private String notifierType; - private volatile Delivery delivery; + private Delivery delivery; /** * Construct a new Bugsnag configuration object diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index d0205ba118..430c5a9bdf 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -1,21 +1,100 @@ package com.bugsnag.android; +import android.net.ConnectivityManager; +import android.net.NetworkInfo; + +import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.Map; + +import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; +import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; + class DefaultDelivery implements Delivery { + private final ConnectivityManager connectivityManager; + + DefaultDelivery(ConnectivityManager connectivityManager) { + this.connectivityManager = connectivityManager; + } + @Override public void deliver(SessionTrackingPayload payload, Configuration config) throws DeliveryFailureException { + String endpoint = config.getSessionEndpoint(); + int status = deliver(endpoint, payload, config.getSessionApiHeaders()); + if (status != 202) { + throw new DeliveryFailureException(REQUEST_FAILURE, + "Request failed with status " + status); + } else { + Logger.info("Completed session tracking request"); + } } @Override public void deliver(Report report, Configuration config) throws DeliveryFailureException { + String endpoint = config.getEndpoint(); + int status = deliver(endpoint, report, config.getErrorApiHeaders()); + + if (status / 100 != 2) { + throw new DeliveryFailureException(REQUEST_FAILURE, + "Request failed with status " + status); + } else { + Logger.info("Completed error API request"); + } } - private void deliver(JsonStream.Streamable streamable, - Configuration config) throws DeliveryFailureException { + private int deliver(String urlString, + JsonStream.Streamable streamable, + Map headers) throws DeliveryFailureException { + checkHasNetworkConnection(); + HttpURLConnection conn = null; + + try { + URL url = new URL(urlString); + conn = (HttpURLConnection) url.openConnection(); + conn.setDoOutput(true); + conn.setChunkedStreamingMode(0); + conn.addRequestProperty("Content-Type", "application/json"); + for (Map.Entry entry : headers.entrySet()) { + conn.addRequestProperty(entry.getKey(), entry.getValue()); + } + + OutputStream out = null; + + try { + out = conn.getOutputStream(); + JsonStream stream = new JsonStream(new OutputStreamWriter(out)); + streamable.toStream(stream); + stream.close(); + } finally { + IOUtils.closeQuietly(out); + } + + // End the request, get the response code + return conn.getResponseCode(); + } catch (IOException exception) { + throw new DeliveryFailureException(CONNECTIVITY, + "IOException encountered in request", exception); + } finally { + IOUtils.close(conn); + } } + + private void checkHasNetworkConnection() throws DeliveryFailureException { + NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo(); + + // conserve device battery by avoiding radio use + if (!(activeNetworkInfo != null && activeNetworkInfo.isConnectedOrConnecting())) { + throw new DeliveryFailureException(CONNECTIVITY, "No network connection available"); + } + } + } diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java b/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java index 6997eaab22..d0a3321395 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java @@ -4,7 +4,7 @@ public class DeliveryFailureException extends Exception { enum Reason { CONNECTIVITY, - BAD_REQUEST + REQUEST_FAILURE } final Reason reason; From 665378d32211065d40ae4909d0b5b538787eaa3f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 15:50:13 +0100 Subject: [PATCH 11/43] feat: change to using delivery rather than separate interfaces --- .../com/bugsnag/android/TestHarnessHooks.kt | 8 +-- .../android/BreadcrumbLifecycleCrashTest.java | 5 +- .../com/bugsnag/android/BugsnagTestUtils.java | 6 +- .../java/com/bugsnag/android/ClientTest.java | 19 ------ .../bugsnag/android/SessionTrackerTest.java | 10 +-- .../main/java/com/bugsnag/android/Client.java | 50 ++++++++------- .../java/com/bugsnag/android/Delivery.java | 2 +- .../java/com/bugsnag/android/ErrorStore.java | 40 +++++++----- .../com/bugsnag/android/SessionTracker.java | 61 ++++++++++--------- 9 files changed, 95 insertions(+), 106 deletions(-) diff --git a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index fab29e34ed..3b19ba621b 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -2,8 +2,6 @@ package com.bugsnag.android import android.content.Context import android.net.ConnectivityManager -import com.bugsnag.android.Bugsnag.client -import java.util.* /** * Accesses the session tracker and flushes all stored sessions @@ -13,11 +11,13 @@ internal fun flushAllSessions() { } internal fun flushErrorStoreAsync(client: Client, apiClient: ErrorReportApiClient) { - client.errorStore.flushAsync(apiClient) + client.setErrorReportApiClient(apiClient) + client.errorStore.flushAsync() } internal fun flushErrorStoreOnLaunch(client: Client, apiClient: ErrorReportApiClient) { - client.errorStore.flushOnLaunch(apiClient) + client.setErrorReportApiClient(apiClient) + client.errorStore.flushOnLaunch() } /** 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 27ff0ef743..d1b35ab325 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -52,12 +52,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 diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java index a14c781634..382caae06d 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 testClientAddToTab() { Client client = generateClient(); diff --git a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java index 1eeb756fa1..90d794fc11 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java @@ -31,8 +31,8 @@ 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 +125,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 +146,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 +161,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/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 144042c946..10aae75f8b 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -76,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 @@ -126,15 +124,11 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat ConnectivityManager cm = (ConnectivityManager) appContext.getSystemService(Context.CONNECTIVITY_SERVICE); + configuration.setDelivery(new DefaultDelivery(cm)); - configuration.setDelivery(new DefaultDelivery()); - - DefaultHttpClient defaultHttpClient = new DefaultHttpClient(cm); - errorReportApiClient = defaultHttpClient; - sessionTrackingApiClient = defaultHttpClient; 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 @@ -167,8 +161,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) { @@ -214,7 +206,7 @@ public void run() { // Flush any on-disk errors - errorStore.flushOnLaunch(errorReportApiClient); + errorStore.flushOnLaunch(); } private class ConnectivityChangeReceiver extends BroadcastReceiver { @@ -226,7 +218,7 @@ public void onReceive(Context context, Intent intent) { boolean retryReports = networkInfo != null && networkInfo.isConnectedOrConnecting(); if (retryReports) { - errorStore.flushAsync(errorReportApiClient); + errorStore.flushAsync(); } } } @@ -638,17 +630,16 @@ void setUserName(String name, boolean notify) { void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { if (errorReportApiClient == null) { throw new IllegalArgumentException("ErrorReportApiClient cannot be null."); - } - this.errorReportApiClient = errorReportApiClient; + }// FIXME +// this.errorReportApiClient = errorReportApiClient; } @SuppressWarnings("ConstantConditions") void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { if (apiClient == null) { throw new IllegalArgumentException("SessionTrackingApiClient cannot be null."); - } - this.sessionTrackingApiClient = apiClient; - sessionTracker.setApiClient(apiClient); + }// FIXME +// this.sessionTrackingApiClient = apiClient; } /** @@ -917,7 +908,7 @@ public void run() { break; case ASYNC_WITH_CACHE: errorStore.write(error); - errorStore.flushAsync(errorReportApiClient); + errorStore.flushAsync(); break; default: break; @@ -1240,16 +1231,23 @@ 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"); + } catch (DeliveryFailureException exception) { - // Save error to disk for later sending - errorStore.write(error); - } catch (BadResponseException exception) { - Logger.info("Bad response when sending data to Bugsnag"); + switch (exception.reason) { + + case CONNECTIVITY: + Logger.info("Could not send error(s) to Bugsnag, saving to disk to send later"); + // Save error to disk for later sending + errorStore.write(error); + break; + case REQUEST_FAILURE: + Logger.info("Bad response when sending data to Bugsnag"); + break; + default: + break; + } } catch (Exception exception) { Logger.warn("Problem sending error to Bugsnag", exception); } diff --git a/sdk/src/main/java/com/bugsnag/android/Delivery.java b/sdk/src/main/java/com/bugsnag/android/Delivery.java index d05769f561..fd1bccb51f 100644 --- a/sdk/src/main/java/com/bugsnag/android/Delivery.java +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -1,6 +1,6 @@ package com.bugsnag.android; -interface Delivery { +public interface Delivery { void deliver(SessionTrackingPayload payload, Configuration config) throws DeliveryFailureException; diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java index c6f32711e4..9662e753f9 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,19 +120,28 @@ 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) { - cancelQueuedFiles(Collections.singleton(errorFile)); - Logger.warn("Could not send previously saved error(s)" - + " to Bugsnag, will try again later", exception); - } catch (Exception exception) { + } catch (DeliveryFailureException exception) { + switch (exception.reason) { + case CONNECTIVITY: + cancelQueuedFiles(Collections.singleton(errorFile)); + Logger.warn("Could not send previously saved error(s)" + + " to Bugsnag, will try again later", exception); + break; + case REQUEST_FAILURE: + deleteStoredFiles(Collections.singleton(errorFile)); + Logger.warn("Problem sending unsent error from disk", exception); + break; + default: + break; + } + } catch (Exception exception) { // FIXME dupe deleteStoredFiles(Collections.singleton(errorFile)); Logger.warn("Problem sending unsent error from disk", exception); } diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index b9d797be07..35a217f739 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,19 @@ 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"); - sessionStore.write(session); - } catch (BadResponseException exception) { // drop bad data - Logger.warn("Invalid session tracking payload", exception); + configuration.getDelivery().deliver(payload, configuration); + } catch (DeliveryFailureException exception) { // store for later sending + switch (exception.reason) { + case CONNECTIVITY: + Logger.info("Failed to post session payload"); + sessionStore.write(session); + break; + case REQUEST_FAILURE: + Logger.warn("Invalid session tracking payload", exception); + break; + default: + break; + } } } }); @@ -168,16 +163,22 @@ void flushStoredSessions() { //FUTURE:SM Reduce duplication here and above try { - final String endpoint = configuration.getSessionEndpoint(); - apiClient.postSessionTrackingPayload(endpoint, payload, - configuration.getSessionApiHeaders()); - sessionStore.deleteStoredFiles(storedFiles); - } catch (NetworkException exception) { // store for later sending - sessionStore.cancelQueuedFiles(storedFiles); - Logger.info("Failed to post stored session payload"); - } catch (BadResponseException exception) { // drop bad data - Logger.warn("Invalid session tracking payload", exception); + configuration.getDelivery().deliver(payload, configuration); sessionStore.deleteStoredFiles(storedFiles); + } catch (DeliveryFailureException exception) { + switch (exception.reason) { + case CONNECTIVITY: // store for later sending + sessionStore.cancelQueuedFiles(storedFiles); + Logger.info("Failed to post stored session payload"); + break; + case REQUEST_FAILURE: + // drop bad data + Logger.warn("Invalid session tracking payload", exception); + sessionStore.deleteStoredFiles(storedFiles); + break; + default: + break; + } } } } finally { From b54aacf7bee7a3e832234c80e98cf3f153914a77 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 15:54:45 +0100 Subject: [PATCH 12/43] docs: add deprecation notice to api client interfaces --- sdk/src/main/java/com/bugsnag/android/Bugsnag.java | 2 ++ sdk/src/main/java/com/bugsnag/android/Client.java | 2 ++ sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java | 1 + .../main/java/com/bugsnag/android/ErrorReportApiClient.java | 3 +-- .../java/com/bugsnag/android/SessionTrackingApiClient.java | 3 +-- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index 59e1ea8e05..07656efa45 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -299,6 +299,7 @@ public static void setUserName(final String name) { * * @param errorReportApiClient the custom HTTP client implementation */ + @Deprecated public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { getClient().setErrorReportApiClient(errorReportApiClient); } @@ -313,6 +314,7 @@ public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorRe * * @param apiClient the custom HTTP client implementation */ + @Deprecated public static void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { getClient().setSessionTrackingApiClient(apiClient); } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 10aae75f8b..d461d1a5b0 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -627,6 +627,7 @@ void setUserName(String name, boolean notify) { } @SuppressWarnings("ConstantConditions") + @Deprecated void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { if (errorReportApiClient == null) { throw new IllegalArgumentException("ErrorReportApiClient cannot be null."); @@ -635,6 +636,7 @@ void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) } @SuppressWarnings("ConstantConditions") + @Deprecated void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { if (apiClient == null) { throw new IllegalArgumentException("SessionTrackingApiClient cannot be null."); diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java index 348b65010b..7575950851 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java @@ -10,6 +10,7 @@ import java.net.URL; import java.util.Map; +@Deprecated class DefaultHttpClient implements ErrorReportApiClient, SessionTrackingApiClient { private final ConnectivityManager connectivityManager; diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java index a67b05c43b..50ed5aa3a2 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java @@ -6,10 +6,9 @@ * Posts an error report to the Bugsnag API. Custom implementations of this client can be used in * place of the default implementation, by calling * {@link Bugsnag#setErrorReportApiClient(ErrorReportApiClient)} - * - * @see DefaultHttpClient */ @SuppressWarnings("WeakerAccess") +@Deprecated public interface ErrorReportApiClient { /** diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java index f82f05288f..ad83761a20 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java @@ -6,9 +6,8 @@ * Posts an array of sessions to the Bugsnag Session Tracking API. Custom implementations * of this client can be used in place of the default implementation, by calling * {@link Bugsnag#setSessionTrackingApiClient(SessionTrackingApiClient)} - * - * @see DefaultHttpClient */ +@Deprecated public interface SessionTrackingApiClient { /** From 78d340b089a2b21e9b7d487b6f87ba70c9a71048 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 May 2018 16:51:32 +0100 Subject: [PATCH 13/43] feat: add deliverycompat class to wrap deprecated http client apis --- .../com/bugsnag/android/BugsnagTestUtils.java | 3 +- .../com/bugsnag/android/ClientConfigTest.java | 5 - .../bugsnag/android/ConfigurationTest.java | 10 +- .../bugsnag/android/DeliveryCompatTest.java | 121 ++++++++++++++++++ .../main/java/com/bugsnag/android/Client.java | 13 +- .../com/bugsnag/android/Configuration.java | 10 +- .../com/bugsnag/android/DeliveryCompat.java | 55 ++++++++ 7 files changed, 203 insertions(+), 14 deletions(-) create mode 100644 sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java create mode 100644 sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index d1b35ab325..3ff319b856 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; } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java index 39a38bea6d..70c7c6f002 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -89,9 +89,4 @@ public void testSetSendThreads() throws Exception { assertFalse(config.getSendThreads()); } - @Test - public void testClientSetsDelivery() { - assertTrue(client.getConfig().getDelivery() instanceof DefaultDelivery); - } - } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java index 7226e0ef5e..0d044fd15c 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java @@ -172,6 +172,14 @@ public void testSetDelivery() { assertNull(configuration.getDelivery()); Delivery delivery = BugsnagTestUtils.generateDelivery(); configuration.setDelivery(delivery); - assertEquals(delivery, configuration.getDelivery()); + + assertTrue(configuration.getDelivery() instanceof DeliveryCompat); + DeliveryCompat compat = (DeliveryCompat) configuration.getDelivery(); + assertEquals(compat.delivery, delivery); + } + + @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..6dbdee304e --- /dev/null +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -0,0 +1,121 @@ +package com.bugsnag.android; + + +import org.junit.Before; +import org.junit.Test; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; +import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; +import static junit.framework.Assert.*; + +public class DeliveryCompatTest { + + private final Configuration config = BugsnagTestUtils.generateConfiguration(); + private DeliveryCompat deliveryCompat; + + private AtomicInteger defaultCount; + private AtomicInteger customCount; + + @Before + public void setUp() throws Exception { + defaultCount = new AtomicInteger(); + customCount = new AtomicInteger(); + + Delivery baseDelivery = new Delivery() { + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { + defaultCount.incrementAndGet(); + } + + @Override + public void deliver(Report report, + Configuration config) throws DeliveryFailureException { + defaultCount.incrementAndGet(); + } + }; + deliveryCompat = new DeliveryCompat(baseDelivery); + + } + + @Test + public void deliverReport() throws Exception { + Report report = null; + deliveryCompat.deliver(report, config); + assertEquals(1, defaultCount.get()); + + 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, defaultCount.get()); + 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, defaultCount.get()); + assertEquals(1, customCount.get()); + } + + @Test + public void testClientCompat() { + Client client = BugsnagTestUtils.generateClient(); + Delivery delivery = client.config.getDelivery(); + assertTrue(delivery instanceof DeliveryCompat); + DeliveryCompat compat = (DeliveryCompat) delivery; + + assertNull(compat.errorReportApiClient); + assertNull(compat.sessionTrackingApiClient); + + ErrorReportApiClient errorClient = BugsnagTestUtils.generateErrorReportApiClient(); + client.setErrorReportApiClient(errorClient); + + assertEquals(errorClient, compat.errorReportApiClient); + assertNull(compat.sessionTrackingApiClient); + + SessionTrackingApiClient sessionClient = BugsnagTestUtils.generateSessionTrackingApiClient(); + client.setSessionTrackingApiClient(sessionClient); + + assertEquals(errorClient, compat.errorReportApiClient); + assertEquals(sessionClient, compat.sessionTrackingApiClient); + } + + @Test + public void testExceptionConversion() { + BadResponseException requestFailExc = new BadResponseException("test", 400); + DeliveryFailureException responseFail = deliveryCompat.convertException(requestFailExc); + assertEquals(REQUEST_FAILURE ,responseFail.reason); + + NetworkException connectivityExc = new NetworkException("test", new RuntimeException("")); + DeliveryFailureException connectivityFail = deliveryCompat.convertException(connectivityExc); + assertEquals(CONNECTIVITY ,connectivityFail.reason); + + assertNull(deliveryCompat.convertException(new RuntimeException())); + } + +} diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index d461d1a5b0..0c89aa3ed7 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -124,7 +124,7 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat ConnectivityManager cm = (ConnectivityManager) appContext.getSystemService(Context.CONNECTIVITY_SERVICE); - configuration.setDelivery(new DefaultDelivery(cm)); + configuration.setDelivery(new DeliveryCompat(new DefaultDelivery(cm))); sessionTracker = @@ -631,8 +631,10 @@ void setUserName(String name, boolean notify) { void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { if (errorReportApiClient == null) { throw new IllegalArgumentException("ErrorReportApiClient cannot be null."); - }// FIXME -// this.errorReportApiClient = errorReportApiClient; + } + + DeliveryCompat compat = (DeliveryCompat) config.getDelivery(); + compat.errorReportApiClient = errorReportApiClient; } @SuppressWarnings("ConstantConditions") @@ -640,8 +642,9 @@ void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { if (apiClient == null) { throw new IllegalArgumentException("SessionTrackingApiClient cannot be null."); - }// FIXME -// this.sessionTrackingApiClient = apiClient; + } + DeliveryCompat compat = (DeliveryCompat) config.getDelivery(); + compat.sessionTrackingApiClient = apiClient; } /** diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 76be364d68..f8a4332106 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -500,7 +500,15 @@ public Delivery getDelivery() { } public void setDelivery(Delivery delivery) { - this.delivery = delivery; + if (delivery == null) { + throw new IllegalArgumentException("Delivery cannot be null"); + } + + if (delivery instanceof DeliveryCompat) { + this.delivery = delivery; + } else { + this.delivery = new DeliveryCompat(delivery); + } } public Map getErrorApiHeaders() { 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..c3e3ea3458 --- /dev/null +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -0,0 +1,55 @@ +package com.bugsnag.android; + +import static com.bugsnag.android.DeliveryFailureException.Reason.*; + +class DeliveryCompat implements Delivery { + + final Delivery delivery; + volatile ErrorReportApiClient errorReportApiClient; + volatile SessionTrackingApiClient sessionTrackingApiClient; + + DeliveryCompat(Delivery delivery) { + this.delivery = delivery; + } + + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { + if (sessionTrackingApiClient != null) { + + try { + sessionTrackingApiClient.postSessionTrackingPayload(config.getSessionEndpoint(), + payload, config.getSessionApiHeaders()); + } catch (NetworkException | BadResponseException exception) { + throw convertException(exception); + } + } else { + delivery.deliver(payload, config); + } + } + + @Override + public void deliver(Report report, Configuration config) throws DeliveryFailureException { + if (errorReportApiClient != null) { + try { + errorReportApiClient.postReport(config.getEndpoint(), + report, config.getErrorApiHeaders()); + } catch (NetworkException | BadResponseException exception) { + throw convertException(exception); + } + } else { + delivery.deliver(report, config); + } + } + + DeliveryFailureException convertException(Exception exception) { + if (exception instanceof NetworkException) { + return new DeliveryFailureException(CONNECTIVITY, exception.getMessage(), exception); + } else if (exception instanceof BadResponseException) { + return new DeliveryFailureException(REQUEST_FAILURE, exception.getMessage(), exception); + } else { + return null; + } + } + +} From 86402dcf1630cc7397795ebeb18979899585ae0f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 10:01:33 +0100 Subject: [PATCH 14/43] docs: first pass of delivery docs --- .../com/bugsnag/android/BugsnagTestUtils.java | 6 +- .../bugsnag/android/DeliveryCompatTest.java | 22 ++++--- .../bugsnag/android/SessionTrackerTest.java | 3 +- .../java/com/bugsnag/android/Bugsnag.java | 19 +----- .../com/bugsnag/android/Configuration.java | 30 ++++++++++ .../com/bugsnag/android/DefaultDelivery.java | 6 +- .../java/com/bugsnag/android/Delivery.java | 59 +++++++++++++++++++ .../com/bugsnag/android/DeliveryCompat.java | 3 +- .../bugsnag/android/ErrorReportApiClient.java | 16 +---- .../android/SessionTrackingApiClient.java | 13 +--- 10 files changed, 123 insertions(+), 54 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index 3ff319b856..7e68a40756 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -94,12 +94,14 @@ public void postReport(String urlString, public static Delivery generateDelivery() { return new Delivery() { @Override - public void deliver(SessionTrackingPayload payload, Configuration config) throws DeliveryFailureException { + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { } @Override - public void deliver(Report report, Configuration config) throws DeliveryFailureException { + public void deliver(Report report, + Configuration config) throws DeliveryFailureException { } }; diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java index 6dbdee304e..798f3fb38c 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -1,5 +1,10 @@ package com.bugsnag.android; +import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; +import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Test; @@ -7,9 +12,6 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; -import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; -import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; -import static junit.framework.Assert.*; public class DeliveryCompatTest { @@ -19,6 +21,11 @@ public class DeliveryCompatTest { private AtomicInteger defaultCount; 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 { defaultCount = new AtomicInteger(); @@ -98,7 +105,8 @@ public void testClientCompat() { assertEquals(errorClient, compat.errorReportApiClient); assertNull(compat.sessionTrackingApiClient); - SessionTrackingApiClient sessionClient = BugsnagTestUtils.generateSessionTrackingApiClient(); + SessionTrackingApiClient sessionClient + = BugsnagTestUtils.generateSessionTrackingApiClient(); client.setSessionTrackingApiClient(sessionClient); assertEquals(errorClient, compat.errorReportApiClient); @@ -109,11 +117,11 @@ public void testClientCompat() { public void testExceptionConversion() { BadResponseException requestFailExc = new BadResponseException("test", 400); DeliveryFailureException responseFail = deliveryCompat.convertException(requestFailExc); - assertEquals(REQUEST_FAILURE ,responseFail.reason); + assertEquals(REQUEST_FAILURE, responseFail.reason); NetworkException connectivityExc = new NetworkException("test", new RuntimeException("")); - DeliveryFailureException connectivityFail = deliveryCompat.convertException(connectivityExc); - assertEquals(CONNECTIVITY ,connectivityFail.reason); + DeliveryFailureException exc = deliveryCompat.convertException(connectivityExc); + assertEquals(CONNECTIVITY, exc.reason); assertNull(deliveryCompat.convertException(new RuntimeException())); } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java index 90d794fc11..4e6c7bba42 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java @@ -32,7 +32,8 @@ public class SessionTrackerTest { public void setUp() throws Exception { configuration = new Configuration("test"); configuration.setDelivery(BugsnagTestUtils.generateDelivery()); - sessionTracker = new SessionTracker(configuration, generateClient(), generateSessionStore()); + sessionTracker + = new SessionTracker(configuration, generateClient(), generateSessionStore()); configuration.setAutoCaptureSessions(true); user = new User(); } diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index 07656efa45..ca3fa4e848 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -289,15 +289,7 @@ public static void setUserName(final String name) { } /** - * Replaces the Default HTTP Client with a custom implementation. This allows for custom - * requirements such as certificate pinning to be achieved. - *

- *

- * The client implementation, and must be capable of sending Error Reports to the Bugsnag API, - * as documented here: - * https://docs.bugsnag.com/api/error-reporting/ - * - * @param errorReportApiClient the custom HTTP client implementation + * @see Configuration#setDelivery(Delivery) */ @Deprecated public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) { @@ -305,14 +297,7 @@ public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorRe } /** - * Replaces the Default HTTP Client with a custom implementation. This allows for custom - * requirements such as certificate pinning to be achieved. - *

- *

- * The client implementation, and must be capable of sending Session Tracking Payloads to - * the Bugsnag API. - * - * @param apiClient the custom HTTP client implementation + * @see Configuration#setDelivery(Delivery) */ @Deprecated public static void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index f8a4332106..4d72e5cb3f 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -453,6 +453,7 @@ public void setLaunchCrashThresholdMs(long launchCrashThresholdMs) { /** * Returns whether automatic breadcrumb capture or common application events is enabled. + * * @return true if automatic capture is enabled, otherwise false. */ public boolean isAutomaticallyCollectingBreadcrumbs() { @@ -473,6 +474,7 @@ public void setAutomaticallyCollectBreadcrumbs(boolean automaticallyCollectBread /** * Intended for internal use only - sets the type of the notifier (e.g. Android, React Native) + * * @param notifierType the notifier type */ public void setNotifierType(String notifierType) { @@ -481,6 +483,7 @@ public void setNotifierType(String notifierType) { /** * Intended for internal use only - sets the code bundle id for React Native + * * @param codeBundleId the code bundle id */ public void setCodeBundleId(String codeBundleId) { @@ -495,10 +498,27 @@ String getNotifierType() { return notifierType; } + /** + * Retrieves the delivery used to make HTTP requests to Bugsnag. + * + * @return the current delivery + */ 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(Delivery delivery) { if (delivery == null) { throw new IllegalArgumentException("Delivery cannot be null"); @@ -511,6 +531,11 @@ public void setDelivery(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"); @@ -519,6 +544,11 @@ public Map getErrorApiHeaders() { return map; } + /** + * 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"); diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index 430c5a9bdf..d4363408ae 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -1,5 +1,8 @@ package com.bugsnag.android; +import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; +import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; + import android.net.ConnectivityManager; import android.net.NetworkInfo; @@ -10,9 +13,6 @@ import java.net.URL; import java.util.Map; -import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; -import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; - class DefaultDelivery implements Delivery { private final ConnectivityManager connectivityManager; diff --git a/sdk/src/main/java/com/bugsnag/android/Delivery.java b/sdk/src/main/java/com/bugsnag/android/Delivery.java index fd1bccb51f..e9c7e4db08 100644 --- a/sdk/src/main/java/com/bugsnag/android/Delivery.java +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -1,10 +1,69 @@ 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 response status code is not 202, then the implementation must throw + * {@link DeliveryFailureException} with a reason of + * {@link DeliveryFailureException.Reason#REQUEST_FAILURE}. + *

+ * If the request could not be delivered due to connectivity issues, then the implementation + * must throw {@link DeliveryFailureException} with a reason of + * {@link DeliveryFailureException.Reason#CONNECTIVITY}, as this will cache the request for + * delivery at a future time. + *

+ * 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 does not receive a 202 status code. + */ 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 response status code is not 2xx, then the implementation must throw + * {@link DeliveryFailureException} with a reason of + * {@link DeliveryFailureException.Reason#REQUEST_FAILURE}. + *

+ * If the request could not be delivered due to connectivity issues, then the implementation + * must throw {@link DeliveryFailureException} with a reason of + * {@link DeliveryFailureException.Reason#CONNECTIVITY}, as this will cache the request for + * delivery at a future time. + *

+ * 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 does not receive a 2xx status code. + */ 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 index c3e3ea3458..b5d8b46254 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -1,6 +1,7 @@ package com.bugsnag.android; -import static com.bugsnag.android.DeliveryFailureException.Reason.*; +import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; +import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; class DeliveryCompat implements Delivery { diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java index 50ed5aa3a2..e531c86466 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java @@ -3,26 +3,16 @@ import java.util.Map; /** - * Posts an error report to the Bugsnag API. Custom implementations of this client can be used in - * place of the default implementation, by calling - * {@link Bugsnag#setErrorReportApiClient(ErrorReportApiClient)} + * @see Delivery */ @SuppressWarnings("WeakerAccess") @Deprecated public interface ErrorReportApiClient { /** - * Posts an Error Report to the Bugsnag API. - *

- * See - * https://docs.bugsnag.com/api/error-reporting/ - * - * @param urlString the Bugsnag endpoint - * @param report The error report - * @param headers the HTTP headers - * @throws NetworkException if the client was unable to complete the request - * @throws BadResponseException when a non-200 response code is received from the server + * @see Delivery#deliver(Report, Configuration) */ + @Deprecated void postReport(String urlString, Report report, Map headers) throws NetworkException, BadResponseException; diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java index ad83761a20..32e502a792 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java @@ -3,22 +3,15 @@ import java.util.Map; /** - * Posts an array of sessions to the Bugsnag Session Tracking API. Custom implementations - * of this client can be used in place of the default implementation, by calling - * {@link Bugsnag#setSessionTrackingApiClient(SessionTrackingApiClient)} + * @see Delivery */ @Deprecated public interface SessionTrackingApiClient { /** - * Posts an array of sessions to the Bugsnag API. - * - * @param urlString the Bugsnag endpoint - * @param payload The session tracking - * @param headers the HTTP headers - * @throws NetworkException if the client was unable to complete the request - * @throws BadResponseException when a non-202 response code is received from the server + * @see Delivery#deliver(SessionTrackingPayload, Configuration) */ + @Deprecated void postSessionTrackingPayload(String urlString, SessionTrackingPayload payload, Map headers) From b15a4f4692c05af50df841313ebbeec3b75ce299 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 10:05:38 +0100 Subject: [PATCH 15/43] fix html tag --- sdk/src/main/java/com/bugsnag/android/Delivery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Delivery.java b/sdk/src/main/java/com/bugsnag/android/Delivery.java index e9c7e4db08..de15adf3cb 100644 --- a/sdk/src/main/java/com/bugsnag/android/Delivery.java +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -10,7 +10,7 @@ *

* Any custom implementation must be capable of sending * - * Error Reports and Sessions as + * Error Reports and Sessions as * documented at https://docs.bugsnag.com/api/ * * @see DefaultDelivery From 72b744cccfc0c7a2a11ee86effcf7f0a561b7e7c Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 10:57:39 +0100 Subject: [PATCH 16/43] style: improve code style for delivery api changes --- .../java/com/bugsnag/android/BugsnagTestUtils.java | 8 ++------ .../java/com/bugsnag/android/ClientConfigTest.java | 3 --- .../java/com/bugsnag/android/DeliveryCompatTest.java | 3 --- sdk/src/main/java/com/bugsnag/android/Client.java | 3 --- sdk/src/main/java/com/bugsnag/android/Configuration.java | 4 ---- .../main/java/com/bugsnag/android/DefaultDelivery.java | 2 -- sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java | 1 - 7 files changed, 2 insertions(+), 22 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index 7e68a40756..66b3c38c44 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -95,15 +95,11 @@ public static Delivery generateDelivery() { return new Delivery() { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException { - - } + Configuration config) throws DeliveryFailureException {} @Override public void deliver(Report report, - Configuration config) throws DeliveryFailureException { - - } + 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 70c7c6f002..875a1d571c 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -3,9 +3,6 @@ import static org.junit.Assert.assertArrayEquals; 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.content.Context; import android.support.test.InstrumentationRegistry; diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java index 798f3fb38c..9a04f493b5 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -12,7 +12,6 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; - public class DeliveryCompatTest { private final Configuration config = BugsnagTestUtils.generateConfiguration(); @@ -45,7 +44,6 @@ public void deliver(Report report, } }; deliveryCompat = new DeliveryCompat(baseDelivery); - } @Test @@ -125,5 +123,4 @@ public void testExceptionConversion() { assertNull(deliveryCompat.convertException(new RuntimeException())); } - } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 0c89aa3ed7..851dae5d72 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -632,7 +632,6 @@ void setErrorReportApiClient(@NonNull ErrorReportApiClient errorReportApiClient) if (errorReportApiClient == null) { throw new IllegalArgumentException("ErrorReportApiClient cannot be null."); } - DeliveryCompat compat = (DeliveryCompat) config.getDelivery(); compat.errorReportApiClient = errorReportApiClient; } @@ -1239,9 +1238,7 @@ void deliver(@NonNull Report report, @NonNull Error error) { config.getDelivery().deliver(report, config); Logger.info("Sent 1 new error to Bugsnag"); } catch (DeliveryFailureException exception) { - switch (exception.reason) { - case CONNECTIVITY: Logger.info("Could not send error(s) to Bugsnag, saving to disk to send later"); // Save error to disk for later sending diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 4d72e5cb3f..5aee543464 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -453,7 +453,6 @@ public void setLaunchCrashThresholdMs(long launchCrashThresholdMs) { /** * Returns whether automatic breadcrumb capture or common application events is enabled. - * * @return true if automatic capture is enabled, otherwise false. */ public boolean isAutomaticallyCollectingBreadcrumbs() { @@ -474,7 +473,6 @@ public void setAutomaticallyCollectBreadcrumbs(boolean automaticallyCollectBread /** * Intended for internal use only - sets the type of the notifier (e.g. Android, React Native) - * * @param notifierType the notifier type */ public void setNotifierType(String notifierType) { @@ -483,7 +481,6 @@ public void setNotifierType(String notifierType) { /** * Intended for internal use only - sets the code bundle id for React Native - * * @param codeBundleId the code bundle id */ public void setCodeBundleId(String codeBundleId) { @@ -523,7 +520,6 @@ public void setDelivery(Delivery delivery) { if (delivery == null) { throw new IllegalArgumentException("Delivery cannot be null"); } - if (delivery instanceof DeliveryCompat) { this.delivery = delivery; } else { diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index d4363408ae..3696da3898 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -38,7 +38,6 @@ public void deliver(SessionTrackingPayload payload, @Override public void deliver(Report report, Configuration config) throws DeliveryFailureException { - String endpoint = config.getEndpoint(); int status = deliver(endpoint, report, config.getErrorApiHeaders()); @@ -96,5 +95,4 @@ private void checkHasNetworkConnection() throws DeliveryFailureException { throw new DeliveryFailureException(CONNECTIVITY, "No network connection available"); } } - } diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java index b5d8b46254..621be51156 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -52,5 +52,4 @@ DeliveryFailureException convertException(Exception exception) { return null; } } - } From db710af8d5411ae9a40f67a41abbe33bf1dc015b Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 11:09:09 +0100 Subject: [PATCH 17/43] refactor: move added tests into separate PR --- .../bugsnag/android/ConfigurationTest.java | 18 --- .../bugsnag/android/DeliveryCompatTest.java | 126 ------------------ 2 files changed, 144 deletions(-) delete mode 100644 sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java index 0d044fd15c..ddf1c0ad18 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java @@ -4,7 +4,6 @@ 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; @@ -165,21 +164,4 @@ 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); - - assertTrue(configuration.getDelivery() instanceof DeliveryCompat); - DeliveryCompat compat = (DeliveryCompat) configuration.getDelivery(); - assertEquals(compat.delivery, delivery); - } - - @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 deleted file mode 100644 index 9a04f493b5..0000000000 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ /dev/null @@ -1,126 +0,0 @@ -package com.bugsnag.android; - -import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; -import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; -import static org.junit.Assert.assertEquals; -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 defaultCount; - 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 { - defaultCount = new AtomicInteger(); - customCount = new AtomicInteger(); - - Delivery baseDelivery = new Delivery() { - @Override - public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException { - defaultCount.incrementAndGet(); - } - - @Override - public void deliver(Report report, - Configuration config) throws DeliveryFailureException { - defaultCount.incrementAndGet(); - } - }; - deliveryCompat = new DeliveryCompat(baseDelivery); - } - - @Test - public void deliverReport() throws Exception { - Report report = null; - deliveryCompat.deliver(report, config); - assertEquals(1, defaultCount.get()); - - 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, defaultCount.get()); - 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, defaultCount.get()); - assertEquals(1, customCount.get()); - } - - @Test - public void testClientCompat() { - Client client = BugsnagTestUtils.generateClient(); - Delivery delivery = client.config.getDelivery(); - assertTrue(delivery instanceof DeliveryCompat); - DeliveryCompat compat = (DeliveryCompat) delivery; - - assertNull(compat.errorReportApiClient); - assertNull(compat.sessionTrackingApiClient); - - ErrorReportApiClient errorClient = BugsnagTestUtils.generateErrorReportApiClient(); - client.setErrorReportApiClient(errorClient); - - assertEquals(errorClient, compat.errorReportApiClient); - assertNull(compat.sessionTrackingApiClient); - - SessionTrackingApiClient sessionClient - = BugsnagTestUtils.generateSessionTrackingApiClient(); - client.setSessionTrackingApiClient(sessionClient); - - assertEquals(errorClient, compat.errorReportApiClient); - assertEquals(sessionClient, compat.sessionTrackingApiClient); - } - - @Test - public void testExceptionConversion() { - BadResponseException requestFailExc = new BadResponseException("test", 400); - DeliveryFailureException responseFail = deliveryCompat.convertException(requestFailExc); - assertEquals(REQUEST_FAILURE, responseFail.reason); - - NetworkException connectivityExc = new NetworkException("test", new RuntimeException("")); - DeliveryFailureException exc = deliveryCompat.convertException(connectivityExc); - assertEquals(CONNECTIVITY, exc.reason); - - assertNull(deliveryCompat.convertException(new RuntimeException())); - } -} From d5c04985992f756c87ba2f7c16b5b1adfb37d19e Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 11:10:43 +0100 Subject: [PATCH 18/43] refactor: add tests for delivery api change --- .../bugsnag/android/ConfigurationTest.java | 18 +++ .../bugsnag/android/DeliveryCompatTest.java | 126 ++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java index ddf1c0ad18..0d044fd15c 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,21 @@ 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); + + assertTrue(configuration.getDelivery() instanceof DeliveryCompat); + DeliveryCompat compat = (DeliveryCompat) configuration.getDelivery(); + assertEquals(compat.delivery, delivery); + } + + @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..9a04f493b5 --- /dev/null +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -0,0 +1,126 @@ +package com.bugsnag.android; + +import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; +import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; +import static org.junit.Assert.assertEquals; +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 defaultCount; + 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 { + defaultCount = new AtomicInteger(); + customCount = new AtomicInteger(); + + Delivery baseDelivery = new Delivery() { + @Override + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException { + defaultCount.incrementAndGet(); + } + + @Override + public void deliver(Report report, + Configuration config) throws DeliveryFailureException { + defaultCount.incrementAndGet(); + } + }; + deliveryCompat = new DeliveryCompat(baseDelivery); + } + + @Test + public void deliverReport() throws Exception { + Report report = null; + deliveryCompat.deliver(report, config); + assertEquals(1, defaultCount.get()); + + 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, defaultCount.get()); + 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, defaultCount.get()); + assertEquals(1, customCount.get()); + } + + @Test + public void testClientCompat() { + Client client = BugsnagTestUtils.generateClient(); + Delivery delivery = client.config.getDelivery(); + assertTrue(delivery instanceof DeliveryCompat); + DeliveryCompat compat = (DeliveryCompat) delivery; + + assertNull(compat.errorReportApiClient); + assertNull(compat.sessionTrackingApiClient); + + ErrorReportApiClient errorClient = BugsnagTestUtils.generateErrorReportApiClient(); + client.setErrorReportApiClient(errorClient); + + assertEquals(errorClient, compat.errorReportApiClient); + assertNull(compat.sessionTrackingApiClient); + + SessionTrackingApiClient sessionClient + = BugsnagTestUtils.generateSessionTrackingApiClient(); + client.setSessionTrackingApiClient(sessionClient); + + assertEquals(errorClient, compat.errorReportApiClient); + assertEquals(sessionClient, compat.sessionTrackingApiClient); + } + + @Test + public void testExceptionConversion() { + BadResponseException requestFailExc = new BadResponseException("test", 400); + DeliveryFailureException responseFail = deliveryCompat.convertException(requestFailExc); + assertEquals(REQUEST_FAILURE, responseFail.reason); + + NetworkException connectivityExc = new NetworkException("test", new RuntimeException("")); + DeliveryFailureException exc = deliveryCompat.convertException(connectivityExc); + assertEquals(CONNECTIVITY, exc.reason); + + assertNull(deliveryCompat.convertException(new RuntimeException())); + } +} From 62826fd711374708d12bc26f6828377c3ba55cdb Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 11:21:22 +0100 Subject: [PATCH 19/43] test: ensure that null session api clients throw exception --- sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java index 382caae06d..42a747a886 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java @@ -235,4 +235,9 @@ public void testClientClearTab() { assertTrue(client.getMetaData().getTab("drink").isEmpty()); } + @Test(expected = IllegalArgumentException.class) + public void testApiClientNullValidation() { + generateClient().setSessionTrackingApiClient(null); + } + } From 6efe024534cc341c4fa509f70025fbc8b198022e Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 12:16:17 +0100 Subject: [PATCH 20/43] refactor: delete defaulthttpclient and update mazerunner tests deletes the defaulthttpclient previously used by mazerunner scenarios, and uses delivery instead --- .../com/bugsnag/android/TestHarnessHooks.kt | 58 ++++++---- .../AsyncErrorConnectivityScenario.kt | 8 +- .../AsyncErrorDoubleFlushScenario.kt | 7 +- .../scenarios/AsyncErrorLaunchScenario.kt | 7 +- .../CustomClientErrorFlushScenario.kt | 12 +-- .../scenarios/CustomClientErrorScenario.kt | 8 +- .../CustomClientSessionFlushScenario.kt | 15 ++- .../scenarios/CustomClientSessionScenario.kt | 11 +- .../com/bugsnag/android/DefaultDelivery.java | 6 +- .../bugsnag/android/DefaultHttpClient.java | 100 ------------------ 10 files changed, 68 insertions(+), 164 deletions(-) delete mode 100644 sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java diff --git a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index 3b19ba621b..6cba832f00 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -10,41 +10,61 @@ internal fun flushAllSessions() { Bugsnag.getClient().sessionTracker.flushStoredSessions() } -internal fun flushErrorStoreAsync(client: Client, apiClient: ErrorReportApiClient) { - client.setErrorReportApiClient(apiClient) +internal fun flushErrorStoreAsync(client: Client) { client.errorStore.flushAsync() } -internal fun flushErrorStoreOnLaunch(client: Client, apiClient: ErrorReportApiClient) { - client.setErrorReportApiClient(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 createDefaultErrorClient(context: Context): ErrorReportApiClient { +internal fun createDefaultDelivery(context: Context): DefaultDelivery { val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? - return DefaultHttpClient(cm) + return DefaultDelivery(cm) } -internal fun createDefaultSessionClient(context: Context): SessionTrackingApiClient { - val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? - return DefaultHttpClient(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 index f3f718a403..f58d75c0b1 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorFlushScenario.kt @@ -3,7 +3,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.createDefaultErrorClient +import com.bugsnag.android.createCustomHeaderDelivery /** * Sends an unhandled exception which is cached on disk to Bugsnag, then sent on a separate launch, @@ -13,14 +13,12 @@ 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) { - Bugsnag.setErrorReportApiClient { urlString, report, headers -> - headers["Custom-Client"] = "Hello World" - createDefaultErrorClient(context).postReport(urlString, report, headers) - } - } else { + 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 index 1817553824..639319a340 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientErrorScenario.kt @@ -3,7 +3,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.createDefaultErrorClient +import com.bugsnag.android.createCustomHeaderDelivery /** * Sends a handled exception to Bugsnag using a custom API client which modifies the request. @@ -12,12 +12,8 @@ internal class CustomClientErrorScenario(config: Configuration, context: Context) : Scenario(config, context) { override fun run() { + config.delivery = createCustomHeaderDelivery(context) super.run() - - Bugsnag.setErrorReportApiClient { urlString, report, headers -> - headers["Custom-Client"] = "Hello World" - createDefaultErrorClient(context).postReport(urlString, report, headers) - } 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 index bd936019b6..66770ae2e9 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt @@ -3,7 +3,8 @@ package com.bugsnag.android.mazerunner.scenarios import android.content.Context import com.bugsnag.android.Bugsnag import com.bugsnag.android.Configuration -import com.bugsnag.android.createDefaultSessionClient +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, @@ -13,18 +14,14 @@ 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) + } + super.run() - Bugsnag.setSessionTrackingApiClient { urlString, report, headers -> - headers["Custom-Client"] = "Hello World" - val sessionClient = createDefaultSessionClient(context) - sessionClient.postSessionTrackingPayload(urlString, report, headers) - } - } else { + if ("DeliverSessions" != eventMetaData) { 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 index 24593e358c..954e3143be 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionScenario.kt @@ -3,7 +3,8 @@ package com.bugsnag.android.mazerunner.scenarios import android.content.Context import com.bugsnag.android.Bugsnag import com.bugsnag.android.Configuration -import com.bugsnag.android.createDefaultSessionClient +import com.bugsnag.android.createCustomHeaderDelivery +import com.bugsnag.android.createDefaultDelivery /** * Sends a session using a custom API client which modifies the request. @@ -12,14 +13,8 @@ internal class CustomClientSessionScenario(config: Configuration, context: Context) : Scenario(config, context) { override fun run() { + config.delivery = createCustomHeaderDelivery(context) super.run() - - Bugsnag.setSessionTrackingApiClient { urlString, report, headers -> - headers["Custom-Client"] = "Hello World" - val sessionClient = createDefaultSessionClient(context) - sessionClient.postSessionTrackingPayload(urlString, report, headers) - } - Bugsnag.startSession() } diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index 3696da3898..5e5b2e2f19 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -49,9 +49,9 @@ public void deliver(Report report, } } - private int deliver(String urlString, - JsonStream.Streamable streamable, - Map headers) throws DeliveryFailureException { + int deliver(String urlString, + JsonStream.Streamable streamable, + Map headers) throws DeliveryFailureException { checkHasNetworkConnection(); HttpURLConnection conn = null; diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java deleted file mode 100644 index 7575950851..0000000000 --- a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java +++ /dev/null @@ -1,100 +0,0 @@ -package com.bugsnag.android; - -import android.net.ConnectivityManager; -import android.net.NetworkInfo; - -import java.io.IOException; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.net.HttpURLConnection; -import java.net.URL; -import java.util.Map; - -@Deprecated -class DefaultHttpClient implements ErrorReportApiClient, SessionTrackingApiClient { - - private final ConnectivityManager connectivityManager; - - DefaultHttpClient(ConnectivityManager connectivityManager) { - this.connectivityManager = connectivityManager; - } - - @Override - public void postReport(String urlString, - Report report, - Map headers) - throws NetworkException, BadResponseException { - - int status = makeRequest(urlString, report, headers); - - if (status / 100 != 2) { - throw new BadResponseException(urlString, status); - } else { - Logger.info("Completed error API request"); - } - } - - @Override - public void postSessionTrackingPayload(String urlString, - SessionTrackingPayload payload, - Map headers) - throws NetworkException, BadResponseException { - - int status = makeRequest(urlString, payload, headers); - - if (status != 202) { - throw new BadResponseException(urlString, status); - } else { - Logger.info("Completed session tracking request"); - } - } - - private int makeRequest(String urlString, - JsonStream.Streamable streamable, - Map headers) throws NetworkException { - checkHasNetworkConnection(urlString); - HttpURLConnection conn = null; - - try { - URL url = new URL(urlString); - conn = (HttpURLConnection) url.openConnection(); - conn.setDoOutput(true); - conn.setChunkedStreamingMode(0); - conn.addRequestProperty("Content-Type", "application/json"); - - for (Map.Entry entry : headers.entrySet()) { - conn.addRequestProperty(entry.getKey(), entry.getValue()); - } - - OutputStream out = null; - - try { - out = conn.getOutputStream(); - JsonStream stream = new JsonStream(new OutputStreamWriter(out)); - streamable.toStream(stream); - stream.close(); - } finally { - IOUtils.closeQuietly(out); - } - - - // End the request, get the response code - return conn.getResponseCode(); - } catch (IOException exception) { - throw new NetworkException(urlString, exception); - } finally { - IOUtils.close(conn); - } - } - - private void checkHasNetworkConnection(String urlString) throws NetworkException { - 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); - } - } - -} From fbdcbe3ed2b9ad246602a7b02f2782d8085439d5 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 12:58:58 +0100 Subject: [PATCH 21/43] refactor: pass mazerunner custom client scenarios --- .../CustomClientSessionFlushScenario.kt | 7 ++--- .../com/bugsnag/android/ClientConfigTest.java | 30 +++++++++++++++++++ .../main/java/com/bugsnag/android/Client.java | 4 ++- 3 files changed, 36 insertions(+), 5 deletions(-) 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 index 66770ae2e9..f57b48a781 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CustomClientSessionFlushScenario.kt @@ -14,14 +14,13 @@ 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) - } - super.run() - - if ("DeliverSessions" != eventMetaData) { + } else { disableAllDelivery() Bugsnag.startSession() } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java index 875a1d571c..9679f59942 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,33 @@ public void testSetSendThreads() throws Exception { assertFalse(config.getSendThreads()); } + @Test + public void testDefaultClientDelivery() { + assertTrue(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); + + Delivery delivery = client.config.getDelivery(); + assertTrue(client.config.getDelivery() instanceof DeliveryCompat); + + DeliveryCompat compat = (DeliveryCompat) delivery; + assertEquals(customDelivery, compat.delivery); + } } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 851dae5d72..7294a983e5 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -124,8 +124,10 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat ConnectivityManager cm = (ConnectivityManager) appContext.getSystemService(Context.CONNECTIVITY_SERVICE); - configuration.setDelivery(new DeliveryCompat(new DefaultDelivery(cm))); + if (configuration.getDelivery() == null) { + configuration.setDelivery(new DeliveryCompat(new DefaultDelivery(cm))); + } sessionTracker = new SessionTracker(configuration, this, sessionStore); From 57f3e81a8434a82069652403669c57dae74a656a Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 13:03:46 +0100 Subject: [PATCH 22/43] reduce duplication in error request failure logic --- .../main/java/com/bugsnag/android/ErrorStore.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java index 9662e753f9..aea0e50f18 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java @@ -135,18 +135,21 @@ private void flushErrorReport(File errorFile) { + " to Bugsnag, will try again later", exception); break; case REQUEST_FAILURE: - deleteStoredFiles(Collections.singleton(errorFile)); - Logger.warn("Problem sending unsent error from disk", exception); + handleRequestFailure(errorFile, exception); break; default: break; } - } catch (Exception exception) { // FIXME dupe - deleteStoredFiles(Collections.singleton(errorFile)); - Logger.warn("Problem sending unsent error from disk", exception); + } catch (Exception exception) { + handleRequestFailure(errorFile, exception); } } + private void handleRequestFailure(File errorFile, Exception exception) { + deleteStoredFiles(Collections.singleton(errorFile)); + Logger.warn("Problem sending unsent error from disk", exception); + } + boolean isLaunchCrashReport(File file) { return file.getName().endsWith("_startupcrash.json"); } From 28f04f45e475c83b31fbbb220c9d93048d464b1a Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 4 May 2018 13:36:36 +0100 Subject: [PATCH 23/43] fix checkstyle --- .../java/com/bugsnag/android/ClientConfigTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java index 9679f59942..58b93658d6 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -98,14 +98,12 @@ public void testCustomDeliveryOverride() { config = BugsnagTestUtils.generateConfiguration(); Delivery customDelivery = new Delivery() { @Override - public void deliver(SessionTrackingPayload payload, Configuration config) throws DeliveryFailureException { - - } + public void deliver(SessionTrackingPayload payload, + Configuration config) throws DeliveryFailureException {} @Override - public void deliver(Report report, Configuration config) throws DeliveryFailureException { - - } + public void deliver(Report report, + Configuration config) throws DeliveryFailureException {} }; config.setDelivery(customDelivery); client = new Client(context, config); From 0b9ce675508327ad6a8b0f94b7c1846735ff3b4c Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 8 May 2018 15:19:24 +0100 Subject: [PATCH 24/43] docs: keep old apiclient docs and add deprecation notice --- .../java/com/bugsnag/android/Bugsnag.java | 24 +++++++++++++++++-- .../bugsnag/android/ErrorReportApiClient.java | 18 +++++++++++--- .../android/SessionTrackingApiClient.java | 15 +++++++++--- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index ca3fa4e848..17b8d4966c 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -289,7 +289,17 @@ public static void setUserName(final String name) { } /** - * @see Configuration#setDelivery(Delivery) + * Replaces the Default HTTP Client with a custom implementation. This allows for custom + * requirements such as certificate pinning to be achieved. + *

+ *

+ * The client implementation, and must be capable of sending Error Reports to the Bugsnag API, + * as documented here: + * 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) { @@ -297,13 +307,23 @@ public static void setErrorReportApiClient(@NonNull ErrorReportApiClient errorRe } /** - * @see Configuration#setDelivery(Delivery) + * Replaces the Default HTTP Client with a custom implementation. This allows for custom + * requirements such as certificate pinning to be achieved. + *

+ *

+ * The client implementation, and must be capable of sending Session Tracking Payloads to + * 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/ErrorReportApiClient.java b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java index e531c86466..1c21bbb4eb 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorReportApiClient.java @@ -3,16 +3,28 @@ import java.util.Map; /** - * @see Delivery + * Posts an error report to the Bugsnag API. Custom implementations of this client can be used in + * place of the default implementation, by calling + * {@link Bugsnag#setErrorReportApiClient(ErrorReportApiClient)} + * + * @deprecated use {@link Delivery} to send error reports */ @SuppressWarnings("WeakerAccess") @Deprecated public interface ErrorReportApiClient { /** - * @see Delivery#deliver(Report, Configuration) + * Posts an Error Report to the Bugsnag API. + *

+ * See + * https://docs.bugsnag.com/api/error-reporting/ + * + * @param urlString the Bugsnag endpoint + * @param report The error report + * @param headers the HTTP headers + * @throws NetworkException if the client was unable to complete the request + * @throws BadResponseException when a non-200 response code is received from the server */ - @Deprecated void postReport(String urlString, Report report, Map headers) throws NetworkException, BadResponseException; diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java index 32e502a792..cabd2d6b23 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTrackingApiClient.java @@ -3,15 +3,24 @@ import java.util.Map; /** - * @see Delivery + * Posts an array of sessions to the Bugsnag Session Tracking API. Custom implementations + * of this client can be used in place of the default implementation, by calling + * {@link Bugsnag#setSessionTrackingApiClient(SessionTrackingApiClient)} + * + * @deprecated use {@link Delivery} to send sessions */ @Deprecated public interface SessionTrackingApiClient { /** - * @see Delivery#deliver(SessionTrackingPayload, Configuration) + * Posts an array of sessions to the Bugsnag API. + * + * @param urlString the Bugsnag endpoint + * @param payload The session tracking + * @param headers the HTTP headers + * @throws NetworkException if the client was unable to complete the request + * @throws BadResponseException when a non-202 response code is received from the server */ - @Deprecated void postSessionTrackingPayload(String urlString, SessionTrackingPayload payload, Map headers) From e503b7dda1a14a8219e9b4113a46c448602485f2 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 8 May 2018 15:24:38 +0100 Subject: [PATCH 25/43] docs: add nullability annotations to delivery config option --- sdk/src/main/java/com/bugsnag/android/Configuration.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 5aee543464..41c3a71466 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/sdk/src/main/java/com/bugsnag/android/Configuration.java @@ -55,6 +55,7 @@ public class Configuration extends Observable implements Observer { = new ConcurrentLinkedQueue<>(); private String codeBundleId; private String notifierType; + private Delivery delivery; /** @@ -500,6 +501,7 @@ String getNotifierType() { * * @return the current delivery */ + @NonNull public Delivery getDelivery() { return delivery; } @@ -516,7 +518,8 @@ public Delivery getDelivery() { * * @param delivery the custom HTTP client implementation */ - public void setDelivery(Delivery delivery) { + public void setDelivery(@NonNull Delivery delivery) { + //noinspection ConstantConditions if (delivery == null) { throw new IllegalArgumentException("Delivery cannot be null"); } From 1582bd071c3aa360dbe1bbe49a0909922b801adf Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 8 May 2018 15:42:47 +0100 Subject: [PATCH 26/43] docs: document deliverycompat class --- sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java index 621be51156..fb7d64e608 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -3,6 +3,11 @@ import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; +/** + * 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 { final Delivery delivery; From ae74ec1ceb5cf35211213654071776828ead387d Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 8 May 2018 16:59:56 +0100 Subject: [PATCH 27/43] refactor: only use deliverycompat when old API called --- .../com/bugsnag/android/ClientConfigTest.java | 9 ++------- .../main/java/com/bugsnag/android/Client.java | 19 ++++++++++++++++--- .../com/bugsnag/android/Configuration.java | 7 +------ .../com/bugsnag/android/DeliveryCompat.java | 9 --------- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java index 58b93658d6..ef7e19a03e 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -89,7 +89,7 @@ public void testSetSendThreads() throws Exception { @Test public void testDefaultClientDelivery() { - assertTrue(client.config.getDelivery() instanceof DeliveryCompat); + assertFalse(client.config.getDelivery() instanceof DeliveryCompat); } @Test @@ -107,11 +107,6 @@ public void deliver(Report report, }; config.setDelivery(customDelivery); client = new Client(context, config); - - Delivery delivery = client.config.getDelivery(); - assertTrue(client.config.getDelivery() instanceof DeliveryCompat); - - DeliveryCompat compat = (DeliveryCompat) delivery; - assertEquals(customDelivery, compat.delivery); + assertEquals(customDelivery, client.config.getDelivery()); } } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 7294a983e5..f6bc2b5406 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -125,8 +125,9 @@ public Client(@NonNull Context androidContext, @NonNull Configuration configurat ConnectivityManager cm = (ConnectivityManager) appContext.getSystemService(Context.CONNECTIVITY_SERVICE); + //noinspection ConstantConditions if (configuration.getDelivery() == null) { - configuration.setDelivery(new DeliveryCompat(new DefaultDelivery(cm))); + configuration.setDelivery(new DefaultDelivery(cm)); } sessionTracker = @@ -628,13 +629,25 @@ 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."); } - DeliveryCompat compat = (DeliveryCompat) config.getDelivery(); + DeliveryCompat compat = getAndSetDeliveryCompat(); compat.errorReportApiClient = errorReportApiClient; } @@ -644,7 +657,7 @@ void setSessionTrackingApiClient(@NonNull SessionTrackingApiClient apiClient) { if (apiClient == null) { throw new IllegalArgumentException("SessionTrackingApiClient cannot be null."); } - DeliveryCompat compat = (DeliveryCompat) config.getDelivery(); + DeliveryCompat compat = getAndSetDeliveryCompat(); compat.sessionTrackingApiClient = apiClient; } diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 41c3a71466..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; @@ -523,11 +522,7 @@ public void setDelivery(@NonNull Delivery delivery) { if (delivery == null) { throw new IllegalArgumentException("Delivery cannot be null"); } - if (delivery instanceof DeliveryCompat) { - this.delivery = delivery; - } else { - this.delivery = new DeliveryCompat(delivery); - } + this.delivery = delivery; } /** diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java index fb7d64e608..e27bc809d0 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -10,14 +10,9 @@ */ class DeliveryCompat implements Delivery { - final Delivery delivery; volatile ErrorReportApiClient errorReportApiClient; volatile SessionTrackingApiClient sessionTrackingApiClient; - DeliveryCompat(Delivery delivery) { - this.delivery = delivery; - } - @Override public void deliver(SessionTrackingPayload payload, Configuration config) throws DeliveryFailureException { @@ -29,8 +24,6 @@ public void deliver(SessionTrackingPayload payload, } catch (NetworkException | BadResponseException exception) { throw convertException(exception); } - } else { - delivery.deliver(payload, config); } } @@ -43,8 +36,6 @@ public void deliver(Report report, Configuration config) throws DeliveryFailureE } catch (NetworkException | BadResponseException exception) { throw convertException(exception); } - } else { - delivery.deliver(report, config); } } From f110e5fe9f1c12b0cefca6b4e1782bd246836d3c Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 8 May 2018 17:08:57 +0100 Subject: [PATCH 28/43] refactor: update tests to assert that deliverycompat is not used by default --- .../bugsnag/android/ConfigurationTest.java | 5 +-- .../bugsnag/android/DeliveryCompatTest.java | 37 +++++-------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java index 0d044fd15c..2971146858 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ConfigurationTest.java @@ -173,9 +173,8 @@ public void testSetDelivery() { Delivery delivery = BugsnagTestUtils.generateDelivery(); configuration.setDelivery(delivery); - assertTrue(configuration.getDelivery() instanceof DeliveryCompat); - DeliveryCompat compat = (DeliveryCompat) configuration.getDelivery(); - assertEquals(compat.delivery, delivery); + assertFalse(configuration.getDelivery() instanceof DeliveryCompat); + assertEquals(delivery, configuration.getDelivery()); } @Test(expected = IllegalArgumentException.class) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java index 9a04f493b5..07ff939ecc 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -3,6 +3,7 @@ import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -17,7 +18,6 @@ public class DeliveryCompatTest { private final Configuration config = BugsnagTestUtils.generateConfiguration(); private DeliveryCompat deliveryCompat; - private AtomicInteger defaultCount; private AtomicInteger customCount; /** @@ -27,30 +27,14 @@ public class DeliveryCompatTest { */ @Before public void setUp() throws Exception { - defaultCount = new AtomicInteger(); customCount = new AtomicInteger(); - - Delivery baseDelivery = new Delivery() { - @Override - public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException { - defaultCount.incrementAndGet(); - } - - @Override - public void deliver(Report report, - Configuration config) throws DeliveryFailureException { - defaultCount.incrementAndGet(); - } - }; - deliveryCompat = new DeliveryCompat(baseDelivery); + deliveryCompat = new DeliveryCompat(); } @Test public void deliverReport() throws Exception { Report report = null; deliveryCompat.deliver(report, config); - assertEquals(1, defaultCount.get()); deliveryCompat.errorReportApiClient = new ErrorReportApiClient() { @Override @@ -63,7 +47,6 @@ public void postReport(String urlString, }; deliveryCompat.deliver(report, config); - assertEquals(1, defaultCount.get()); assertEquals(1, customCount.get()); } @@ -83,7 +66,6 @@ public void postSessionTrackingPayload(String urlString, }; deliveryCompat.deliver(payload, config); - assertEquals(1, defaultCount.get()); assertEquals(1, customCount.get()); } @@ -91,23 +73,22 @@ public void postSessionTrackingPayload(String urlString, public void testClientCompat() { Client client = BugsnagTestUtils.generateClient(); Delivery delivery = client.config.getDelivery(); - assertTrue(delivery instanceof DeliveryCompat); - DeliveryCompat compat = (DeliveryCompat) delivery; + assertFalse(delivery instanceof DeliveryCompat); - assertNull(compat.errorReportApiClient); - assertNull(compat.sessionTrackingApiClient); + ErrorReportApiClient errorReportApiClient = BugsnagTestUtils.generateErrorReportApiClient(); + client.setErrorReportApiClient(errorReportApiClient); - ErrorReportApiClient errorClient = BugsnagTestUtils.generateErrorReportApiClient(); - client.setErrorReportApiClient(errorClient); + assertTrue(client.config.getDelivery() instanceof DeliveryCompat); - assertEquals(errorClient, compat.errorReportApiClient); + DeliveryCompat compat = (DeliveryCompat) client.config.getDelivery(); + assertEquals(errorReportApiClient, compat.errorReportApiClient); assertNull(compat.sessionTrackingApiClient); SessionTrackingApiClient sessionClient = BugsnagTestUtils.generateSessionTrackingApiClient(); client.setSessionTrackingApiClient(sessionClient); - assertEquals(errorClient, compat.errorReportApiClient); + assertEquals(errorReportApiClient, compat.errorReportApiClient); assertEquals(sessionClient, compat.sessionTrackingApiClient); } From 4969511a2d89d85559248de2d1b3f19f3e40acd3 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Wed, 9 May 2018 10:35:07 +0100 Subject: [PATCH 29/43] refactor: close jsonstream Close JsonStream rather than Writer, as this also closes the writer. --- .../java/com/bugsnag/android/DefaultHttpClient.java | 9 ++++----- sdk/src/main/java/com/bugsnag/android/FileStore.java | 12 +++++------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java index aa77868358..1265f783f5 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultHttpClient.java @@ -67,17 +67,16 @@ private int makeRequest(String urlString, conn.addRequestProperty(entry.getKey(), entry.getValue()); } - OutputStream out = null; + JsonStream stream = null; try { - out = conn.getOutputStream(); + OutputStream out = conn.getOutputStream(); Charset charset = Charset.forName("UTF-8"); BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, charset)); - JsonStream stream = new JsonStream(writer); + stream = new JsonStream(writer); streamable.toStream(stream); - stream.close(); } finally { - IOUtils.closeQuietly(out); + IOUtils.closeQuietly(stream); } diff --git a/sdk/src/main/java/com/bugsnag/android/FileStore.java b/sdk/src/main/java/com/bugsnag/android/FileStore.java index 13f78fd6fe..0593fc8b0e 100644 --- a/sdk/src/main/java/com/bugsnag/android/FileStore.java +++ b/sdk/src/main/java/com/bugsnag/android/FileStore.java @@ -70,22 +70,20 @@ String write(@NonNull T streamable) { String filename = getFilename(streamable); - Writer out = null; + + JsonStream stream = null; try { FileOutputStream fos = new FileOutputStream(filename); - out = new BufferedWriter(new OutputStreamWriter(fos, "UTF-8")); - - JsonStream stream = new JsonStream(out); + 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); } return null; } From 0c110145af7351bd934104f1b5b5113f331d5513 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 10 May 2018 12:03:07 +0100 Subject: [PATCH 30/43] test: add test to ensure that jsonwriter closes underlying stream --- .../com/bugsnag/android/JsonWriterTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java diff --git a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java new file mode 100644 index 0000000000..03d29033c9 --- /dev/null +++ b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java @@ -0,0 +1,27 @@ +package com.bugsnag.android; + +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; + +import static org.junit.Assert.*; + +public class JsonWriterTest { + + @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 + } + +} From 7e57e92853fe44104663dbb3cbd2cc42c65bb4df Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 10 May 2018 13:18:21 +0100 Subject: [PATCH 31/43] style: pass checkstyle import checks --- .../androidTest/java/com/bugsnag/android/JsonWriterTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java index 03d29033c9..ef906211d6 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java @@ -1,13 +1,13 @@ package com.bugsnag.android; +import static org.junit.Assert.assertEquals; + import org.junit.Test; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStreamWriter; -import static org.junit.Assert.*; - public class JsonWriterTest { @Test(expected = IOException.class) From 2111babadd151f5a30ad05ad1f6dcf70baa5a05f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 10 May 2018 14:46:08 +0100 Subject: [PATCH 32/43] refactor: use exception hierarchy for Delivery API Use two separate exception classes to indicate request failure, rather than a single exception class with an enum --- .../com/bugsnag/android/BugsnagTestUtils.java | 7 +++- .../com/bugsnag/android/ClientConfigTest.java | 7 +++- .../bugsnag/android/DeliveryCompatTest.java | 14 ------- .../bugsnag/android/BadResponseException.java | 8 +--- .../main/java/com/bugsnag/android/Client.java | 19 +++------- .../com/bugsnag/android/DefaultDelivery.java | 22 ++++------- .../java/com/bugsnag/android/Delivery.java | 22 +++++------ .../com/bugsnag/android/DeliveryCompat.java | 34 ++++------------- .../android/DeliveryFailureException.java | 21 ---------- .../java/com/bugsnag/android/ErrorStore.java | 15 ++------ .../com/bugsnag/android/NetworkException.java | 5 +-- .../com/bugsnag/android/SessionTracker.java | 38 ++++++------------- 12 files changed, 60 insertions(+), 152 deletions(-) delete mode 100644 sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index 66b3c38c44..43a45cc4d2 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -95,11 +95,14 @@ public static Delivery generateDelivery() { return new Delivery() { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException {} + Configuration config) + throws BadResponseException, NetworkException {} @Override public void deliver(Report report, - Configuration config) throws DeliveryFailureException {} + Configuration config) + throws BadResponseException, NetworkException {} + }; } } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java index ef7e19a03e..46f8bca49f 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -99,11 +99,14 @@ public void testCustomDeliveryOverride() { Delivery customDelivery = new Delivery() { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException {} + Configuration config) + throws BadResponseException, NetworkException {} @Override public void deliver(Report report, - Configuration config) throws DeliveryFailureException {} + Configuration config) + throws BadResponseException, NetworkException {} + }; config.setDelivery(customDelivery); client = new Client(context, config); diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java index 07ff939ecc..de0b5f621b 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -1,7 +1,5 @@ package com.bugsnag.android; -import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; -import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -92,16 +90,4 @@ public void testClientCompat() { assertEquals(sessionClient, compat.sessionTrackingApiClient); } - @Test - public void testExceptionConversion() { - BadResponseException requestFailExc = new BadResponseException("test", 400); - DeliveryFailureException responseFail = deliveryCompat.convertException(requestFailExc); - assertEquals(REQUEST_FAILURE, responseFail.reason); - - NetworkException connectivityExc = new NetworkException("test", new RuntimeException("")); - DeliveryFailureException exc = deliveryCompat.convertException(connectivityExc); - assertEquals(CONNECTIVITY, exc.reason); - - assertNull(deliveryCompat.convertException(new RuntimeException())); - } } diff --git a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java index 9b6d1e0f71..f9e175d65f 100644 --- a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java +++ b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java @@ -1,11 +1,7 @@ package com.bugsnag.android; - -import java.util.Locale; - 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, Throwable cause) { + super(msg, cause); } } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index f6bc2b5406..a3b2292b36 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -1252,19 +1252,12 @@ void deliver(@NonNull Report report, @NonNull Error error) { try { config.getDelivery().deliver(report, config); Logger.info("Sent 1 new error to Bugsnag"); - } catch (DeliveryFailureException exception) { - switch (exception.reason) { - case CONNECTIVITY: - Logger.info("Could not send error(s) to Bugsnag, saving to disk to send later"); - // Save error to disk for later sending - errorStore.write(error); - break; - case REQUEST_FAILURE: - Logger.info("Bad response when sending data to Bugsnag"); - break; - default: - break; - } + } 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 + 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/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index 0941be4bbb..235a386865 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -1,8 +1,5 @@ package com.bugsnag.android; -import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; -import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; - import android.net.ConnectivityManager; import android.net.NetworkInfo; @@ -25,13 +22,12 @@ class DefaultDelivery implements Delivery { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException { + Configuration config) throws BadResponseException, NetworkException { String endpoint = config.getSessionEndpoint(); int status = deliver(endpoint, payload, config.getSessionApiHeaders()); if (status != 202) { - throw new DeliveryFailureException(REQUEST_FAILURE, - "Request failed with status " + status); + throw new BadResponseException("Request failed with status " + status, null); } else { Logger.info("Completed session tracking request"); } @@ -39,13 +35,12 @@ public void deliver(SessionTrackingPayload payload, @Override public void deliver(Report report, - Configuration config) throws DeliveryFailureException { + Configuration config) throws BadResponseException, NetworkException { String endpoint = config.getEndpoint(); int status = deliver(endpoint, report, config.getErrorApiHeaders()); if (status / 100 != 2) { - throw new DeliveryFailureException(REQUEST_FAILURE, - "Request failed with status " + status); + throw new BadResponseException("Request failed with status " + status, null); } else { Logger.info("Completed error API request"); } @@ -53,7 +48,7 @@ public void deliver(Report report, int deliver(String urlString, JsonStream.Streamable streamable, - Map headers) throws DeliveryFailureException { + Map headers) throws NetworkException { checkHasNetworkConnection(); HttpURLConnection conn = null; @@ -83,19 +78,18 @@ int deliver(String urlString, // End the request, get the response code return conn.getResponseCode(); } catch (IOException exception) { - throw new DeliveryFailureException(CONNECTIVITY, - "IOException encountered in request", exception); + throw new NetworkException("IOException encountered in request", exception); } finally { IOUtils.close(conn); } } - private void checkHasNetworkConnection() throws DeliveryFailureException { + private void checkHasNetworkConnection() throws NetworkException { NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo(); // conserve device battery by avoiding radio use if (!(activeNetworkInfo != null && activeNetworkInfo.isConnectedOrConnecting())) { - throw new DeliveryFailureException(CONNECTIVITY, "No network connection available"); + throw new NetworkException("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 index de15adf3cb..f6f11e82bd 100644 --- a/sdk/src/main/java/com/bugsnag/android/Delivery.java +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -24,12 +24,10 @@ public interface Delivery { * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. *

* If the response status code is not 202, then the implementation must throw - * {@link DeliveryFailureException} with a reason of - * {@link DeliveryFailureException.Reason#REQUEST_FAILURE}. + * {@link BadResponseException}. *

* If the request could not be delivered due to connectivity issues, then the implementation - * must throw {@link DeliveryFailureException} with a reason of - * {@link DeliveryFailureException.Reason#CONNECTIVITY}, as this will cache the request for + * must throw {@link NetworkException}, as this will cache the request for * delivery at a future time. *

* See @@ -37,10 +35,11 @@ public interface Delivery { * * @param payload The session tracking payload * @param config The configuration by which this request will be sent - * @throws DeliveryFailureException when delivery does not receive a 202 status code. + * @throws BadResponseException when delivery does not receive a 202 status code. + * @throws NetworkException when delivery is not successful due to connectivity issues. */ void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException; + Configuration config) throws BadResponseException, NetworkException; /** * Posts an Error Report to the Bugsnag Error Reporting API. @@ -49,12 +48,10 @@ void deliver(SessionTrackingPayload payload, * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. *

* If the response status code is not 2xx, then the implementation must throw - * {@link DeliveryFailureException} with a reason of - * {@link DeliveryFailureException.Reason#REQUEST_FAILURE}. + * {@link BadResponseException}. *

* If the request could not be delivered due to connectivity issues, then the implementation - * must throw {@link DeliveryFailureException} with a reason of - * {@link DeliveryFailureException.Reason#CONNECTIVITY}, as this will cache the request for + * must throw {@link NetworkException}, as this will cache the request for * delivery at a future time. *

* See @@ -62,8 +59,9 @@ void deliver(SessionTrackingPayload payload, * * @param report The error report * @param config The configuration by which this request will be sent - * @throws DeliveryFailureException when delivery does not receive a 2xx status code. + * @throws BadResponseException when delivery does not receive a 2xx status code. + * @throws NetworkException when delivery is not successful due to connectivity issues. */ void deliver(Report report, - Configuration config) throws DeliveryFailureException; + Configuration config) throws BadResponseException, NetworkException; } diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java index e27bc809d0..99bdccc128 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -1,8 +1,5 @@ package com.bugsnag.android; -import static com.bugsnag.android.DeliveryFailureException.Reason.CONNECTIVITY; -import static com.bugsnag.android.DeliveryFailureException.Reason.REQUEST_FAILURE; - /** * A compatibility implementation of {@link Delivery} which wraps {@link ErrorReportApiClient} and * {@link SessionTrackingApiClient}. This class allows for backwards compatibility for users still @@ -15,37 +12,20 @@ class DeliveryCompat implements Delivery { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws DeliveryFailureException { + Configuration config) throws BadResponseException, NetworkException { if (sessionTrackingApiClient != null) { - - try { - sessionTrackingApiClient.postSessionTrackingPayload(config.getSessionEndpoint(), - payload, config.getSessionApiHeaders()); - } catch (NetworkException | BadResponseException exception) { - throw convertException(exception); - } + sessionTrackingApiClient.postSessionTrackingPayload(config.getSessionEndpoint(), + payload, config.getSessionApiHeaders()); } } @Override - public void deliver(Report report, Configuration config) throws DeliveryFailureException { + public void deliver(Report report, Configuration config) + throws BadResponseException, NetworkException { if (errorReportApiClient != null) { - try { - errorReportApiClient.postReport(config.getEndpoint(), - report, config.getErrorApiHeaders()); - } catch (NetworkException | BadResponseException exception) { - throw convertException(exception); - } + errorReportApiClient.postReport(config.getEndpoint(), + report, config.getErrorApiHeaders()); } } - DeliveryFailureException convertException(Exception exception) { - if (exception instanceof NetworkException) { - return new DeliveryFailureException(CONNECTIVITY, exception.getMessage(), exception); - } else if (exception instanceof BadResponseException) { - return new DeliveryFailureException(REQUEST_FAILURE, exception.getMessage(), exception); - } else { - return null; - } - } } diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java b/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java deleted file mode 100644 index d0a3321395..0000000000 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java +++ /dev/null @@ -1,21 +0,0 @@ -package com.bugsnag.android; - -public class DeliveryFailureException extends Exception { - - enum Reason { - CONNECTIVITY, - REQUEST_FAILURE - } - - final Reason reason; - - public DeliveryFailureException(Reason reason, String msg) { - super(msg); - this.reason = reason; - } - - public DeliveryFailureException(Reason reason, String msg, Throwable throwable) { - super(msg, throwable); - this.reason = reason; - } -} diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java index aea0e50f18..fa9255a159 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java @@ -127,19 +127,10 @@ private void flushErrorReport(File errorFile) { deleteStoredFiles(Collections.singleton(errorFile)); Logger.info("Deleting sent error file " + errorFile.getName()); - } catch (DeliveryFailureException exception) { - switch (exception.reason) { - case CONNECTIVITY: - cancelQueuedFiles(Collections.singleton(errorFile)); - Logger.warn("Could not send previously saved error(s)" + } catch (NetworkException exception) { + cancelQueuedFiles(Collections.singleton(errorFile)); + Logger.warn("Could not send previously saved error(s)" + " to Bugsnag, will try again later", exception); - break; - case REQUEST_FAILURE: - handleRequestFailure(errorFile, exception); - break; - default: - break; - } } catch (Exception exception) { handleRequestFailure(errorFile, exception); } diff --git a/sdk/src/main/java/com/bugsnag/android/NetworkException.java b/sdk/src/main/java/com/bugsnag/android/NetworkException.java index f473b15810..6236b6c125 100644 --- a/sdk/src/main/java/com/bugsnag/android/NetworkException.java +++ b/sdk/src/main/java/com/bugsnag/android/NetworkException.java @@ -3,8 +3,7 @@ import java.io.IOException; 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/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index 35a217f739..7a5e80288a 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -91,18 +91,11 @@ public void run() { try { configuration.getDelivery().deliver(payload, configuration); - } catch (DeliveryFailureException exception) { // store for later sending - switch (exception.reason) { - case CONNECTIVITY: - Logger.info("Failed to post session payload"); - sessionStore.write(session); - break; - case REQUEST_FAILURE: - Logger.warn("Invalid session tracking payload", exception); - break; - default: - break; - } + } catch (NetworkException exception) { // store for later sending + Logger.info("Failed to post session payload"); + sessionStore.write(session); + } catch (BadResponseException exception) { + Logger.warn("Invalid session tracking payload", exception); } } }); @@ -165,20 +158,13 @@ void flushStoredSessions() { try { configuration.getDelivery().deliver(payload, configuration); sessionStore.deleteStoredFiles(storedFiles); - } catch (DeliveryFailureException exception) { - switch (exception.reason) { - case CONNECTIVITY: // store for later sending - sessionStore.cancelQueuedFiles(storedFiles); - Logger.info("Failed to post stored session payload"); - break; - case REQUEST_FAILURE: - // drop bad data - Logger.warn("Invalid session tracking payload", exception); - sessionStore.deleteStoredFiles(storedFiles); - break; - default: - break; - } + } catch (NetworkException 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); + sessionStore.deleteStoredFiles(storedFiles); } } } finally { From 73f5d2b500ab8a728c250922d9650961a41a4cfa Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 10 May 2018 15:16:18 +0100 Subject: [PATCH 33/43] refactor: update mazerunner to use delivery api Updates mazerunner to use the delivery api rather than deprecated api clients. --- .../android/mazerunner/scenarios/Scenario.kt | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) 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..2b6e999efb 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,34 @@ 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 NetworkException("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 NetworkException("Session Delivery NOP", RuntimeException("NOP")) + + } + } + } /** From 07e6f12e0e6ee0ed16c1023487344067df2c5c84 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 11 May 2018 16:13:55 +0100 Subject: [PATCH 34/43] refactor: use single DeliveryFailureException in Delivery API, deprecate existing two exceptions, ad --- .../bugsnag/android/BadResponseException.java | 4 +++ .../java/com/bugsnag/android/Delivery.java | 32 ++++++++----------- .../android/DeliveryFailureException.java | 17 ++++++++++ .../com/bugsnag/android/NetworkException.java | 4 +++ 4 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 sdk/src/main/java/com/bugsnag/android/DeliveryFailureException.java diff --git a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java index f9e175d65f..745a16ff06 100644 --- a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java +++ b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java @@ -1,5 +1,9 @@ package com.bugsnag.android; +/** + * @deprecated use {@link DeliveryFailureException} instead + */ +@Deprecated public class BadResponseException extends Exception { public BadResponseException(String msg, Throwable cause) { super(msg, cause); diff --git a/sdk/src/main/java/com/bugsnag/android/Delivery.java b/sdk/src/main/java/com/bugsnag/android/Delivery.java index f6f11e82bd..a680392797 100644 --- a/sdk/src/main/java/com/bugsnag/android/Delivery.java +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -23,23 +23,21 @@ public interface Delivery { * This request must be delivered to the endpoint at {@link Configuration#getSessionEndpoint()}, * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. *

- * If the response status code is not 202, then the implementation must throw - * {@link BadResponseException}. - *

- * If the request could not be delivered due to connectivity issues, then the implementation - * must throw {@link NetworkException}, as this will cache the request for - * delivery at a future time. + * If the request was not successful and you wish to try again later, throw + * {@link BadResponseException}. 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 BadResponseException when delivery does not receive a 202 status code. - * @throws NetworkException when delivery is not successful due to connectivity issues. + * @throws DeliveryFailureException when delivery is not successful and + * the report should be stored for future attempts. */ void deliver(SessionTrackingPayload payload, - Configuration config) throws BadResponseException, NetworkException; + Configuration config) throws DeliveryFailureException; /** * Posts an Error Report to the Bugsnag Error Reporting API. @@ -47,21 +45,19 @@ void deliver(SessionTrackingPayload payload, * This request must be delivered to the endpoint at {@link Configuration#getSessionEndpoint()}, * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. *

- * If the response status code is not 2xx, then the implementation must throw - * {@link BadResponseException}. - *

- * If the request could not be delivered due to connectivity issues, then the implementation - * must throw {@link NetworkException}, as this will cache the request for - * delivery at a future time. + * If the request was not successful and you wish to try again later, throw + * {@link BadResponseException}. 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 BadResponseException when delivery does not receive a 2xx status code. - * @throws NetworkException when delivery is not successful due to connectivity issues. + * @throws DeliveryFailureException when delivery is not successful and + * the report should be stored for future attempts. */ void deliver(Report report, - Configuration config) throws BadResponseException, NetworkException; + Configuration config) throws DeliveryFailureException; } 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/NetworkException.java b/sdk/src/main/java/com/bugsnag/android/NetworkException.java index 6236b6c125..6023a0fc58 100644 --- a/sdk/src/main/java/com/bugsnag/android/NetworkException.java +++ b/sdk/src/main/java/com/bugsnag/android/NetworkException.java @@ -2,6 +2,10 @@ import java.io.IOException; +/** + * @deprecated use {@link DeliveryFailureException} instead + */ +@Deprecated public class NetworkException extends IOException { public NetworkException(String msg, Throwable cause) { super(msg, cause); From 12cbbb682ebcf15cc5f413cc7b0ba52805ad0b39 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 11 May 2018 16:24:08 +0100 Subject: [PATCH 35/43] refactor: Convert legacy NetworkException and BadResponseException to DeliveryFailureException in De --- .../bugsnag/android/DeliveryCompatTest.java | 9 ++++++ .../com/bugsnag/android/DeliveryCompat.java | 29 ++++++++++++++----- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java index de0b5f621b..caa7d307aa 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -90,4 +90,13 @@ public void testClientCompat() { 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("", null)); + } } diff --git a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java index 99bdccc128..a277f94c1b 100644 --- a/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java +++ b/sdk/src/main/java/com/bugsnag/android/DeliveryCompat.java @@ -12,19 +12,34 @@ class DeliveryCompat implements Delivery { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws BadResponseException, NetworkException { + Configuration config) throws DeliveryFailureException { if (sessionTrackingApiClient != null) { - sessionTrackingApiClient.postSessionTrackingPayload(config.getSessionEndpoint(), - payload, config.getSessionApiHeaders()); + try { + sessionTrackingApiClient.postSessionTrackingPayload(config.getSessionEndpoint(), + payload, config.getSessionApiHeaders()); + } catch (Throwable throwable) { + handleException(throwable); + } } } @Override - public void deliver(Report report, Configuration config) - throws BadResponseException, NetworkException { + public void deliver(Report report, Configuration config) throws DeliveryFailureException { if (errorReportApiClient != null) { - errorReportApiClient.postReport(config.getEndpoint(), - report, config.getErrorApiHeaders()); + 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); } } From da6c00d9dc7a38d2c6829ac5e25c3f6d0ee76f35 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 11 May 2018 16:26:21 +0100 Subject: [PATCH 36/43] refactor: update defaultdelivery to use new deliveryfailureexception and log on bad response --- .../com/bugsnag/android/DefaultDelivery.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java index 235a386865..3c65791d7c 100644 --- a/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java +++ b/sdk/src/main/java/com/bugsnag/android/DefaultDelivery.java @@ -22,12 +22,12 @@ class DefaultDelivery implements Delivery { @Override public void deliver(SessionTrackingPayload payload, - Configuration config) throws BadResponseException, NetworkException { + Configuration config) throws DeliveryFailureException { String endpoint = config.getSessionEndpoint(); int status = deliver(endpoint, payload, config.getSessionApiHeaders()); if (status != 202) { - throw new BadResponseException("Request failed with status " + status, null); + Logger.warn("Session API request failed with status " + status, null); } else { Logger.info("Completed session tracking request"); } @@ -35,12 +35,12 @@ public void deliver(SessionTrackingPayload payload, @Override public void deliver(Report report, - Configuration config) throws BadResponseException, NetworkException { + Configuration config) throws DeliveryFailureException { String endpoint = config.getEndpoint(); int status = deliver(endpoint, report, config.getErrorApiHeaders()); if (status / 100 != 2) { - throw new BadResponseException("Request failed with status " + status, null); + Logger.warn("Error API request failed with status " + status, null); } else { Logger.info("Completed error API request"); } @@ -48,7 +48,7 @@ public void deliver(Report report, int deliver(String urlString, JsonStream.Streamable streamable, - Map headers) throws NetworkException { + Map headers) throws DeliveryFailureException { checkHasNetworkConnection(); HttpURLConnection conn = null; @@ -78,18 +78,18 @@ int deliver(String urlString, // End the request, get the response code return conn.getResponseCode(); } catch (IOException exception) { - throw new NetworkException("IOException encountered in request", exception); + throw new DeliveryFailureException("IOException encountered in request", exception); } finally { IOUtils.close(conn); } } - private void checkHasNetworkConnection() throws NetworkException { + private void checkHasNetworkConnection() throws DeliveryFailureException { NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo(); // conserve device battery by avoiding radio use if (!(activeNetworkInfo != null && activeNetworkInfo.isConnectedOrConnecting())) { - throw new NetworkException("No network connection available", null); + throw new DeliveryFailureException("No network connection available", null); } } } From 6ff5e1370fed75a2a087147a442654b6bd4a5e2f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 11 May 2018 16:35:36 +0100 Subject: [PATCH 37/43] refactor: update internal error handling to use deliveryfailureexception --- .../com/bugsnag/android/BugsnagTestUtils.java | 4 ++-- .../com/bugsnag/android/ClientConfigTest.java | 4 ++-- .../main/java/com/bugsnag/android/Client.java | 5 +---- .../java/com/bugsnag/android/ErrorStore.java | 10 +++------- .../java/com/bugsnag/android/SessionTracker.java | 16 ++++++++-------- 5 files changed, 16 insertions(+), 23 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index 43a45cc4d2..59eaaa1356 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -96,12 +96,12 @@ public static Delivery generateDelivery() { @Override public void deliver(SessionTrackingPayload payload, Configuration config) - throws BadResponseException, NetworkException {} + throws DeliveryFailureException {} @Override public void deliver(Report report, Configuration config) - throws BadResponseException, NetworkException {} + 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 46f8bca49f..8e1696009d 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ClientConfigTest.java @@ -100,12 +100,12 @@ public void testCustomDeliveryOverride() { @Override public void deliver(SessionTrackingPayload payload, Configuration config) - throws BadResponseException, NetworkException {} + throws DeliveryFailureException {} @Override public void deliver(Report report, Configuration config) - throws BadResponseException, NetworkException {} + throws DeliveryFailureException {} }; config.setDelivery(customDelivery); diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index a3b2292b36..4882440f1a 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -1252,12 +1252,9 @@ void deliver(@NonNull Report report, @NonNull Error error) { try { config.getDelivery().deliver(report, config); Logger.info("Sent 1 new error to Bugsnag"); - } catch (NetworkException exception) { + } catch (DeliveryFailureException exception) { Logger.info("Could not send error(s) to Bugsnag, saving to disk to send later"); - // Save error to disk for later sending 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/ErrorStore.java b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java index fa9255a159..7afc6a7ec3 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java @@ -127,20 +127,16 @@ private void flushErrorReport(File errorFile) { 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); } catch (Exception exception) { - handleRequestFailure(errorFile, exception); + deleteStoredFiles(Collections.singleton(errorFile)); + Logger.warn("Problem sending unsent error from disk", exception); } } - private void handleRequestFailure(File errorFile, Exception exception) { - deleteStoredFiles(Collections.singleton(errorFile)); - Logger.warn("Problem sending unsent error from disk", exception); - } - boolean isLaunchCrashReport(File file) { return file.getName().endsWith("_startupcrash.json"); } diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index 7a5e80288a..3a7f319f8a 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -91,11 +91,11 @@ public void run() { try { configuration.getDelivery().deliver(payload, configuration); - } catch (NetworkException exception) { // store for later sending - Logger.info("Failed to post session payload"); + } catch (DeliveryFailureException exception) { // store for later sending + Logger.info("Storing session payload for future delivery"); sessionStore.write(session); - } catch (BadResponseException exception) { - Logger.warn("Invalid session tracking payload", exception); + } catch (Exception exception) { + Logger.warn("Dropping invalid session tracking payload", exception); } } }); @@ -158,12 +158,12 @@ void flushStoredSessions() { try { configuration.getDelivery().deliver(payload, configuration); sessionStore.deleteStoredFiles(storedFiles); - } catch (NetworkException exception) { + } catch (DeliveryFailureException exception) { sessionStore.cancelQueuedFiles(storedFiles); - Logger.info("Failed to post stored session payload"); - } catch (BadResponseException exception) { + Logger.info("Leaving session payload for future delivery"); + } catch (Exception exception) { // drop bad data - Logger.warn("Invalid session tracking payload", exception); + Logger.warn("Deleting invalid session tracking payload", exception); sessionStore.deleteStoredFiles(storedFiles); } } From 3b5babdf98c874bae1df19c0cb91a79978741c82 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 11 May 2018 16:44:35 +0100 Subject: [PATCH 38/43] refactor: update logging to include exceptions as requested --- sdk/src/main/java/com/bugsnag/android/Client.java | 2 +- sdk/src/main/java/com/bugsnag/android/SessionTracker.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 4882440f1a..e27362f519 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -1253,7 +1253,7 @@ void deliver(@NonNull Report report, @NonNull Error error) { config.getDelivery().deliver(report, config); Logger.info("Sent 1 new error to Bugsnag"); } catch (DeliveryFailureException exception) { - Logger.info("Could not send error(s) to Bugsnag, saving to disk to send later"); + Logger.warn("Could not send error(s) to Bugsnag, saving to disk to send later", exception); errorStore.write(error); } catch (Exception exception) { Logger.warn("Problem sending error to Bugsnag", exception); diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index 3a7f319f8a..0348b81173 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -92,7 +92,7 @@ public void run() { try { configuration.getDelivery().deliver(payload, configuration); } catch (DeliveryFailureException exception) { // store for later sending - Logger.info("Storing session payload for future delivery"); + Logger.warn("Storing session payload for future delivery", exception); sessionStore.write(session); } catch (Exception exception) { Logger.warn("Dropping invalid session tracking payload", exception); @@ -160,7 +160,7 @@ void flushStoredSessions() { sessionStore.deleteStoredFiles(storedFiles); } catch (DeliveryFailureException exception) { sessionStore.cancelQueuedFiles(storedFiles); - Logger.info("Leaving session payload for future delivery"); + Logger.warn("Leaving session payload for future delivery", exception); } catch (Exception exception) { // drop bad data Logger.warn("Deleting invalid session tracking payload", exception); From 792f0175fa4202c769412033f02beb5e468559dd Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 11 May 2018 16:48:34 +0100 Subject: [PATCH 39/43] refactor: update mazerunner to use new deliveryfailureexception --- .../com/bugsnag/android/mazerunner/scenarios/Scenario.kt | 5 ++--- sdk/src/main/java/com/bugsnag/android/Client.java | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) 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 2b6e999efb..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 @@ -20,7 +20,7 @@ abstract internal class Scenario(protected val config: Configuration, val baseDelivery = Bugsnag.getClient().config.delivery Bugsnag.getClient().config.delivery = object: Delivery { override fun deliver(payload: SessionTrackingPayload?, config: Configuration?) { - throw NetworkException("Session Delivery NOP", RuntimeException("NOP")) + throw DeliveryFailureException("Session Delivery NOP", RuntimeException("NOP")) } override fun deliver(report: Report?, config: Configuration?) { @@ -40,8 +40,7 @@ abstract internal class Scenario(protected val config: Configuration, } override fun deliver(report: Report?, config: Configuration?) { - throw NetworkException("Session Delivery NOP", RuntimeException("NOP")) - + throw DeliveryFailureException("Session Delivery NOP", RuntimeException("NOP")) } } diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index e27362f519..788568719a 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -1253,7 +1253,8 @@ void deliver(@NonNull Report report, @NonNull Error error) { config.getDelivery().deliver(report, config); Logger.info("Sent 1 new error to Bugsnag"); } catch (DeliveryFailureException exception) { - Logger.warn("Could not send error(s) to Bugsnag, saving to disk to send later", exception); + Logger.warn("Could not send error(s) to Bugsnag," + + " saving to disk to send later", exception); errorStore.write(error); } catch (Exception exception) { Logger.warn("Problem sending error to Bugsnag", exception); From 6d84cdd9fa79a171a39eb6e2bbe5eb93f68ebbbd Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Mon, 14 May 2018 09:08:33 +0100 Subject: [PATCH 40/43] docs: use correct exception subclass in javadocs --- sdk/src/main/java/com/bugsnag/android/Delivery.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Delivery.java b/sdk/src/main/java/com/bugsnag/android/Delivery.java index a680392797..2bb75990c2 100644 --- a/sdk/src/main/java/com/bugsnag/android/Delivery.java +++ b/sdk/src/main/java/com/bugsnag/android/Delivery.java @@ -24,7 +24,7 @@ public interface Delivery { * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. *

* If the request was not successful and you wish to try again later, throw - * {@link BadResponseException}. The notifier will cache the payload and initiate delivery + * {@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. *

@@ -46,7 +46,7 @@ void deliver(SessionTrackingPayload payload, * and contain the HTTP headers from {@link Configuration#getSessionApiHeaders()}. *

* If the request was not successful and you wish to try again later, throw - * {@link BadResponseException}. The notifier will cache the payload and initiate delivery + * {@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. *

From 7ca12a72a83b71660275ea642b3fe40eec5286f2 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 17 May 2018 10:38:06 +0100 Subject: [PATCH 41/43] refactor: avoid breaking compatibility with previous badresponseexception constructor signature --- .../java/com/bugsnag/android/DeliveryCompatTest.java | 2 +- .../main/java/com/bugsnag/android/BadResponseException.java | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java index caa7d307aa..919e280967 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/DeliveryCompatTest.java @@ -97,6 +97,6 @@ public void testExceptionConversion() throws Exception { @Test public void testSwallowExceptionConversion() throws Exception { // no exception thrown - deliveryCompat.handleException(new BadResponseException("", null)); + deliveryCompat.handleException(new BadResponseException("", 0)); } } diff --git a/sdk/src/main/java/com/bugsnag/android/BadResponseException.java b/sdk/src/main/java/com/bugsnag/android/BadResponseException.java index 745a16ff06..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 msg, Throwable cause) { - super(msg, cause); + public BadResponseException(String msg, int responseCode) { + super(String.format(Locale.US, "%s (%d)", msg, responseCode)); } } From 6712cc48cf12c7002f8c4cdf5c02f614d18f6c68 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 17 May 2018 10:46:30 +0100 Subject: [PATCH 42/43] v4.4.0 --- CHANGELOG.md | 19 +++++++++++++++++++ gradle.properties | 2 +- .../java/com/bugsnag/android/Notifier.java | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) 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/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/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; From 48a807f975846dab2f9f7170b31468a15fa83c15 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 17 May 2018 10:48:54 +0100 Subject: [PATCH 43/43] style: organise test imports to pass checkstyle --- .../java/com/bugsnag/android/JsonWriterBugsnagTest.java | 4 ++-- .../java/com/bugsnag/android/JsonWriterTest.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java index 123c214fdd..9866644d08 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterBugsnagTest.java @@ -1,5 +1,7 @@ package com.bugsnag.android; +import static org.junit.Assert.assertEquals; + import android.support.test.filters.MediumTest; import android.support.test.runner.AndroidJUnit4; @@ -10,8 +12,6 @@ import java.io.IOException; import java.io.OutputStreamWriter; -import static org.junit.Assert.assertEquals; - @RunWith(AndroidJUnit4.class) @MediumTest public class JsonWriterBugsnagTest { diff --git a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java index 36783136e2..4ffb643e20 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/JsonWriterTest.java @@ -1,11 +1,11 @@ package com.bugsnag.android; -import android.support.test.filters.MediumTest; -import android.support.test.runner.AndroidJUnit4; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import android.support.test.filters.MediumTest; +import android.support.test.runner.AndroidJUnit4; + import org.junit.Test; import org.junit.runner.RunWith;