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

Use v2 API format #111

Merged
merged 9 commits into from
Jul 22, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
all: build

.PHONY: build test clean

build:
./gradlew build

clean:
./gradlew clean

test:
./gradlew :connectedCheck
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.bugsnag.android.MetaData;
import com.bugsnag.android.Severity;
import com.bugsnag.android.other.Other;
import com.bugsnag.android.BreadcrumbType;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -47,7 +48,7 @@ public boolean run(Error error) {
Bugsnag.addToTab("user", "age", 31);
Bugsnag.addToTab("custom", "account", "something");

Bugsnag.leaveBreadcrumb("onCreate");
Bugsnag.leaveBreadcrumb("onCreate", BreadcrumbType.NAVIGATION, new HashMap<String, String>());

new Thread(new Runnable() {
public void run() {
Expand Down
43 changes: 38 additions & 5 deletions src/androidTest/java/com/bugsnag/android/BreadcrumbsTest.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.bugsnag.android;

import java.io.IOException;
import java.util.HashMap;

import org.json.JSONException;
import org.json.JSONArray;
import org.json.JSONObject;

public class BreadcrumbsTest extends BugsnagTestCase {
public void testSerialization() throws JSONException, IOException {
Expand All @@ -14,7 +16,7 @@ public void testSerialization() throws JSONException, IOException {

JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);
assertEquals(3, breadcrumbsJson.length());
assertEquals("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim", breadcrumbsJson.getJSONArray(2).get(1));
assertEquals("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim", breadcrumbsJson.getJSONObject(2).getJSONObject("metaData").get("message"));
}

public void testSizeLimit() throws JSONException, IOException {
Expand All @@ -29,8 +31,8 @@ public void testSizeLimit() throws JSONException, IOException {

JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);
assertEquals(5, breadcrumbsJson.length());
assertEquals("2", breadcrumbsJson.getJSONArray(0).get(1));
assertEquals("6", breadcrumbsJson.getJSONArray(4).get(1));
assertEquals("2", breadcrumbsJson.getJSONObject(0).getJSONObject("metaData").get("message"));
assertEquals("6", breadcrumbsJson.getJSONObject(4).getJSONObject("metaData").get("message"));
}

public void testResize() throws JSONException, IOException {
Expand All @@ -45,8 +47,8 @@ public void testResize() throws JSONException, IOException {

JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);
assertEquals(5, breadcrumbsJson.length());
assertEquals("2", breadcrumbsJson.getJSONArray(0).get(1));
assertEquals("6", breadcrumbsJson.getJSONArray(4).get(1));
assertEquals("2", breadcrumbsJson.getJSONObject(0).getJSONObject("metaData").get("message"));
assertEquals("6", breadcrumbsJson.getJSONObject(4).getJSONObject("metaData").get("message"));
}

public void testClear() throws JSONException, IOException {
Expand All @@ -59,4 +61,35 @@ public void testClear() throws JSONException, IOException {
JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);
assertEquals(0, breadcrumbsJson.length());
}

public void testType() throws JSONException, IOException {
Breadcrumbs breadcrumbs = new Breadcrumbs();
breadcrumbs.add("1");
JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);
assertEquals("manual", breadcrumbsJson.getJSONObject(0).get("type"));
}

public void testPayloadSizeLimit() throws JSONException, IOException {
Breadcrumbs breadcrumbs = new Breadcrumbs();
HashMap<String, String> metadata = new HashMap<String, String>();
for (int i = 0; i < 400; i++) {
metadata.put(String.format("%d", i), "!!");
}
breadcrumbs.add("Rotated Menu", BreadcrumbType.STATE, metadata);
JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);
assertEquals(0, breadcrumbsJson.length());
}

public void testPayloadType() throws JSONException, IOException {
Breadcrumbs breadcrumbs = new Breadcrumbs();
HashMap<String, String> metadata = new HashMap<String, String>();
metadata.put("direction", "left");
breadcrumbs.add("Rotated Menu", BreadcrumbType.STATE, metadata);
JSONArray breadcrumbsJson = streamableToJsonArray(breadcrumbs);

assertEquals("Rotated Menu", breadcrumbsJson.getJSONObject(0).get("name"));
assertEquals("state", breadcrumbsJson.getJSONObject(0).get("type"));
assertEquals("left", breadcrumbsJson.getJSONObject(0).getJSONObject("metaData").get("direction"));
assertEquals(1, breadcrumbsJson.length());
}
}
2 changes: 1 addition & 1 deletion src/androidTest/java/com/bugsnag/android/ErrorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void testBasicSerialization() throws JSONException, IOException {

JSONObject errorJson = streamableToJson(error);
assertEquals("warning", errorJson.get("severity"));
assertEquals("2", errorJson.get("payloadVersion"));
assertEquals("3", errorJson.get("payloadVersion"));
assertNotNull(errorJson.get("severity"));
assertNotNull(errorJson.get("metaData"));
assertNotNull(errorJson.get("threads"));
Expand Down
48 changes: 48 additions & 0 deletions src/main/java/com/bugsnag/android/BreadcrumbType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.bugsnag.android;

/**
* Recognized types of breadcrumbs
*/
public enum BreadcrumbType {
/**
* An error was sent to Bugsnag (internal use only)
*/
ERROR ("error"),
/**
* A log message
*/
LOG ("log"),
/**
* A manual invocation of `leaveBreadcrumb` (default)
*/
MANUAL ("manual"),
/**
* A navigation event, such as a window opening or closing
*/
NAVIGATION ("navigation"),
/**
* A background process such as a database query
*/
PROCESS ("process"),
/**
* A network request
*/
REQUEST ("request"),
/**
* A change in application state, such as launch or memory warning
*/
STATE ("state"),
/**
* A user action, such as tapping a button
*/
USER ("user");

private final String type;

BreadcrumbType(String type) {
this.type = type;
}

String serialize() { return type; }
Copy link
Contributor

Choose a reason for hiding this comment

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

How about making this the implementation of toString()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The compiler wasn't a fan, though maybe I missed something:

toString() in BreadcrumbType cannot override toString() in Enum

Copy link

Choose a reason for hiding this comment

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

Did you make it public? If you override toString you need to make it public otherwise it will be trying to reduce the access privileges of an inherited method which is not allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense and would've made a good error message. Patched in b63033e. Non-breaking change since it was package scoped before.

}

84 changes: 72 additions & 12 deletions src/main/java/com/bugsnag/android/Breadcrumbs.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,89 @@
import android.support.annotation.NonNull;

import java.io.IOException;
import java.io.StringWriter;
import java.util.Date;
import java.util.Queue;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentLinkedQueue;


class Breadcrumbs implements JsonStream.Streamable {
private static class Breadcrumb {
private static class Breadcrumb implements JsonStream.Streamable {
private static final int MAX_MESSAGE_LENGTH = 140;
private static final String DEFAULT_NAME = "manual";
private static final String MESSAGE_METAKEY = "message";
private final String TIMESTAMP_KEY = "timestamp";
private final String NAME_KEY = "name";
private final String METADATA_KEY = "metaData";
private final String TYPE_KEY = "type";
final String timestamp;
final String message;
final String name;
final BreadcrumbType type;
final HashMap<String, String> metadata;

Breadcrumb(@NonNull String message) {
this.timestamp = DateUtils.toISO8601(new Date());
this.message = message.substring(0, Math.min(message.length(), MAX_MESSAGE_LENGTH));
this.type = BreadcrumbType.MANUAL;
HashMap<String, String> metadata = new HashMap<String, String>();
metadata.put(MESSAGE_METAKEY,
message.substring(0, Math.min(message.length(), MAX_MESSAGE_LENGTH)));
this.metadata = metadata;
this.name = DEFAULT_NAME;
}

Breadcrumb(@NonNull String name, BreadcrumbType type, HashMap<String, String> metadata) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Map not HashMap

this.timestamp = DateUtils.toISO8601(new Date());
this.type = type;
this.metadata = metadata;
this.name = name;
}

public void toStream(@NonNull JsonStream writer) throws IOException {
writer.beginObject();
writer.name(TIMESTAMP_KEY).value(this.timestamp);
writer.name(NAME_KEY).value(this.name);
writer.name(TYPE_KEY).value(this.type.serialize());
writer.name(METADATA_KEY);
writer.beginObject();
for (Map.Entry<String, String> entry : this.metadata.entrySet()) {
writer.name(entry.getKey()).value(entry.getValue());
}
writer.endObject();
writer.endObject();
}

public int payloadSize() throws IOException {
StringWriter writer = new StringWriter();
JsonStream jsonStream = new JsonStream(writer);
toStream(jsonStream);

return writer.toString().length();
}
}

private static final int DEFAULT_MAX_SIZE = 20;
private static final int MAX_PAYLOAD_SIZE = 4096;
private final Queue<Breadcrumb> store = new ConcurrentLinkedQueue<>();
private int maxSize = DEFAULT_MAX_SIZE;

public void toStream(@NonNull JsonStream writer) throws IOException {
writer.beginArray();

for (Breadcrumb breadcrumb : store) {
writer.beginArray();
writer.value(breadcrumb.timestamp);
writer.value(breadcrumb.message);
writer.endArray();
breadcrumb.toStream(writer);
}

writer.endArray();
}

void add(@NonNull String message) {
if (store.size() >= maxSize) {
// Remove oldest breadcrumb
store.poll();
}
store.add(new Breadcrumb(message));
addToStore(new Breadcrumb(message));
}

void add(@NonNull String name, BreadcrumbType type, HashMap<String, String> metadata) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Map not HashMap

addToStore(new Breadcrumb(name, type, metadata));
}

void clear() {
Expand All @@ -58,4 +102,20 @@ void setSize(int size) {
}
}
}

private void addToStore(Breadcrumb breadcrumb) {
try {
if (breadcrumb.payloadSize() > MAX_PAYLOAD_SIZE) {
Logger.warn("Dropping breadcrumb because payload exceeds 4KB limit");
return;
}
if (store.size() >= maxSize) {
// Remove oldest breadcrumb
store.poll();
}
store.add(breadcrumb);
} catch (IOException ex) {
Logger.warn("Dropping breadcrumb because it could not be serialized", ex);
}
}
}
13 changes: 13 additions & 0 deletions src/main/java/com/bugsnag/android/Bugsnag.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.bugsnag.android;

import android.content.Context;
import java.util.HashMap;

/**
* Static access to a Bugsnag Client, the easiest way to use Bugsnag in your Android app.
Expand Down Expand Up @@ -372,6 +373,18 @@ public static void leaveBreadcrumb(String message) {
getClient().leaveBreadcrumb(message);
}

/**
* Leave a "breadcrumb" log message representing an action or event which
* occurred in your app, to aid with debugging
*
* @param name A short label (max 32 chars)
* @param type A category for the breadcrumb
* @param metadata Additional diagnostic information about the app environment
*/
public static void leaveBreadcrumb(String name, BreadcrumbType type, HashMap<String, String> metadata) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should take Map, not HashMap.

getClient().leaveBreadcrumb(name, type, metadata);
}

