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 changing NDK Event's api key in OnErrorCallback #964

Merged
merged 7 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
* Increase breadcrumb time precision to milliseconds
[#954](https://github.com/bugsnag/bugsnag-android/pull/954)

* Support changing NDK Event's api key in OnErrorCallback
[#964](https://github.com/bugsnag/bugsnag-android/pull/964)

## 5.2.2 (2020-10-19)

### Bug fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ class FileStoreTest {
dir.mkdir()

val store = CustomFileStore(appContext, "", 1, null, delegate)
store.enqueueContentForDelivery("foo")
store.enqueueContentForDelivery("foo", "foo.json")

assertEquals("NDK Crash report copy", delegate.context)
assertEquals(File("/foo.json"), delegate.errorFile)
assertEquals(File("foo.json"), delegate.errorFile)
assertTrue(delegate.exception is FileNotFoundException)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ internal class ClientObservable : BaseObservable() {
fun postNdkInstall(conf: ImmutableConfig) {
notifyObservers(
StateEvent.Install(
conf.enabledErrorTypes.ndkCrashes, conf.appVersion,
conf.buildUuid, conf.releaseStage
conf.apiKey,
conf.enabledErrorTypes.ndkCrashes,
conf.appVersion,
conf.buildUuid,
conf.releaseStage
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,11 @@ private List<File> findLaunchCrashReports(Collection<File> storedFiles) {
String getFilename(Object object) {
String uuid = UUID.randomUUID().toString();
long now = System.currentTimeMillis();
return getFilename(object, uuid, now, storeDirectory);
return getFilename(object, uuid, null, now, storeDirectory);
}

String getFilename(Object object, String uuid, long timestamp, String storeDirectory) {
String getFilename(Object object, String uuid, String apiKey,
long timestamp, String storeDirectory) {
String suffix = "";

if (object instanceof Event) {
Expand All @@ -239,14 +240,21 @@ String getFilename(Object object, String uuid, long timestamp, String storeDirec
if (duration != null && isStartupCrash(duration.longValue())) {
suffix = STARTUP_CRASH;
}
return String.format(Locale.US, "%s%d_%s_%s%s.json",
storeDirectory, timestamp, event.getApiKey(), uuid, suffix);

apiKey = event.getApiKey();
} else { // generating a filename for an NDK event
suffix = "not-jvm";
return String.format(Locale.US, "%s%d_%s%s.json",
storeDirectory, timestamp, uuid, suffix);
if (apiKey.isEmpty()) {
apiKey = config.getApiKey();
}
}
return String.format(Locale.US, "%s%d_%s_%s%s.json",
storeDirectory, timestamp, apiKey, uuid, suffix);
}

String getNdkFilename(Object object, String apiKey) {
String uuid = UUID.randomUUID().toString();
long now = System.currentTimeMillis();
return getFilename(object, uuid, apiKey, now, storeDirectory);
}

boolean isStartupCrash(long durationMs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,10 @@ interface Delegate {
this.storeDirectory = path;
}

void enqueueContentForDelivery(String content) {
void enqueueContentForDelivery(String content, String filename) {
if (storeDirectory == null) {
return;
}
String filename = getFilename(content);
discardOldestFileIfNeeded();
lock.lock();
Writer out = null;
Expand All @@ -96,7 +95,7 @@ void enqueueContentForDelivery(String content) {
}
} catch (Exception exception) {
logger.w(String.format("Failed to close unsent payload writer (%s) ",
filename), exception);
filename), exception);
}
lock.unlock();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,12 @@ public static void registerSession(long startedAt, @Nullable String sessionId,
* should be discarded, based on configured release
* stages
* @param payloadBytes The raw JSON payload of the event
* @param apiKey The apiKey for the event
*/
@SuppressWarnings("unused")
public static void deliverReport(@Nullable byte[] releaseStageBytes,
@NonNull byte[] payloadBytes) {
@NonNull byte[] payloadBytes,
@NonNull String apiKey) {
if (payloadBytes == null) {
return;
}
Expand All @@ -311,11 +313,15 @@ public static void deliverReport(@Nullable byte[] releaseStageBytes,
? null
: new String(releaseStageBytes, UTF8Charset);
Client client = getClient();
ImmutableConfig config = client.getConfig();
if (releaseStage == null
|| releaseStage.length() == 0
|| client.getConfig().shouldNotifyForReleaseStage()) {
client.getEventStore().enqueueContentForDelivery(payload);
client.getEventStore().flushAsync();
|| config.shouldNotifyForReleaseStage()) {
EventStore eventStore = client.getEventStore();

String filename = eventStore.getNdkFilename(payload, apiKey);
eventStore.enqueueContentForDelivery(payload, filename);
eventStore.flushAsync();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.bugsnag.android

sealed class StateEvent {
class Install(
val apiKey: String,
val autoDetectNdkCrashes: Boolean,
val appVersion: String?,
val buildUuid: String?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class EventFilenameTest {

@Test
fun regularJvmEventName() {
val filename = eventStore.getFilename(event, "my-uuid-123", 1504255147933, "/errors/")
val filename = eventStore.getFilename(event, "my-uuid-123", null, 1504255147933, "/errors/")
assertEquals("/errors/1504255147933_0000111122223333aaaabbbbcccc9999_my-uuid-123.json", filename)
}

Expand All @@ -110,7 +110,7 @@ class EventFilenameTest {
@Test
fun startupCrashJvmEventName() {
`when`(app.duration).thenReturn(1000)
val filename = eventStore.getFilename(event, "my-uuid-123", 1504255147933, "/errors/")
val filename = eventStore.getFilename(event, "my-uuid-123", null, 1504255147933, "/errors/")
assertEquals("/errors/1504255147933_0000111122223333aaaabbbbcccc9999_my-uuid-123_startupcrash.json", filename)
}

Expand All @@ -120,14 +120,21 @@ class EventFilenameTest {
@Test
fun nonStartupCrashCrashJvmEventName() {
`when`(app.duration).thenReturn(10000)
val filename = eventStore.getFilename(event, "my-uuid-123", 1504255147933, "/errors/")
val filename = eventStore.getFilename(event, "my-uuid-123", null, 1504255147933, "/errors/")
assertEquals("/errors/1504255147933_0000111122223333aaaabbbbcccc9999_my-uuid-123.json", filename)
}

@Test
fun ndkEventName() {
val filename = eventStore.getFilename("{}", "my-uuid-123", 1504255147933, "/errors/")
assertEquals("/errors/1504255147933_my-uuid-123not-jvm.json", filename)
val filename = eventStore.getFilename("{}", "my-uuid-123",
"0000111122223333aaaabbbbcccc9999", 1504255147933, "/errors/")
assertEquals("/errors/1504255147933_0000111122223333aaaabbbbcccc9999_my-uuid-123not-jvm.json", filename)
}

@Test
fun ndkEventNameNoApiKey() {
val filename = eventStore.getFilename("{}", "my-uuid-123", "", 1504255147933, "/errors/")
assertEquals("/errors/1504255147933_5d1ec5bd39a74caa1267142706a7fb21_my-uuid-123not-jvm.json", filename)
}

@Test
Expand All @@ -151,4 +158,17 @@ class EventFilenameTest {
"_683c6b92-b325-4987-80ad-77086509ca1e.json")
assertEquals("0000111122223333aaaabbbbcccc9999", eventStore.getApiKeyFromFilename(file))
}

@Test
fun apiKeyFromLegacyNdkFilename() {
`when`(file.name).thenReturn("1603191800142_7e1041e0-7f37-4cfb-9d29-0aa6930bbb72not-jvm.json")
assertNull(eventStore.getApiKeyFromFilename(file))
}

@Test
fun apiKeyFromNdkFilename() {
`when`(file.name).thenReturn("1603191800142_5d1ec8bd39a74caa1267142706a7fb20_" +
"7e1041e0-7f37-4cfb-9d29-0aa6930bbb72not-jvm.json")
assertEquals("5d1ec8bd39a74caa1267142706a7fb20", eventStore.getApiKeyFromFilename(file))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ internal class NativeInterfaceApiTest {

@Test
fun deliverReport() {
NativeInterface.deliverReport(null, "{}".toByteArray())
verify(eventStore, times(1)).enqueueContentForDelivery("{}")
NativeInterface.deliverReport(null, "{}".toByteArray(), "")
verify(eventStore, times(1)).enqueueContentForDelivery(eq("{}"), any())
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion bugsnag-plugin-android-ndk/detekt-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<ManuallySuppressedIssues></ManuallySuppressedIssues>
<CurrentIssues>
<ID>ComplexMethod:NativeBridge.kt$NativeBridge$override fun update(observable: Observable, arg: Any?)</ID>
<ID>LongParameterList:NativeBridge.kt$NativeBridge$( reportingDirectory: String, autoDetectNdkCrashes: Boolean, apiLevel: Int, is32bit: Boolean, appVersion: String, buildUuid: String, releaseStage: String )</ID>
<ID>LongParameterList:NativeBridge.kt$NativeBridge$( apiKey: String, reportingDirectory: String, autoDetectNdkCrashes: Boolean, apiLevel: Int, is32bit: Boolean, appVersion: String, buildUuid: String, releaseStage: String )</ID>
<ID>MaxLineLength:NativeBridge.kt$NativeBridge$is AddBreadcrumb -&gt; addBreadcrumb(makeSafe(msg.message), makeSafe(msg.type.toString()), makeSafe(msg.timestamp), msg.metadata)</ID>
<ID>MaxLineLength:NativeBridge.kt$NativeBridge$is StartSession -&gt; startedSession(makeSafe(msg.id), makeSafe(msg.startedAt), msg.handledCount, msg.unhandledCount)</ID>
<ID>NestedBlockDepth:NativeBridge.kt$NativeBridge$private fun deliverPendingReports()</ID>
Expand Down
16 changes: 16 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/assets/include/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,22 @@ typedef struct {
extern "C" {
#endif

/* Accessors for event.api_key */

/**
* Retrieves the event api key
* @param event_ptr a pointer to the event supplied in an on_error callback
* @return the event api key, or NULL if this has not been set
*/
char *bugsnag_event_get_api_key(void *event_ptr);

/**
* Sets the event api key
* @param event_ptr a pointer to the event supplied in an on_error callback
* @param value the new event api key value, which cannot be NULL
*/
void bugsnag_event_set_api_key(void *event_ptr, char *value);

/* Accessors for event.context */

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class NativeBridge : Observer {
}

external fun install(
apiKey: String,
reportingDirectory: String,
autoDetectNdkCrashes: Boolean,
apiLevel: Int,
Expand Down Expand Up @@ -155,6 +156,7 @@ class NativeBridge : Observer {
} else {
val reportPath = reportDirectory + UUID.randomUUID().toString() + ".crash"
install(
makeSafe(arg.apiKey),
reportPath,
arg.autoDetectNdkCrashes,
Build.VERSION.SDK_INT,
Expand Down
16 changes: 11 additions & 5 deletions bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_disableCrashRep
}

JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
JNIEnv *env, jobject _this, jstring _event_path, jboolean auto_detect_ndk_crashes,
jint _api_level, jboolean is32bit) {
JNIEnv *env, jobject _this, jstring _api_key, jstring _event_path,
jboolean auto_detect_ndk_crashes, jint _api_level, jboolean is32bit) {
bsg_environment *bugsnag_env = calloc(1, sizeof(bsg_environment));
bsg_set_unwind_types((int)_api_level, (bool)is32bit,
&bugsnag_env->signal_unwind_style,
Expand All @@ -106,6 +106,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
bugsnag_env->report_header.version = BUGSNAG_EVENT_VERSION;
const char *event_path = (*env)->GetStringUTFChars(env, _event_path, 0);
sprintf(bugsnag_env->next_event_path, "%s", event_path);
(*env)->ReleaseStringUTFChars(env, _event_path, event_path);

if ((bool)auto_detect_ndk_crashes) {
bsg_handler_install_signal(bugsnag_env);
Expand All @@ -126,7 +127,10 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
sizeof(bugsnag_env->report_header.os_build));
}

(*env)->ReleaseStringUTFChars(env, _event_path, event_path);
const char *api_key = (*env)->GetStringUTFChars(env, _api_key, 0);
bsg_strncpy_safe(bugsnag_env->next_event.api_key, (char *) api_key, sizeof(bugsnag_env->next_event.api_key));
(*env)->ReleaseStringUTFChars(env, _api_key, api_key);

bsg_global_env = bugsnag_env;
BUGSNAG_LOG("Initialization complete!");
}
Expand All @@ -147,7 +151,7 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
(*env)->FindClass(env, "com/bugsnag/android/NativeInterface");
jmethodID jdeliver_method =
(*env)->GetStaticMethodID(env, interface_class, "deliverReport",
"([B[B)V");
"([B[BLjava/lang/String;)V");
size_t payload_length = bsg_strlen(payload);
jbyteArray jpayload = (*env)->NewByteArray(env, payload_length);
(*env)->SetByteArrayRegion(env, jpayload, 0, payload_length, (jbyte *)payload);
Expand All @@ -156,8 +160,10 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
jbyteArray jstage = (*env)->NewByteArray(env, stage_length);
(*env)->SetByteArrayRegion(env, jstage, 0, stage_length, (jbyte *)event->app.release_stage);

jstring japi_key = (*env)->NewStringUTF(env, event->api_key);
(*env)->CallStaticVoidMethod(env, interface_class, jdeliver_method,
jstage, jpayload);
jstage, jpayload, japi_key);
(*env)->DeleteLocalRef(env, japi_key);
(*env)->ReleaseByteArrayElements(env, jpayload, (jbyte *)payload, 0); // <-- frees payload
(*env)->ReleaseByteArrayElements(env, jstage, (jbyte *)event->app.release_stage, JNI_COMMIT);
(*env)->DeleteLocalRef(env, jpayload);
Expand Down
10 changes: 10 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/jni/event.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ void bugsnag_event_start_session(bugsnag_event *event, char *session_id,
event->unhandled_events = unhandled_count;
}

char *bugsnag_event_get_api_key(void *event_ptr) {
bugsnag_event *event = (bugsnag_event *) event_ptr;
return event->api_key;
}

void bugsnag_event_set_api_key(void *event_ptr, char *value) {
bugsnag_event *event = (bugsnag_event *) event_ptr;
bsg_strncpy_safe(event->api_key, value, sizeof(event->api_key));
}

char *bugsnag_event_get_context(void *event_ptr) {
bugsnag_event *event = (bugsnag_event *) event_ptr;
return event->context;
Expand Down
3 changes: 2 additions & 1 deletion bugsnag-plugin-android-ndk/src/main/jni/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
/**
* Version of the bugsnag_event struct. Serialized to report header.
*/
#define BUGSNAG_EVENT_VERSION 3
#define BUGSNAG_EVENT_VERSION 4


#ifdef __cplusplus
Expand Down Expand Up @@ -206,6 +206,7 @@ typedef struct {
int unhandled_events;
char grouping_hash[64];
bool unhandled;
char api_key[64];
} bugsnag_event;

void bugsnag_event_add_breadcrumb(bugsnag_event *event,
Expand Down
25 changes: 25 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/jni/utils/migrate.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,31 @@ typedef struct {
int unhandled_events;
} bugsnag_report_v2;

typedef struct {
bsg_notifier notifier;
bsg_app_info app;
bsg_device_info device;
bugsnag_user user;
bsg_error error;
bugsnag_metadata metadata;

int crumb_count;
// Breadcrumbs are a ring; the first index moves as the
// structure is filled and replaced.
int crumb_first_index;
bugsnag_breadcrumb breadcrumbs[BUGSNAG_CRUMBS_MAX];

char context[64];
bugsnag_severity severity;

char session_id[33];
char session_start[33];
int handled_events;
int unhandled_events;
char grouping_hash[64];
bool unhandled;
} bugsnag_report_v3;

#ifdef __cplusplus
}
#endif
Expand Down