Skip to content

Commit

Permalink
Merge pull request #964 from bugsnag/PLAT-5012/ndk-event-api-key
Browse files Browse the repository at this point in the history
Support changing NDK Event's api key in OnErrorCallback
  • Loading branch information
fractalwrench authored Oct 28, 2020
2 parents a35475f + 349815a commit 747bfb5
Show file tree
Hide file tree
Showing 19 changed files with 274 additions and 44 deletions.
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
Loading

0 comments on commit 747bfb5

Please sign in to comment.