/**
* Set the maximum number of breadcrumbs to keep and sent to Bugsnag.
* By default, we'll keep and send the 20 most recent breadcrumb log
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/bugsnag/android/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.support.annotation.Nullable;
import android.text.TextUtils;
import java.util.Locale;
import java.util.HashMap;

/**
* A Bugsnag Client instance allows you to use Bugsnag in your Android app.
Expand Down Expand Up @@ -536,6 +537,10 @@ public void leaveBreadcrumb(String breadcrumb) {
breadcrumbs.add(breadcrumb);
}

public void leaveBreadcrumb(String name, BreadcrumbType type, HashMap<String, String> metadata) {
breadcrumbs.add(name, type, metadata);
}

/**
* Set the maximum number of breadcrumbs to keep and sent to Bugsnag.
* By default, we'll keep and send the 20 most recent breadcrumb log
Expand Down Expand Up @@ -580,6 +585,11 @@ private void notify(final Error error, boolean blocking) {
return;
}

// Add a breadcrumb for this error occurring
HashMap<String, String> breadcrumbMetadata = new HashMap<String, String>();
breadcrumbMetadata.put("message", error.getExceptionMessage());
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Collections.singletonMap for a much more space-efficient implementation of a single-element Map

breadcrumbs.add(error.getExceptionName(), BreadcrumbType.ERROR, breadcrumbMetadata);

// Capture the state of the app and device and attach diagnostics to the error
error.setAppData(appData);
error.setDeviceData(deviceData);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/bugsnag/android/Error.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* @see BeforeNotify
*/
public class Error implements JsonStream.Streamable {
private static final String PAYLOAD_VERSION = "2";
private static final String PAYLOAD_VERSION = "3";

private final Configuration config;
private AppData appData;
Expand Down