Skip to content

Commit e2b7f44

Browse files
adinauergetsentry-botromtsnmarkushighasemdev
authored
Do not report cached events as lost (#4575)
* Do not report cached events as lost * E2E tests for OpenTelemetry based console sample (#4563) * e2e tests for console app * fix test failures by waiting for 10s after first try to find envelopes * add system-test-runner.py script to replace bash scripts for running e2e / system tests * use py script for ci, cleanup, makefile * Format code * remove bash scripts * install requests module * api * fix gh script * Implement E2E tests for OTel based console sample * fixes after merge * Format code * e2e tests for console app * Implement E2E tests for OTel based console sample * fixes after merge * Format code * api * Reduce scope forking when using OpenTelemetry (#4565) * Reduce scope forking in OpenTelemetry * Format code * api * changelog --------- Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> * SDKs send queue is no longer shutdown immediately on re-init (#4564) * Let queue drain on a restart * Format code * Format code * Update sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt * Let queue drain on a restart * Format code * Format code * Update sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt * adapt tests * changelog --------- Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> --------- Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> * release: 8.18.0 * ref(replay): Use main thread to schedule capture (#4542) * perf(connectivity): Cache network capabilities and status to reduce IPC calls (#4560) * fix(breadcrumbs): Deduplicate battery breadcrumbs (#4561) * fix(ci): remove obsolete NDK debug symbols (#4581) As they don't exist anymore and this is done within sentry-native directly: https://github.com/getsentry/sentry-native/pull/1327/files * fix(android): Remove unused method (#4585) * fix(android): Remove unused method * Update Changelog * Add rules file for documenting SDK offline behaviour (#4572) #skip-changelog ## 📜 Description <!--- Describe your changes in detail --> Add rules file for documenting SDK offline behaviour ## 💡 Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> Should help speed up AI reasoning about the SDK offline/retry behaviour. ## 💚 How did you test it? ## 📝 Checklist <!--- Put an `x` in the boxes that apply --> - [ ] I added tests to verify the changes. - [ ] No new PII added or SDK only sends newly added PII if `sendDefaultPII` is enabled. - [ ] I updated the docs if needed. - [ ] I updated the wizard if needed. - [ ] Review from the native team if needed. - [ ] No breaking change or entry added to the changelog. - [ ] No breaking change for hybrid SDKs or communicated to hybrid SDKs. ## 🔮 Next steps * perf(connectivity): Have only one NetworkCallback active at a time (#4562) * fix(scripts): update-gradle script set-version (#4591) * fix: sentry-android-ndk proguard rule keeps all native class (#4427) * fix: sentry-androi-ndk proguard rule keeps all native class * docs: update CHANGELOG * fix: update CHANGELOG * Update CHANGELOG.md * Update CHANGELOG.md --------- Co-authored-by: Markus Hintersteiner <m.hintersteiner@gmail.com> Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io> * refactor(lifecycle): Use single lifecycle observer (#4567) * perf(connectivity): Cache network capabilities and status to reduce IPC calls * changelog * Changelog * revert * fix(breadcrumbs): Deduplicate battery breadcrumbs * ref * Changelog * Fix test * perf(connectivity): Have only one NetworkCallback active at a time * Changelog * perf(integrations): Use single lifecycle observer * Add tests * Changelog * Fix tests * Improve callback handling and test visibility (#4593) * Null-check lifecycleObserver --------- Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io> * fix(sqlite): Fix abstract method error (#4597) * fix(sqlite): Fix abstract method error * Update CHANGELOG.md * Suppress metadata version checks * perf(integrations): Do not register for SystemEvents and NetworkCallbacks when launched with background importance (#4579) * fix(android): Ensure frame metrics listeners are registered/unregistered on the main thread (#4582) * fix(android): Ensure frame metrics listeners are registered/unregistered on the main thread * Fix race conditions * Update Changelog * Update CHANGELOG.md * Address PR feedback * perf(executor): Prewarm SentryExecutorService (#4606) * review feedback * changelog * pass through whether cache stored in AndroidEnvelopeCache + test * Format code --------- Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> Co-authored-by: getsentry-bot <bot@sentry.io> Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io> Co-authored-by: Ghasem Shirdel <shirdelghasem79@gmail.com> Co-authored-by: Markus Hintersteiner <m.hintersteiner@gmail.com>
1 parent f2b0ffb commit e2b7f44

File tree

11 files changed

+139
-44
lines changed

11 files changed

+139
-44
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
```
2727
- Fix abstract method error in `SentrySupportSQLiteDatabase` ([#4597](https://github.com/getsentry/sentry-java/pull/4597))
2828
- Ensure frame metrics listeners are registered/unregistered on the main thread ([#4582](https://github.com/getsentry/sentry-java/pull/4582))
29+
- Do not report cached events as lost ([#4575](https://github.com/getsentry/sentry-java/pull/4575))
30+
- Previously events were recorded as lost early despite being retried later through the cache
2931

3032
## 8.18.0
3133

‎sentry-android-core/api/sentry-android-core.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry
483483
public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z
484484
public static fun lastReportedAnr (Lio/sentry/SentryOptions;)Ljava/lang/Long;
485485
public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
486+
public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z
486487
}
487488

488489
public class io/sentry/android/core/performance/ActivityLifecycleCallbacksAdapter : android/app/Application$ActivityLifecycleCallbacks {

‎sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,19 @@ public AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options) {
4747
this.currentDateProvider = currentDateProvider;
4848
}
4949

50+
@SuppressWarnings("deprecation")
5051
@Override
5152
public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
52-
super.store(envelope, hint);
53+
storeInternalAndroid(envelope, hint);
54+
}
55+
56+
@Override
57+
public boolean storeEnvelope(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
58+
return storeInternalAndroid(envelope, hint);
59+
}
60+
61+
private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
62+
final boolean didStore = super.storeEnvelope(envelope, hint);
5363

5464
final SentryAndroidOptions options = (SentryAndroidOptions) this.options;
5565
final TimeSpan sdkInitTimeSpan = AppStartMetrics.getInstance().getSdkInitTimeSpan();
@@ -83,6 +93,7 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
8393

8494
writeLastReportedAnrMarker(timestamp);
8595
});
96+
return didStore;
8697
}
8798

8899
@TestOnly

‎sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package io.sentry.android.core.cache
22

3+
import io.sentry.ISerializer
34
import io.sentry.NoOpLogger
45
import io.sentry.SentryEnvelope
6+
import io.sentry.SentryOptions
57
import io.sentry.UncaughtExceptionHandlerIntegration.UncaughtExceptionHint
68
import io.sentry.android.core.AnrV2Integration.AnrV2Hint
79
import io.sentry.android.core.SentryAndroidOptions
@@ -18,7 +20,9 @@ import kotlin.test.assertFalse
1820
import kotlin.test.assertTrue
1921
import org.junit.Rule
2022
import org.junit.rules.TemporaryFolder
23+
import org.mockito.kotlin.any
2124
import org.mockito.kotlin.mock
25+
import org.mockito.kotlin.same
2226
import org.mockito.kotlin.whenever
2327

2428
class AndroidEnvelopeCacheTest {
@@ -35,8 +39,10 @@ class AndroidEnvelopeCacheTest {
3539
dir: TemporaryFolder,
3640
appStartMillis: Long? = null,
3741
currentTimeMillis: Long? = null,
42+
optionsCallback: ((SentryOptions) -> Unit)? = null,
3843
): AndroidEnvelopeCache {
3944
options.cacheDirPath = dir.newFolder("sentry-cache").absolutePath
45+
optionsCallback?.invoke(options)
4046
val outboxDir = File(options.outboxPath!!)
4147
outboxDir.mkdirs()
4248

@@ -82,7 +88,7 @@ class AndroidEnvelopeCacheTest {
8288
val cache = fixture.getSut(tmpDir)
8389

8490
val hints = HintUtils.createWithTypeCheckHint(UncaughtHint())
85-
cache.store(fixture.envelope, hints)
91+
cache.storeEnvelope(fixture.envelope, hints)
8692

8793
assertFalse(fixture.startupCrashMarkerFile.exists())
8894
}
@@ -92,7 +98,7 @@ class AndroidEnvelopeCacheTest {
9298
val cache = fixture.getSut(dir = tmpDir, appStartMillis = 1000L, currentTimeMillis = 5000L)
9399

94100
val hints = HintUtils.createWithTypeCheckHint(UncaughtHint())
95-
cache.store(fixture.envelope, hints)
101+
cache.storeEnvelope(fixture.envelope, hints)
96102

97103
assertFalse(fixture.startupCrashMarkerFile.exists())
98104
}
@@ -104,7 +110,7 @@ class AndroidEnvelopeCacheTest {
104110
fixture.options.cacheDirPath = null
105111

106112
val hints = HintUtils.createWithTypeCheckHint(UncaughtHint())
107-
cache.store(fixture.envelope, hints)
113+
cache.storeEnvelope(fixture.envelope, hints)
108114

109115
assertFalse(fixture.startupCrashMarkerFile.exists())
110116
}
@@ -114,7 +120,7 @@ class AndroidEnvelopeCacheTest {
114120
val cache = fixture.getSut(dir = tmpDir, appStartMillis = 1000L, currentTimeMillis = 2000L)
115121

116122
val hints = HintUtils.createWithTypeCheckHint(UncaughtHint())
117-
cache.store(fixture.envelope, hints)
123+
cache.storeEnvelope(fixture.envelope, hints)
118124

119125
assertTrue(fixture.startupCrashMarkerFile.exists())
120126
}
@@ -138,7 +144,7 @@ class AndroidEnvelopeCacheTest {
138144
HintUtils.createWithTypeCheckHint(
139145
AnrV2Hint(0, NoOpLogger.getInstance(), 12345678L, false, false)
140146
)
141-
cache.store(fixture.envelope, hints)
147+
cache.storeEnvelope(fixture.envelope, hints)
142148

143149
assertFalse(fixture.lastReportedAnrFile.exists())
144150
}
@@ -151,7 +157,7 @@ class AndroidEnvelopeCacheTest {
151157
HintUtils.createWithTypeCheckHint(
152158
AnrV2Hint(0, NoOpLogger.getInstance(), 12345678L, false, false)
153159
)
154-
cache.store(fixture.envelope, hints)
160+
cache.storeEnvelope(fixture.envelope, hints)
155161

156162
assertTrue(fixture.lastReportedAnrFile.exists())
157163
assertEquals("12345678", fixture.lastReportedAnrFile.readText())
@@ -189,5 +195,17 @@ class AndroidEnvelopeCacheTest {
189195
assertEquals(87654321L, lastReportedAnr)
190196
}
191197

198+
@Test
199+
fun `returns false if storing fails`() {
200+
val serializer = mock<ISerializer>()
201+
val cache = fixture.getSut(tmpDir) { options -> options.setSerializer(serializer) }
202+
whenever(serializer.serialize(same(fixture.envelope), any()))
203+
.thenThrow(RuntimeException("forced ex"))
204+
val hints = HintUtils.createWithTypeCheckHint(UncaughtHint())
205+
206+
val didStore = cache.storeEnvelope(fixture.envelope, hints)
207+
assertFalse(didStore)
208+
}
209+
192210
internal class UncaughtHint : UncaughtExceptionHint(0, NoOpLogger.getInstance())
193211
}

‎sentry/api/sentry.api‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4354,13 +4354,15 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache {
43544354
public static fun getPreviousSessionFile (Ljava/lang/String;)Ljava/io/File;
43554355
public fun iterator ()Ljava/util/Iterator;
43564356
public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
4357+
public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z
43574358
public fun waitPreviousSessionFlush ()Z
43584359
}
43594360

43604361
public abstract interface class io/sentry/cache/IEnvelopeCache : java/lang/Iterable {
43614362
public abstract fun discard (Lio/sentry/SentryEnvelope;)V
43624363
public fun store (Lio/sentry/SentryEnvelope;)V
43634364
public abstract fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
4365+
public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z
43644366
}
43654367

43664368
public final class io/sentry/cache/PersistingOptionsObserver : io/sentry/IOptionsObserver {
@@ -6663,6 +6665,7 @@ public final class io/sentry/transport/NoOpEnvelopeCache : io/sentry/cache/IEnve
66636665
public static fun getInstance ()Lio/sentry/transport/NoOpEnvelopeCache;
66646666
public fun iterator ()Ljava/util/Iterator;
66656667
public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
6668+
public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z
66666669
}
66676670

66686671
public final class io/sentry/transport/NoOpTransport : io/sentry/transport/ITransport {

‎sentry/src/main/java/io/sentry/cache/EnvelopeCache.java‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,18 @@ public EnvelopeCache(
9393
previousSessionLatch = new CountDownLatch(1);
9494
}
9595

96+
@SuppressWarnings("deprecation")
9697
@Override
9798
public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) {
99+
storeInternal(envelope, hint);
100+
}
101+
102+
@Override
103+
public boolean storeEnvelope(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) {
104+
return storeInternal(envelope, hint);
105+
}
106+
107+
private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) {
98108
Objects.requireNonNull(envelope, "Envelope is required.");
99109

100110
rotateCacheIfNeeded(allEnvelopeFiles());
@@ -171,19 +181,20 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi
171181
WARNING,
172182
"Not adding Envelope to offline storage because it already exists: %s",
173183
envelopeFile.getAbsolutePath());
174-
return;
184+
return true;
175185
} else {
176186
options
177187
.getLogger()
178188
.log(DEBUG, "Adding Envelope to offline storage: %s", envelopeFile.getAbsolutePath());
179189
}
180190

181-
writeEnvelopeToDisk(envelopeFile, envelope);
191+
final boolean didWriteToDisk = writeEnvelopeToDisk(envelopeFile, envelope);
182192

183193
// write file to the disk when its about to crash so crashedLastRun can be marked on restart
184194
if (HintUtils.hasType(hint, UncaughtExceptionHandlerIntegration.UncaughtExceptionHint.class)) {
185195
writeCrashMarkerFile();
186196
}
197+
return didWriteToDisk;
187198
}
188199

189200
/**
@@ -295,7 +306,7 @@ private void updateCurrentSession(
295306
}
296307
}
297308

298-
private void writeEnvelopeToDisk(
309+
private boolean writeEnvelopeToDisk(
299310
final @NotNull File file, final @NotNull SentryEnvelope envelope) {
300311
if (file.exists()) {
301312
options
@@ -312,7 +323,9 @@ private void writeEnvelopeToDisk(
312323
options
313324
.getLogger()
314325
.log(ERROR, e, "Error writing Envelope %s to offline storage", file.getAbsolutePath());
326+
return false;
315327
}
328+
return true;
316329
}
317330

318331
private void writeSessionToDisk(final @NotNull File file, final @NotNull Session session) {

‎sentry/src/main/java/io/sentry/cache/IEnvelopeCache.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,17 @@
66

77
public interface IEnvelopeCache extends Iterable<SentryEnvelope> {
88

9+
@Deprecated
910
void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint);
1011

12+
default boolean storeEnvelope(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
13+
store(envelope, hint);
14+
return true;
15+
}
16+
17+
@Deprecated
1118
default void store(@NotNull SentryEnvelope envelope) {
12-
store(envelope, new Hint());
19+
storeEnvelope(envelope, new Hint());
1320
}
1421

1522
void discard(@NotNull SentryEnvelope envelope);

‎sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ private static QueuedThreadPoolExecutor initExecutor(
139139
final EnvelopeSender envelopeSender = (EnvelopeSender) r;
140140

141141
if (!HintUtils.hasType(envelopeSender.hint, Cached.class)) {
142-
envelopeCache.store(envelopeSender.envelope, envelopeSender.hint);
142+
envelopeCache.storeEnvelope(envelopeSender.envelope, envelopeSender.hint);
143143
}
144144

145145
markHintWhenSendingFailed(envelopeSender.hint, true);
@@ -271,7 +271,7 @@ public void run() {
271271
TransportResult result = this.failedResult;
272272

273273
envelope.getHeader().setSentAt(null);
274-
envelopeCache.store(envelope, hint);
274+
boolean cached = envelopeCache.storeEnvelope(envelope, hint);
275275

276276
HintUtils.runIfHasType(
277277
hint,
@@ -311,14 +311,17 @@ public void run() {
311311

312312
// ignore e.g. 429 as we're not the ones actively dropping
313313
if (result.getResponseCode() >= 400 && result.getResponseCode() != 429) {
314-
HintUtils.runIfDoesNotHaveType(
315-
hint,
316-
Retryable.class,
317-
(hint) -> {
318-
options
319-
.getClientReportRecorder()
320-
.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
321-
});
314+
if (!cached) {
315+
HintUtils.runIfDoesNotHaveType(
316+
hint,
317+
Retryable.class,
318+
(hint) -> {
319+
options
320+
.getClientReportRecorder()
321+
.recordLostEnvelope(
322+
DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
323+
});
324+
}
322325
}
323326

324327
throw new IllegalStateException(message);
@@ -332,10 +335,12 @@ public void run() {
332335
retryable.setRetry(true);
333336
},
334337
(hint, clazz) -> {
335-
LogUtils.logNotInstanceOf(clazz, hint, options.getLogger());
336-
options
337-
.getClientReportRecorder()
338-
.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
338+
if (!cached) {
339+
LogUtils.logNotInstanceOf(clazz, hint, options.getLogger());
340+
options
341+
.getClientReportRecorder()
342+
.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
343+
}
339344
});
340345
throw new IllegalStateException("Sending the event failed.", e);
341346
}
@@ -348,10 +353,12 @@ public void run() {
348353
retryable.setRetry(true);
349354
},
350355
(hint, clazz) -> {
351-
LogUtils.logNotInstanceOf(clazz, hint, options.getLogger());
352-
options
353-
.getClientReportRecorder()
354-
.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelope);
356+
if (!cached) {
357+
LogUtils.logNotInstanceOf(clazz, hint, options.getLogger());
358+
options
359+
.getClientReportRecorder()
360+
.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelope);
361+
}
355362
});
356363
}
357364
return result;

‎sentry/src/main/java/io/sentry/transport/NoOpEnvelopeCache.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,15 @@ public static NoOpEnvelopeCache getInstance() {
1414
return instance;
1515
}
1616

17+
@SuppressWarnings("deprecation")
1718
@Override
1819
public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {}
1920

21+
@Override
22+
public boolean storeEnvelope(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
23+
return false;
24+
}
25+
2026
@Override
2127
public void discard(@NotNull SentryEnvelope envelope) {}
2228

0 commit comments

Comments
 (0)