From 080877395d150ae6ba47f6f20619168c0c01e478 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 19 May 2020 12:09:28 +0200 Subject: [PATCH 1/3] enha: end session when user kills the App. --- .../src/main/AndroidManifest.xml | 3 + .../core/AndroidOptionsInitializer.java | 6 ++ .../io/sentry/android/core/SentryService.java | 77 +++++++++++++++++++ .../src/main/java/io/sentry/core/Hub.java | 4 +- .../main/java/io/sentry/core/HubAdapter.java | 4 +- .../src/main/java/io/sentry/core/IHub.java | 7 +- .../src/main/java/io/sentry/core/NoOpHub.java | 2 +- .../core/transport/AsyncConnection.java | 5 ++ 8 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java diff --git a/sentry-android-core/src/main/AndroidManifest.xml b/sentry-android-core/src/main/AndroidManifest.xml index facbc0d01..9001f0316 100644 --- a/sentry-android-core/src/main/AndroidManifest.xml +++ b/sentry-android-core/src/main/AndroidManifest.xml @@ -8,5 +8,8 @@ android:name=".SentryInitProvider" android:authorities="${applicationId}.SentryInitProvider" android:exported="false"/> + + diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index d9909bdf8..18dfab58c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -76,6 +76,12 @@ static void init( options.setSerializer(new AndroidSerializer(options.getLogger(), envelopeReader)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); + + try { + context.startService(SentryService.getIntent(context)); + } catch (IllegalStateException e) { + options.getLogger().log(SentryLevel.ERROR, e, "SentryService can't be started."); + } } private static void installDefaultIntegrations( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java new file mode 100644 index 000000000..d149eda37 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java @@ -0,0 +1,77 @@ +package io.sentry.android.core; + +import android.app.Service; +import android.content.Context; +import android.content.Intent; +import android.os.IBinder; +import androidx.annotation.NonNull; +import io.sentry.core.HubAdapter; +import io.sentry.core.hints.DiskFlushNotification; +import io.sentry.core.hints.Flushable; +import io.sentry.core.hints.SessionEnd; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +/** + * This is a best effort to end a session during onTaskRemoved (user swapped up the App. aka killed + * it). It needs to be public. + */ +@ApiStatus.Internal +public final class SentryService extends Service { + + @Override + public @Nullable IBinder onBind(Intent intent) { + return null; + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + super.onStartCommand(intent, flags, startId); + return START_NOT_STICKY; + } + + @Override + public void onTaskRemoved(Intent rootIntent) { + // TODO: get logger and flush timeout from options thru onStartCommand.intent + final TaskRemovedHint hint = new TaskRemovedHint(15000); + HubAdapter.getInstance().endSession(hint); + + hint.waitFlush(); + + super.onTaskRemoved(rootIntent); + } + + @NonNull + public static Intent getIntent(Context context) { + return new Intent(context, SentryService.class); + } + + private static final class TaskRemovedHint + implements DiskFlushNotification, Flushable, SessionEnd { + + private final CountDownLatch latch; + private final long flushTimeoutMillis; + + TaskRemovedHint(final long flushTimeoutMillis) { + this.flushTimeoutMillis = flushTimeoutMillis; + latch = new CountDownLatch(1); + } + + @Override + public boolean waitFlush() { + try { + return latch.await(flushTimeoutMillis, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return false; + } + + @Override + public void markFlushed() { + latch.countDown(); + } + } +} diff --git a/sentry-core/src/main/java/io/sentry/core/Hub.java b/sentry-core/src/main/java/io/sentry/core/Hub.java index 138fbc7c7..01d8afc1c 100644 --- a/sentry-core/src/main/java/io/sentry/core/Hub.java +++ b/sentry-core/src/main/java/io/sentry/core/Hub.java @@ -218,7 +218,7 @@ public void startSession() { } @Override - public void endSession() { + public void endSession(final @Nullable Object hint) { if (!isEnabled()) { options .getLogger() @@ -228,7 +228,7 @@ public void endSession() { if (item != null) { final Session previousSession = item.scope.endSession(); if (previousSession != null) { - item.client.captureSession(previousSession, new SessionEndHint()); + item.client.captureSession(previousSession, hint); } } else { options.getLogger().log(SentryLevel.FATAL, "Stack peek was null when endSession"); diff --git a/sentry-core/src/main/java/io/sentry/core/HubAdapter.java b/sentry-core/src/main/java/io/sentry/core/HubAdapter.java index 6bf16abc3..24c0049a4 100644 --- a/sentry-core/src/main/java/io/sentry/core/HubAdapter.java +++ b/sentry-core/src/main/java/io/sentry/core/HubAdapter.java @@ -48,8 +48,8 @@ public void startSession() { } @Override - public void endSession() { - Sentry.endSession(); + public void endSession(@Nullable Object hint) { + Sentry.getCurrentHub().endSession(hint); } @Override diff --git a/sentry-core/src/main/java/io/sentry/core/IHub.java b/sentry-core/src/main/java/io/sentry/core/IHub.java index 1b71b994b..ad14c2e50 100644 --- a/sentry-core/src/main/java/io/sentry/core/IHub.java +++ b/sentry-core/src/main/java/io/sentry/core/IHub.java @@ -1,5 +1,6 @@ package io.sentry.core; +import io.sentry.core.hints.SessionEndHint; import io.sentry.core.protocol.SentryId; import io.sentry.core.protocol.User; import java.util.List; @@ -96,7 +97,11 @@ default SentryId captureException(Throwable throwable) { void startSession(); /** Ends the current session */ - void endSession(); + default void endSession() { + endSession(new SessionEndHint()); + } + + void endSession(@Nullable Object hint); /** Flushes out the queue for up to timeout seconds and disable the Hub. */ void close(); diff --git a/sentry-core/src/main/java/io/sentry/core/NoOpHub.java b/sentry-core/src/main/java/io/sentry/core/NoOpHub.java index fb2b14bb0..cb68796c1 100644 --- a/sentry-core/src/main/java/io/sentry/core/NoOpHub.java +++ b/sentry-core/src/main/java/io/sentry/core/NoOpHub.java @@ -44,7 +44,7 @@ public SentryId captureException(Throwable throwable, @Nullable Object hint) { public void startSession() {} @Override - public void endSession() {} + public void endSession(@Nullable Object hint) {} @Override public void close() {} diff --git a/sentry-core/src/main/java/io/sentry/core/transport/AsyncConnection.java b/sentry-core/src/main/java/io/sentry/core/transport/AsyncConnection.java index ed7956ba9..eee2ff3a5 100644 --- a/sentry-core/src/main/java/io/sentry/core/transport/AsyncConnection.java +++ b/sentry-core/src/main/java/io/sentry/core/transport/AsyncConnection.java @@ -370,6 +370,11 @@ public void run() { sessionCache.store(envelope, hint); + if (hint instanceof DiskFlushNotification) { + ((DiskFlushNotification) hint).markFlushed(); + options.getLogger().log(SentryLevel.DEBUG, "Disk flush envelope fired."); + } + // we only flush a session update to the disk, but not to the network if (hint instanceof SessionUpdate) { options From 3f80265027f86ebe358f987ec46c4e4fe2d04ecf Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 19 May 2020 12:20:27 +0200 Subject: [PATCH 2/3] fix comments --- .../src/main/java/io/sentry/android/core/SentryService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java index d149eda37..27e723d7d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryService.java @@ -15,8 +15,8 @@ import org.jetbrains.annotations.Nullable; /** - * This is a best effort to end a session during onTaskRemoved (user swapped up the App. aka killed - * it). It needs to be public. + * This is a best effort to end a session during onTaskRemoved (user swiped up the App. aka killed + * it. It needs to be public. */ @ApiStatus.Internal public final class SentryService extends Service { @@ -29,6 +29,7 @@ public final class SentryService extends Service { @Override public int onStartCommand(Intent intent, int flags, int startId) { super.onStartCommand(intent, flags, startId); + // START_NOT_STICKY, we don't want to recreate the service nor the AppContext. return START_NOT_STICKY; } From 8fa9236d3b1a8f9ba9e6a316a741eab416638c20 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 19 May 2020 12:24:06 +0200 Subject: [PATCH 3/3] add comments --- .../java/io/sentry/android/core/AndroidOptionsInitializer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 18dfab58c..d08fed79d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -80,6 +80,8 @@ static void init( try { context.startService(SentryService.getIntent(context)); } catch (IllegalStateException e) { + // maybe we should make Throwable instead of to be safe than sorry + // services have different behaviours across OS versioms eg foreground etc options.getLogger().log(SentryLevel.ERROR, e, "SentryService can't be started."); } }