diff --git a/Gemfile.lock b/Gemfile.lock index 0a41d4b585..9028431903 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: git@github.com:bugsnag/maze-runner - revision: 9f5b7f220441d047888ffdd91bd6b32be6367078 + revision: f7123450d5a75b719911c6dd3baa0507e6062c2d specs: bugsnag-maze-runner (1.0.0) cucumber (~> 3.1.0) @@ -12,7 +12,7 @@ GIT GEM remote: https://rubygems.org/ specs: - backports (3.11.2) + backports (3.11.3) builder (3.2.3) coderay (1.1.2) cucumber (3.1.0) diff --git a/features/multi_client.feature b/features/multi_client.feature index 4c18e5bbff..01b9f483f0 100644 --- a/features/multi_client.feature +++ b/features/multi_client.feature @@ -4,6 +4,14 @@ Feature: Multi-client support configuration options. A report which is captured by a given client should use the correct API key and configuration options. +Scenario: Multiple clients send errors stored in the old directory + When I run "MultiClientOldDirScenario" with the defaults + When I force stop the "com.bugsnag.android.mazerunner" Android app + And I set environment variable "EVENT_TYPE" to "MultiClientOldDirScenario" + And I set environment variable "EVENT_METADATA" to "DeliverReport" + And I start the "com.bugsnag.android.mazerunner" Android app using the "com.bugsnag.android.mazerunner.MainActivity" activity + Then I should receive 2 requests + Scenario: A handled error captured while offline is only sent by the original client When I run "MultiClientNotifyScenario" with the defaults When I force stop the "com.bugsnag.android.mazerunner" Android app @@ -56,4 +64,4 @@ Scenario: Sessions while captured offline is detected by two clients with differ And the "Bugsnag-API-Key" header equals "a35a2a72bd230ac0aa0f52715bbdc6aa" for request 0 And the payload field "sessions.0.user.name" equals "Bob" for request 0 - And the payload field "sessions" is an array with 1 element for request 10 + And the payload field "sessions" is an array with 1 element for request 0 diff --git a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt index fca5cb5b90..113810be6d 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt @@ -2,6 +2,8 @@ package com.bugsnag.android import android.content.Context import android.net.ConnectivityManager +import java.io.File +import java.io.FileWriter /** * Accesses the session tracker and flushes all stored sessions @@ -33,6 +35,19 @@ internal fun createSlowErrorApiClient(context: Context): ErrorReportApiClient { }) } +/** + * Writes an error to the old directory format (i.e. bugsnag-errors) + */ +internal fun writeErrorToOldDir(client: Client) { + val configuration = Configuration("api-key") + val error = Error.Builder(configuration, RuntimeException(), null).build() + val filename = client.errorStore.getFilename(error) + + val file = File(client.errorStore.oldDirectory, filename) + val writer = JsonStream(FileWriter(file)) + error.toStream(writer) +} + 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/MultiClientOldDirScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiClientOldDirScenario.kt new file mode 100644 index 0000000000..317a535328 --- /dev/null +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiClientOldDirScenario.kt @@ -0,0 +1,39 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Client +import com.bugsnag.android.Configuration +import com.bugsnag.android.disableAllDelivery +import com.bugsnag.android.writeErrorToOldDir + +/** + * Configures two Bugsnag clients with different API keys. A single error report is written to the + * old directory format, which should then be reported by both clients. + * + */ +internal class MultiClientOldDirScenario(config: Configuration, + context: Context) : Scenario(config, context) { + var firstClient: Client? = null + var secondClient: Client? = null + + fun configureClients() { + firstClient = Client(context, config) + + Thread.sleep(10) // enforce request order + val secondConfig = Configuration("abc123") + secondConfig.endpoint = config.endpoint + secondConfig.sessionEndpoint = config.sessionEndpoint + secondClient = Client(context, secondConfig) + } + + override fun run() { + configureClients() + + if ("DeliverReport" != eventMetaData) { + disableAllDelivery(firstClient!!) + disableAllDelivery(secondClient!!) + writeErrorToOldDir(firstClient!!) + } + } + +} diff --git a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiClientSessionScenario.kt b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiClientSessionScenario.kt index 983f26c4f3..2a4498e586 100644 --- a/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiClientSessionScenario.kt +++ b/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiClientSessionScenario.kt @@ -35,7 +35,7 @@ internal class MultiClientSessionScenario(config: Configuration, secondClient!!.startSession() } else { flushAllSessions(firstClient!!) - Thread.sleep(10) // enforce request order + Thread.sleep(50) // enforce request order flushAllSessions(secondClient!!) } } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java index 840c238557..d7ab963e51 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java @@ -23,6 +23,7 @@ import java.io.File; import java.io.IOException; import java.io.StringWriter; +import java.util.List; @RunWith(AndroidJUnit4.class) @SmallTest @@ -42,9 +43,8 @@ public void setUp() throws Exception { Client client = new Client(InstrumentationRegistry.getContext(), "api-key"); config = client.config; errorStore = client.errorStore; - Assert.assertNotNull(errorStore.storeDirectory); - errorStorageDir = new File(errorStore.storeDirectory); - FileUtils.clearFilesInDir(errorStorageDir); + errorStorageDir = errorStore.storageDir; + FileUtils.clearFiles(errorStore); } /** @@ -54,7 +54,7 @@ public void setUp() throws Exception { */ @After public void tearDown() throws Exception { - FileUtils.clearFilesInDir(errorStorageDir); + FileUtils.clearFiles(errorStore); } @Test @@ -123,6 +123,19 @@ public void isStartupCrash() throws Exception { assertFalse(errorStore.isStartupCrash(10000)); } + @Test + public void testFindOldFiles() throws Throwable { + new File(errorStore.oldDirectory, "foo.json").createNewFile(); + File dir = errorStore.storageDir; + new File(dir, "foo.json").createNewFile(); + + List files = errorStore.findStoredFiles(); + for (File file : files) { + assertTrue(file.getAbsolutePath().endsWith("foo.json")); + } + assertEquals(2, files.size()); + } + /** * Ensures that the file can be serialised back into a JSON report, and contains the same info * as the original diff --git a/sdk/src/androidTest/java/com/bugsnag/android/FileUtils.java b/sdk/src/androidTest/java/com/bugsnag/android/FileUtils.java index 6cae7efda1..bae1d92258 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/FileUtils.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/FileUtils.java @@ -7,7 +7,13 @@ final class FileUtils { private FileUtils() { } - static void clearFilesInDir(File storageDir) { + static void clearFiles(FileStore fileStore) { + File storageDir = fileStore.storageDir; + clearFilesInDir(storageDir); + clearFilesInDir(new File(fileStore.oldDirectory)); + } + + private static void clearFilesInDir(File storageDir) { if (!storageDir.isDirectory()) { throw new IllegalArgumentException(); } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/SessionStoreTest.java b/sdk/src/androidTest/java/com/bugsnag/android/SessionStoreTest.java index b1b10ba52e..4c4d25afbf 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/SessionStoreTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/SessionStoreTest.java @@ -15,12 +15,14 @@ import org.junit.runner.RunWith; import java.io.File; +import java.util.List; @RunWith(AndroidJUnit4.class) @SmallTest public class SessionStoreTest { private File storageDir; + private SessionStore sessionStore; /** * Generates a session store with 0 files @@ -30,10 +32,9 @@ public class SessionStoreTest { @Before public void setUp() throws Exception { Client client = new Client(InstrumentationRegistry.getContext(), "api-key"); - SessionStore sessionStore = client.sessionStore; - assertNotNull(sessionStore.storeDirectory); - storageDir = new File(sessionStore.storeDirectory); - FileUtils.clearFilesInDir(storageDir); + sessionStore = client.sessionStore; + storageDir = sessionStore.storageDir; + FileUtils.clearFiles(sessionStore); } /** @@ -43,7 +44,7 @@ public void setUp() throws Exception { */ @After public void tearDown() throws Exception { - FileUtils.clearFilesInDir(storageDir); + FileUtils.clearFiles(sessionStore); } @Test @@ -70,4 +71,17 @@ public void testComparator() throws Exception { assertTrue(SESSION_COMPARATOR.compare(new File(second), new File(startup)) > 0); } + @Test + public void testFindOldFiles() throws Throwable { + new File(sessionStore.oldDirectory, "foo.json").createNewFile(); + File dir = sessionStore.storageDir; + new File(dir, "foo.json").createNewFile(); + + List files = sessionStore.findStoredFiles(); + for (File file : files) { + assertTrue(file.getAbsolutePath().endsWith("foo.json")); + } + assertEquals(2, files.size()); + } + } diff --git a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackingPayloadTest.java b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackingPayloadTest.java index aeb48e9435..4c5d3416b3 100644 --- a/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackingPayloadTest.java +++ b/sdk/src/androidTest/java/com/bugsnag/android/SessionTrackingPayloadTest.java @@ -26,7 +26,6 @@ public class SessionTrackingPayloadTest { private AppData appData; private SessionStore sessionStore; - private File storageDir; /** * Configures a session tracking payload and session store, ensuring that 0 files are present @@ -38,9 +37,7 @@ public void setUp() throws Exception { Context context = InstrumentationRegistry.getContext(); Client client = new Client(context, "api-key"); sessionStore = client.sessionStore; - Assert.assertNotNull(sessionStore.storeDirectory); - storageDir = new File(sessionStore.storeDirectory); - FileUtils.clearFilesInDir(storageDir); + FileUtils.clearFiles(sessionStore); session = generateSession(); appData = new AppData(context, new Configuration("a"), generateSessionTracker()); @@ -55,7 +52,7 @@ public void setUp() throws Exception { */ @After public void tearDown() throws Exception { - FileUtils.clearFilesInDir(storageDir); + FileUtils.clearFiles(sessionStore); } @Test diff --git a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java index 4415d79d6e..804d623a20 100644 --- a/sdk/src/main/java/com/bugsnag/android/ErrorStore.java +++ b/sdk/src/main/java/com/bugsnag/android/ErrorStore.java @@ -46,7 +46,7 @@ public int compare(File lhs, File rhs) { ErrorStore(@NonNull Configuration config, @NonNull Context appContext) { super(config, appContext, - "/bugsnag-errors/", 128, ERROR_REPORT_COMPARATOR); + "bugsnag-errors", 128, ERROR_REPORT_COMPARATOR); } void flushOnLaunch(final ErrorReportApiClient errorReportApiClient) { @@ -88,10 +88,9 @@ public void run() { * Flush any on-disk errors to Bugsnag */ void flushAsync(final ErrorReportApiClient errorReportApiClient) { - if (storeDirectory == null) { + if (storageDir == null) { return; } - try { Async.run(new Runnable() { @Override @@ -162,8 +161,9 @@ private List findLaunchCrashReports() { String getFilename(Error error) { boolean isStartupCrash = isStartupCrash(AppData.getDurationMs()); String suffix = isStartupCrash ? STARTUP_CRASH : ""; - return String.format(Locale.US, "%s%d_%s%s.json", - storeDirectory, System.currentTimeMillis(), UUID.randomUUID().toString(), suffix); + return String.format(Locale.US, "%s/%d_%s%s.json", + storageDir.getAbsolutePath(), System.currentTimeMillis(), + UUID.randomUUID().toString(), suffix); } boolean isStartupCrash(long durationMs) { diff --git a/sdk/src/main/java/com/bugsnag/android/FileStore.java b/sdk/src/main/java/com/bugsnag/android/FileStore.java index 0593c2bb9c..9d4a9b1801 100644 --- a/sdk/src/main/java/com/bugsnag/android/FileStore.java +++ b/sdk/src/main/java/com/bugsnag/android/FileStore.java @@ -7,8 +7,10 @@ import java.io.File; import java.io.FileWriter; import java.io.Writer; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -16,8 +18,11 @@ abstract class FileStore { @NonNull protected final Configuration config; + @Nullable - final String storeDirectory; + final String oldDirectory; + + File storageDir; private final int maxStoreCount; private final Comparator comparator; @@ -29,11 +34,11 @@ abstract class FileStore { String path; try { - path = appContext.getCacheDir().getAbsolutePath() + folder; + File baseDir = new File(appContext.getCacheDir().getAbsolutePath(), folder); + path = baseDir.getAbsolutePath(); + storageDir = getStorageDir(path, config); - File outFile = new File(path); - outFile.mkdirs(); - if (!outFile.exists()) { + if (!storageDir.exists()) { Logger.warn("Could not prepare file storage directory"); path = null; } @@ -41,26 +46,27 @@ abstract class FileStore { Logger.warn("Could not prepare file storage directory", exception); path = null; } - this.storeDirectory = path; + this.oldDirectory = path; } @Nullable String write(@NonNull T streamable) { - if (storeDirectory == null) { + if (storageDir == null) { return null; } // Limit number of saved errors to prevent disk space issues - File exceptionDir = new File(storeDirectory); - if (exceptionDir.isDirectory()) { - File[] files = exceptionDir.listFiles(); - if (files != null && files.length >= maxStoreCount) { + if (storageDir.isDirectory()) { + List storedFiles = findStoredFiles(); + + if (storedFiles.size() >= maxStoreCount) { // Sort files then delete the first one (oldest timestamp) - Arrays.sort(files, comparator); + Collections.sort(storedFiles, comparator); + File oldestFile = storedFiles.get(0); Logger.warn(String.format("Discarding oldest error as stored " - + "error limit reached (%s)", files[0].getPath())); - if (!files[0].delete()) { - files[0].deleteOnExit(); + + "error limit reached (%s)", oldestFile.getPath())); + if (!oldestFile.delete()) { + oldestFile.deleteOnExit(); } } } @@ -86,22 +92,50 @@ String write(@NonNull T streamable) { return null; } - @NonNull abstract String getFilename(T streamable); + @NonNull + abstract String getFilename(T streamable); List findStoredFiles() { List files = new ArrayList<>(); - if (storeDirectory != null) { - File dir = new File(storeDirectory); + if (oldDirectory != null) { + File dir = new File(oldDirectory); + addStoredFiles(dir, files); + } + if (storageDir != null) { + addStoredFiles(storageDir, files); + } + return files; + } - if (dir.exists() && dir.isDirectory()) { - File[] values = dir.listFiles(); + void addStoredFiles(File dir, List files) { + if (!dir.exists() || !dir.isDirectory()) { + return; + } + File[] values = dir.listFiles(); - if (values != null) { - files.addAll(Arrays.asList(values)); + if (values != null) { + for (File value : values) { + if (value.isFile()) { + files.add(value); } } } - return files; } + + // support multiple clients in the same app by using a unique directory path + + private File getStorageDir(String path, @NonNull Configuration config) { + String apiKey = "" + config.getApiKey().hashCode(); + String endpoint = "" + config.getEndpoint().hashCode(); + + File apiDir = new File(path, apiKey); + apiDir.mkdirs(); + + File dir = new File(apiDir, endpoint); + dir.mkdirs(); + + return dir; + } + } diff --git a/sdk/src/main/java/com/bugsnag/android/NativeInterface.java b/sdk/src/main/java/com/bugsnag/android/NativeInterface.java index e1e42ddd8d..17a722c58a 100644 --- a/sdk/src/main/java/com/bugsnag/android/NativeInterface.java +++ b/sdk/src/main/java/com/bugsnag/android/NativeInterface.java @@ -65,7 +65,7 @@ public static String getContext() { @Nullable public static String getErrorStorePath() { - return getClient().errorStore.storeDirectory; + return getClient().errorStore.storageDir.getAbsolutePath(); } public static String getUserId() { diff --git a/sdk/src/main/java/com/bugsnag/android/SessionStore.java b/sdk/src/main/java/com/bugsnag/android/SessionStore.java index ad084213b8..23f90e5cf3 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionStore.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionStore.java @@ -33,15 +33,15 @@ public int compare(File lhs, File rhs) { }; SessionStore(@NonNull Configuration config, @NonNull Context appContext) { - super(config, appContext, "/bugsnag-sessions/", + super(config, appContext, "bugsnag-sessions", 128, SESSION_COMPARATOR); } @NonNull @Override String getFilename(Session session) { - return String.format(Locale.US, "%s%s%d.json", - storeDirectory, UUID.randomUUID().toString(), System.currentTimeMillis()); + return String.format(Locale.US, "%s/%s%d.json", + storageDir.getAbsolutePath(), UUID.randomUUID().toString(), System.currentTimeMillis()); } }