From 2a7b9693b059ec7852b73636c7ec517c9e5a382a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 22 Apr 2022 08:07:06 +0200 Subject: [PATCH 01/13] Change order of filtering mechanisms and add early return --- .../src/main/java/io/sentry/SentryClient.java | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index ee4f14a1dcf..c31295d74ab 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -81,6 +81,22 @@ private boolean shouldApplyScopeData( options.getLogger().log(SentryLevel.DEBUG, "Capturing event: %s", event.getEventId()); + if (event != null) { + if (event.getThrowable() != null + && options.containsIgnoredExceptionForType(event.getThrowable())) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Event was dropped as the exception %s is ignored", + event.getThrowable().getClass()); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Error); + return SentryId.EMPTY_ID; + } + } + if (shouldApplyScopeData(event, hint)) { // Event has already passed through here before it was cached // Going through again could be reading data that is no longer relevant @@ -95,6 +111,21 @@ private boolean shouldApplyScopeData( event = processEvent(event, hint, options.getEventProcessors()); + if (event != null) { + event = executeBeforeSend(event, hint); + + if (event == null) { + options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend"); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error); + } + } + + if (event == null) { + return SentryId.EMPTY_ID; + } + Session session = null; if (event != null) { @@ -115,30 +146,6 @@ private boolean shouldApplyScopeData( } } - if (event != null) { - if (event.getThrowable() != null - && options.containsIgnoredExceptionForType(event.getThrowable())) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Event was dropped as the exception %s is ignored", - event.getThrowable().getClass()); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Error); - return SentryId.EMPTY_ID; - } - event = executeBeforeSend(event, hint); - - if (event == null) { - options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error); - } - } - SentryId sentryId = SentryId.EMPTY_ID; if (event != null && event.getEventId() != null) { sentryId = event.getEventId(); From 45629e75041bd5c000b738a4dd2834154975e159 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 22 Apr 2022 09:01:20 +0200 Subject: [PATCH 02/13] Only send session update for dropped events if state changed --- .../src/main/java/io/sentry/SentryClient.java | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index c31295d74ab..09c4d4cb444 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -126,7 +126,10 @@ private boolean shouldApplyScopeData( return SentryId.EMPTY_ID; } - Session session = null; + @Nullable + Session oldSession = + scope != null ? scope.withSession((@Nullable Session session) -> {}) : null; + @Nullable Session session = null; if (event != null) { session = updateSessionData(event, hint, scope); @@ -146,6 +149,12 @@ private boolean shouldApplyScopeData( } } + boolean shouldSendSessionUpdate = shouldSendSessionUpdate(oldSession, session); + + if (event == null && !shouldSendSessionUpdate) { + return SentryId.EMPTY_ID; + } + SentryId sentryId = SentryId.EMPTY_ID; if (event != null && event.getEventId() != null) { sentryId = event.getEventId(); @@ -156,8 +165,9 @@ private boolean shouldApplyScopeData( scope != null && scope.getTransaction() != null ? scope.getTransaction().traceState() : null; - final SentryEnvelope envelope = - buildEnvelope(event, getAttachments(scope, hint), session, traceState, null); + boolean shouldSendAttachments = event != null; + List attachments = shouldSendAttachments ? getAttachments(scope, hint) : null; + final SentryEnvelope envelope = buildEnvelope(event, attachments, session, traceState, null); if (envelope != null) { transport.send(envelope, hint); @@ -172,6 +182,28 @@ private boolean shouldApplyScopeData( return sentryId; } + private boolean shouldSendSessionUpdate( + @Nullable Session oldSession, @Nullable Session newSession) { + if (newSession == null) { + return false; + } + + if (oldSession == null) { + return true; + } + + if (newSession.getStatus() == Session.State.Crashed + && oldSession.getStatus() != Session.State.Crashed) { + return true; + } + + if (newSession.errorCount() > 0 && oldSession.errorCount() <= 0) { + return true; + } + + return false; + } + private @Nullable List getAttachments( final @Nullable Scope scope, final @NotNull Map hint) { List attachments = null; From 9f6f2baafec1ff08b87052fd1a4312f3bf47e8cc Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 11:05:51 +0200 Subject: [PATCH 03/13] Extract variable for throwable --- sentry/src/main/java/io/sentry/SentryClient.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index c31295d74ab..f93da04f8ae 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -82,14 +82,14 @@ private boolean shouldApplyScopeData( options.getLogger().log(SentryLevel.DEBUG, "Capturing event: %s", event.getEventId()); if (event != null) { - if (event.getThrowable() != null - && options.containsIgnoredExceptionForType(event.getThrowable())) { + Throwable eventThrowable = event.getThrowable(); + if (eventThrowable != null && options.containsIgnoredExceptionForType(eventThrowable)) { options .getLogger() .log( SentryLevel.DEBUG, "Event was dropped as the exception %s is ignored", - event.getThrowable().getClass()); + eventThrowable.getClass()); options .getClientReportRecorder() .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Error); From c5eff6cf8e0fc81c3500d7b721930ee41fa6b4af Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 11:31:39 +0200 Subject: [PATCH 04/13] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e97eeca710..7251579738d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased * Fix: Allow disabling sending of client reports via Android Manifest and external options (#2007) +* Fix: Change order of event filtering mechanisms (#2001) ## 6.0.0-alpha.6 From cde0fce77de8c01a9daa0aff77cf20f11805149f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 11:34:13 +0200 Subject: [PATCH 05/13] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7251579738d..a9b39ce4f76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Fix: Allow disabling sending of client reports via Android Manifest and external options (#2007) * Fix: Change order of event filtering mechanisms (#2001) +* Fix: Only send session update for dropped events if state changed (#2002) ## 6.0.0-alpha.6 From c885d69f88790d0ab6504ad80d1d4338151f6fff Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 11:50:27 +0200 Subject: [PATCH 06/13] Rename method --- sentry/src/main/java/io/sentry/SentryClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index f2219d00289..a13814ddf78 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -149,7 +149,7 @@ private boolean shouldApplyScopeData( } } - boolean shouldSendSessionUpdate = shouldSendSessionUpdate(oldSession, session); + boolean shouldSendSessionUpdate = shouldSendSessionUpdateForDroppedEvent(oldSession, session); if (event == null && !shouldSendSessionUpdate) { return SentryId.EMPTY_ID; @@ -182,7 +182,7 @@ private boolean shouldApplyScopeData( return sentryId; } - private boolean shouldSendSessionUpdate( + private boolean shouldSendSessionUpdateForDroppedEvent( @Nullable Session oldSession, @Nullable Session newSession) { if (newSession == null) { return false; From 5832a4b6d1d1e1548e9baf59764961f38baa1207 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 12:21:12 +0200 Subject: [PATCH 07/13] Rename things --- .../src/main/java/io/sentry/SentryClient.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index a13814ddf78..e2283baa2b5 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -127,7 +127,7 @@ private boolean shouldApplyScopeData( } @Nullable - Session oldSession = + Session sessionBeforeUpdate = scope != null ? scope.withSession((@Nullable Session session) -> {}) : null; @Nullable Session session = null; @@ -149,7 +149,8 @@ private boolean shouldApplyScopeData( } } - boolean shouldSendSessionUpdate = shouldSendSessionUpdateForDroppedEvent(oldSession, session); + boolean shouldSendSessionUpdate = + shouldSendSessionUpdateForDroppedEvent(sessionBeforeUpdate, session); if (event == null && !shouldSendSessionUpdate) { return SentryId.EMPTY_ID; @@ -183,21 +184,25 @@ private boolean shouldApplyScopeData( } private boolean shouldSendSessionUpdateForDroppedEvent( - @Nullable Session oldSession, @Nullable Session newSession) { - if (newSession == null) { + @Nullable Session sessionBeforeUpdate, @Nullable Session sessionAfterUpdate) { + if (sessionAfterUpdate == null) { return false; } - if (oldSession == null) { + if (sessionBeforeUpdate == null) { return true; } - if (newSession.getStatus() == Session.State.Crashed - && oldSession.getStatus() != Session.State.Crashed) { + final boolean didSessionMoveToCrashedState = + sessionAfterUpdate.getStatus() == Session.State.Crashed + && sessionBeforeUpdate.getStatus() != Session.State.Crashed; + if (didSessionMoveToCrashedState) { return true; } - if (newSession.errorCount() > 0 && oldSession.errorCount() <= 0) { + final boolean didSessionMoveToErroredState = + sessionAfterUpdate.errorCount() > 0 && sessionBeforeUpdate.errorCount() <= 0; + if (didSessionMoveToErroredState) { return true; } From 1f51c55f3f20b5c99e2dca20aeaf8a54805c330e Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 12:22:15 +0200 Subject: [PATCH 08/13] Make var final --- sentry/src/main/java/io/sentry/SentryClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index f93da04f8ae..7b094f6d377 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -82,7 +82,7 @@ private boolean shouldApplyScopeData( options.getLogger().log(SentryLevel.DEBUG, "Capturing event: %s", event.getEventId()); if (event != null) { - Throwable eventThrowable = event.getThrowable(); + final Throwable eventThrowable = event.getThrowable(); if (eventThrowable != null && options.containsIgnoredExceptionForType(eventThrowable)) { options .getLogger() From 05aa2ad16585de7b81fd9f492dd4021c479fb633 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 15:29:41 +0200 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- sentry/src/main/java/io/sentry/SentryClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index ffd75a92e40..1afee617a71 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -166,7 +166,7 @@ private boolean shouldApplyScopeData( scope != null && scope.getTransaction() != null ? scope.getTransaction().traceState() : null; - boolean shouldSendAttachments = event != null; + final boolean shouldSendAttachments = event != null; List attachments = shouldSendAttachments ? getAttachments(scope, hint) : null; final SentryEnvelope envelope = buildEnvelope(event, attachments, session, traceState, null); From 89145b844d98281170812ff1b1d71875707502e8 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 26 Apr 2022 15:30:47 +0200 Subject: [PATCH 10/13] Update sentry/src/main/java/io/sentry/SentryClient.java Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- sentry/src/main/java/io/sentry/SentryClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 1afee617a71..2a195ae121b 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -149,7 +149,7 @@ private boolean shouldApplyScopeData( } } - boolean shouldSendSessionUpdate = + final boolean shouldSendSessionUpdate = shouldSendSessionUpdateForDroppedEvent(sessionBeforeUpdate, session); if (event == null && !shouldSendSessionUpdate) { From 8488bc8694868129ec7daad841034d160c45e7f7 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 29 Apr 2022 14:48:49 +0200 Subject: [PATCH 11/13] Add tests to verify order, session updates and sending --- .../test/java/io/sentry/SentryClientTest.kt | 342 +++++++++++++++++- 1 file changed, 339 insertions(+), 3 deletions(-) diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 59b4e78832c..853f4de4b7f 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -2,11 +2,15 @@ package io.sentry import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull +import com.nhaarman.mockitokotlin2.argumentCaptor import com.nhaarman.mockitokotlin2.check +import com.nhaarman.mockitokotlin2.doAnswer import com.nhaarman.mockitokotlin2.eq +import com.nhaarman.mockitokotlin2.inOrder import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.mockingDetails import com.nhaarman.mockitokotlin2.never +import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions import com.nhaarman.mockitokotlin2.whenever @@ -83,7 +87,10 @@ class SentryClientTest { var profilingTraceData = ProfilingTraceData(profilingTraceFile, sentryTracer) var profilingNonExistingTraceData = ProfilingTraceData(File("non_existent.trace"), sentryTracer) - fun getSut() = SentryClient(sentryOptions) + fun getSut(optionsCallback: ((SentryOptions) -> Unit)? = null): SentryClient { + optionsCallback?.invoke(sentryOptions) + return SentryClient(sentryOptions) + } } private val fixture = Fixture() @@ -1306,6 +1313,331 @@ class SentryClientTest { ) } + @Test + fun `capturing an error updates session and sends event + session`() { + val sut = fixture.getSut() + val scope = givenScopeWithStartedSession() + + sut.captureEvent(SentryEvent().apply { exceptions = createHandledException() }, scope) + + thenSessionIsErrored(scope) + thenEnvelopeIsSentWith(eventCount = 1, sessionCount = 1) + } + + @Test + fun `dropping a captured error from beforeSend has no effect on session and does not send anything`() { + val sut = fixture.getSut { options -> + options.beforeSend = SentryOptions.BeforeSendCallback { _, _ -> null } + } + val scope = givenScopeWithStartedSession() + + sut.captureEvent(SentryEvent().apply { exceptions = createHandledException() }, scope) + + thenSessionIsStillOK(scope) + thenNothingIsSent() + } + + @Test + fun `dropping a captured error from eventProcessor has no effect on session and does not send anything`() { + val sut = fixture.getSut { options -> + options.addEventProcessor(DropEverythingEventProcessor()) + } + val scope = givenScopeWithStartedSession() + + sut.captureEvent(SentryEvent().apply { exceptions = createHandledException() }, scope) + + thenSessionIsStillOK(scope) + thenNothingIsSent() + } + + @Test + fun `dropping a captured error via sampling updates the session and only sends the session for a new session`() { + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + } + val scope = givenScopeWithStartedSession() + + sut.captureEvent(SentryEvent().apply { exceptions = createHandledException() }, scope) + + thenSessionIsErrored(scope) + thenEnvelopeIsSentWith(eventCount = 0, sessionCount = 1) + } + + @Test + fun `dropping a captured error via sampling updates the session and does not send anything for an errored session`() { + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + } + val scope = givenScopeWithStartedSession(errored = true) + + sut.captureEvent(SentryEvent().apply { exceptions = createHandledException() }, scope) + + thenSessionIsErrored(scope) + thenNothingIsSent() + } + + @Test + fun `dropping a captured error via sampling updates the session and does not send anything for a crashed session`() { + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + } + val scope = givenScopeWithStartedSession(crashed = true) + + sut.captureEvent(SentryEvent().apply { exceptions = createHandledException() }, scope) + + thenSessionIsCrashed(scope) + thenNothingIsSent() + } + + @Test + fun `dropping a captured crash via sampling updates the session and only sends the session for a new session`() { + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + } + val scope = givenScopeWithStartedSession() + + sut.captureEvent(SentryEvent().apply { exceptions = createNonHandledException() }, scope) + + thenSessionIsCrashed(scope) + thenEnvelopeIsSentWith(eventCount = 0, sessionCount = 1) + } + + @Test + fun `dropping a captured crash via sampling updates the session and sends the session for an errored session`() { + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + } + val scope = givenScopeWithStartedSession(errored = true) + + sut.captureEvent(SentryEvent().apply { exceptions = createNonHandledException() }, scope) + + thenSessionIsCrashed(scope) + thenEnvelopeIsSentWith(eventCount = 0, sessionCount = 1) + } + + @Test + fun `dropping a captured crash via sampling updates the session and does not send anything for a crashed session`() { + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + } + val scope = givenScopeWithStartedSession(crashed = true) + + sut.captureEvent(SentryEvent().apply { exceptions = createNonHandledException() }, scope) + + thenSessionIsCrashed(scope) + thenNothingIsSent() + } + + @Test + fun `ignored exceptions are checked before other filter mechanisms`() { + val beforeSendMock = mock() + val scopedEventProcessorMock = mock() + val globalEventProcessorMock = mock() + + whenever(scopedEventProcessorMock.process(any(), anyOrNull())).thenReturn(null) + whenever(globalEventProcessorMock.process(any(), anyOrNull())).thenReturn(null) + whenever(beforeSendMock.execute(any(), anyOrNull())).thenReturn(null) + + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + options.addIgnoredExceptionForType(NegativeArraySizeException::class.java) + options.beforeSend = beforeSendMock + options.addEventProcessor(globalEventProcessorMock) + } + val scope = givenScopeWithStartedSession() + scope.addEventProcessor(scopedEventProcessorMock) + + sut.captureException(NegativeArraySizeException(), scope) + + verify(scopedEventProcessorMock, never()).process(any(), anyOrNull()) + verify(globalEventProcessorMock, never()).process(any(), anyOrNull()) + verify(beforeSendMock, never()).execute(any(), anyOrNull()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Error.category, 1)) + ) + } + + @Test + fun `sampling is last filter mechanism`() { + val beforeSendMock = mock() + val scopedEventProcessorMock = mock() + val globalEventProcessorMock = mock() + + whenever(scopedEventProcessorMock.process(any(), anyOrNull())).doAnswer { it.arguments.first() as SentryEvent } + whenever(globalEventProcessorMock.process(any(), anyOrNull())).doAnswer { it.arguments.first() as SentryEvent } + whenever(beforeSendMock.execute(any(), anyOrNull())).doAnswer { it.arguments.first() as SentryEvent } + + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + options.addIgnoredExceptionForType(NegativeArraySizeException::class.java) + options.beforeSend = beforeSendMock + options.addEventProcessor(globalEventProcessorMock) + } + val scope = givenScopeWithStartedSession() + scope.addEventProcessor(scopedEventProcessorMock) + + sut.captureException(IllegalStateException(), scope) + + val order = inOrder(scopedEventProcessorMock, globalEventProcessorMock, beforeSendMock) + + order.verify(scopedEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(globalEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(beforeSendMock, times(1)).execute(any(), anyOrNull()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Error.category, 1)) + ) + } + + @Test + fun `filter mechanism order check for beforeSend`() { + val beforeSendMock = mock() + val scopedEventProcessorMock = mock() + val globalEventProcessorMock = mock() + + whenever(scopedEventProcessorMock.process(any(), anyOrNull())).doAnswer { it.arguments.first() as SentryEvent } + whenever(globalEventProcessorMock.process(any(), anyOrNull())).doAnswer { it.arguments.first() as SentryEvent } + whenever(beforeSendMock.execute(any(), anyOrNull())).thenReturn(null) + + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + options.addIgnoredExceptionForType(NegativeArraySizeException::class.java) + options.beforeSend = beforeSendMock + options.addEventProcessor(globalEventProcessorMock) + } + val scope = givenScopeWithStartedSession() + scope.addEventProcessor(scopedEventProcessorMock) + + sut.captureException(IllegalStateException(), scope) + + val order = inOrder(scopedEventProcessorMock, globalEventProcessorMock, beforeSendMock) + + order.verify(scopedEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(globalEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(beforeSendMock, times(1)).execute(any(), anyOrNull()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Error.category, 1)) + ) + } + + @Test + fun `filter mechanism order check for scoped eventProcessor`() { + val beforeSendMock = mock() + val scopedEventProcessorMock = mock() + val globalEventProcessorMock = mock() + + whenever(scopedEventProcessorMock.process(any(), anyOrNull())).thenReturn(null) + whenever(globalEventProcessorMock.process(any(), anyOrNull())).thenReturn(null) + whenever(beforeSendMock.execute(any(), anyOrNull())).thenReturn(null) + + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + options.addIgnoredExceptionForType(NegativeArraySizeException::class.java) + options.beforeSend = beforeSendMock + options.addEventProcessor(globalEventProcessorMock) + } + val scope = givenScopeWithStartedSession() + scope.addEventProcessor(scopedEventProcessorMock) + + sut.captureException(IllegalStateException(), scope) + + val order = inOrder(scopedEventProcessorMock, globalEventProcessorMock, beforeSendMock) + + order.verify(scopedEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(globalEventProcessorMock, never()).process(any(), anyOrNull()) + order.verify(beforeSendMock, never()).execute(any(), anyOrNull()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Error.category, 1)) + ) + } + + @Test + fun `filter mechanism order check for global eventProcessor`() { + val beforeSendMock = mock() + val scopedEventProcessorMock = mock() + val globalEventProcessorMock = mock() + + whenever(scopedEventProcessorMock.process(any(), anyOrNull())).doAnswer { it.arguments.first() as SentryEvent } + whenever(globalEventProcessorMock.process(any(), anyOrNull())).thenReturn(null) + whenever(beforeSendMock.execute(any(), anyOrNull())).thenReturn(null) + + val sut = fixture.getSut { options -> + options.sampleRate = 0.000000000001 + options.addIgnoredExceptionForType(NegativeArraySizeException::class.java) + options.beforeSend = beforeSendMock + options.addEventProcessor(globalEventProcessorMock) + } + val scope = givenScopeWithStartedSession() + scope.addEventProcessor(scopedEventProcessorMock) + + sut.captureException(IllegalStateException(), scope) + + val order = inOrder(scopedEventProcessorMock, globalEventProcessorMock, beforeSendMock) + + order.verify(scopedEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(globalEventProcessorMock, times(1)).process(any(), anyOrNull()) + order.verify(beforeSendMock, never()).execute(any(), anyOrNull()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Error.category, 1)) + ) + } + + private fun givenScopeWithStartedSession(errored: Boolean = false, crashed: Boolean = false): Scope { + val scope = createScope(fixture.sentryOptions) + scope.startSession() + + if (errored) { + scope.withSession { it?.update(Session.State.Ok, "some-user-agent", true) } + } + + if (crashed) { + scope.withSession { it?.update(Session.State.Crashed, "some-user-agent", true) } + } + + return scope + } + + private fun thenNothingIsSent() { + verify(fixture.transport, never()).send(anyOrNull(), anyOrNull()) + } + + private fun thenEnvelopeIsSentWith(eventCount: Int, sessionCount: Int) { + val argumentCaptor = argumentCaptor() + verify(fixture.transport, times(1)).send(argumentCaptor.capture(), anyOrNull()) + + val envelope = argumentCaptor.firstValue + val envelopeItemTypes = envelope.items.map { it.header.type } + assertEquals(eventCount, envelopeItemTypes.count { it == SentryItemType.Event }) + assertEquals(sessionCount, envelopeItemTypes.count { it == SentryItemType.Session }) + } + + private fun thenSessionIsStillOK(scope: Scope) { + val sessionAfterCapture = scope.withSession { }!! + assertEquals(0, sessionAfterCapture.errorCount()) + assertEquals(Session.State.Ok, sessionAfterCapture.status) + } + + private fun thenSessionIsErrored(scope: Scope) { + val sessionAfterCapture = scope.withSession { }!! + assertTrue(sessionAfterCapture.errorCount() > 0) + assertEquals(Session.State.Ok, sessionAfterCapture.status) + } + + private fun thenSessionIsCrashed(scope: Scope) { + val sessionAfterCapture = scope.withSession { }!! + assertTrue(sessionAfterCapture.errorCount() > 0) + assertEquals(Session.State.Crashed, sessionAfterCapture.status) + } + class CustomBeforeSendCallback : SentryOptions.BeforeSendCallback { override fun execute(event: SentryEvent, hint: MutableMap?): SentryEvent? { hint?.remove(SENTRY_SCREENSHOT) @@ -1314,8 +1646,8 @@ class SentryClientTest { } } - private fun createScope(): Scope { - return Scope(SentryOptions()).apply { + private fun createScope(options: SentryOptions = SentryOptions()): Scope { + return Scope(options).apply { addBreadcrumb( Breadcrumb().apply { message = "message" @@ -1399,6 +1731,10 @@ class SentryClientTest { return listOf(exception) } + private fun createHandledException(): List { + return listOf(SentryException()) + } + private fun getEventFromData(data: ByteArray): SentryEvent { val inputStream = InputStreamReader(ByteArrayInputStream(data)) return fixture.sentryOptions.serializer.deserialize(inputStream, SentryEvent::class.java)!! From 8b0321a6a7d2cdd01affe4290b5b5ba628e3d04e Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 3 May 2022 10:44:23 +0200 Subject: [PATCH 12/13] Add debug log for dropped events ... ... where the session update is not sent because it does not change the health of the sesssion --- sentry/src/main/java/io/sentry/SentryClient.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 2a195ae121b..269574ca64c 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -153,6 +153,11 @@ private boolean shouldApplyScopeData( shouldSendSessionUpdateForDroppedEvent(sessionBeforeUpdate, session); if (event == null && !shouldSendSessionUpdate) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Not sending session update for dropped event as it did not cause the session health to change."); return SentryId.EMPTY_ID; } From 949d24826e468ea6e689c544b19fdff84874c92c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 3 May 2022 11:36:16 +0200 Subject: [PATCH 13/13] Fix merge mistake --- sentry/src/main/java/io/sentry/SentryClient.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 0bcdb5c96bd..269574ca64c 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -126,22 +126,6 @@ private boolean shouldApplyScopeData( return SentryId.EMPTY_ID; } - Session session = null; - if (event != null) { - event = executeBeforeSend(event, hint); - - if (event == null) { - options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error); - } - } - - if (event == null) { - return SentryId.EMPTY_ID; - } - @Nullable Session sessionBeforeUpdate = scope != null ? scope.withSession((@Nullable Session session) -> {}) : null;