Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multi client storage #290

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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)
Expand Down
10 changes: 9 additions & 1 deletion features/multi_client.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
15 changes: 15 additions & 0 deletions mazerunner/src/main/java/com/bugsnag/android/TestHarnessHooks.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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!!)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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!!)
}
}
Expand Down
21 changes: 17 additions & 4 deletions sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.File;
import java.io.IOException;
import java.io.StringWriter;
import java.util.List;

@RunWith(AndroidJUnit4.class)
@SmallTest
Expand All @@ -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);
}

/**
Expand All @@ -54,7 +54,7 @@ public void setUp() throws Exception {
*/
@After
public void tearDown() throws Exception {
FileUtils.clearFilesInDir(errorStorageDir);
FileUtils.clearFiles(errorStore);
}

@Test
Expand Down Expand Up @@ -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<File> files = errorStore.findStoredFiles();
for (File file : files) {
assertTrue(file.getAbsolutePath().endsWith("foo.json"));
}
assertEquals(2, files.size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are multiple clients, will all attempt to find and send files from the "old" directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I've added a mazerunner scenario which covers this.

}

/**
* Ensures that the file can be serialised back into a JSON report, and contains the same info
* as the original
Expand Down
8 changes: 7 additions & 1 deletion sdk/src/androidTest/java/com/bugsnag/android/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
24 changes: 19 additions & 5 deletions sdk/src/androidTest/java/com/bugsnag/android/SessionStoreTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}

/**
Expand All @@ -43,7 +44,7 @@ public void setUp() throws Exception {
*/
@After
public void tearDown() throws Exception {
FileUtils.clearFilesInDir(storageDir);
FileUtils.clearFiles(sessionStore);
}

@Test
Expand All @@ -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<File> files = sessionStore.findStoredFiles();
for (File file : files) {
assertTrue(file.getAbsolutePath().endsWith("foo.json"));
}
assertEquals(2, files.size());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
Expand All @@ -55,7 +52,7 @@ public void setUp() throws Exception {
*/
@After
public void tearDown() throws Exception {
FileUtils.clearFilesInDir(storageDir);
FileUtils.clearFiles(sessionStore);
}

@Test
Expand Down
10 changes: 5 additions & 5 deletions sdk/src/main/java/com/bugsnag/android/ErrorStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -162,8 +161,9 @@ private List<File> 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) {
Expand Down