Skip to content

Commit bd16527

Browse files
authored
fix(session-replay): Do not crash if navigation breadcrumb has no destination (#4185) (#4269)
* fix(session-replay): Do not crash if navigation breadcrumb has no destination (#4185) * Do not crash if navigation breadcrumb has not destination * Changelog * Fix test
1 parent b14c7bf commit bd16527

File tree

3 files changed

+34
-2
lines changed

3 files changed

+34
-2
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### Fixes
66

7+
- Session Replay: Fix crash when a navigation breadcrumb does not have "to" destination ([#4185](https://github.com/getsentry/sentry-java/pull/4185))
8+
- Session Replay: Cap video segment duration to maximum 5 minutes to prevent endless video encoding in background ([#4185](https://github.com/getsentry/sentry-java/pull/4185))
79
- Avoid logging an error when a float is passed in the manifest ([#4266](https://github.com/getsentry/sentry-java/pull/4266))
810

911
## 7.22.3

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ internal interface CaptureStrategy {
5858
companion object {
5959
private const val BREADCRUMB_START_OFFSET = 100L
6060

61+
// 5 minutes, otherwise relay will just drop it. Can prevent the case where the device
62+
// time is wrong and the segment is too long.
63+
private const val MAX_SEGMENT_DURATION = 1000L * 60 * 5
64+
6165
fun createSegment(
6266
hub: IHub?,
6367
options: SentryOptions,
@@ -76,7 +80,7 @@ internal interface CaptureStrategy {
7680
events: Deque<RRWebEvent>
7781
): ReplaySegment {
7882
val generatedVideo = cache?.createVideoOf(
79-
duration,
83+
minOf(duration, MAX_SEGMENT_DURATION),
8084
currentSegmentTimestamp.time,
8185
segmentId,
8286
height,
@@ -179,7 +183,9 @@ internal interface CaptureStrategy {
179183
recordingPayload += rrwebEvent
180184

181185
// fill in the urls array from navigation breadcrumbs
182-
if ((rrwebEvent as? RRWebBreadcrumbEvent)?.category == "navigation") {
186+
if ((rrwebEvent as? RRWebBreadcrumbEvent)?.category == "navigation" &&
187+
rrwebEvent.data?.getOrElse("to", { null }) is String
188+
) {
183189
urls.add(rrwebEvent.data!!["to"] as String)
184190
}
185191
}

sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,30 @@ class SessionCaptureStrategyTest {
336336
)
337337
}
338338

339+
@Test
340+
fun `does not throw when navigation destination is not a String`() {
341+
val now =
342+
System.currentTimeMillis() + (fixture.options.sessionReplay.sessionSegmentDuration * 5)
343+
val strategy = fixture.getSut(dateProvider = { now })
344+
strategy.start(fixture.recorderConfig)
345+
346+
fixture.scope.addBreadcrumb(Breadcrumb().apply { category = "navigation" })
347+
348+
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
349+
350+
verify(fixture.hub).captureReplay(
351+
check {
352+
assertNull(it.urls?.firstOrNull())
353+
},
354+
check {
355+
val breadcrumbEvents =
356+
it.replayRecording?.payload?.filterIsInstance<RRWebBreadcrumbEvent>()
357+
assertEquals("navigation", breadcrumbEvents?.first()?.category)
358+
assertNull(breadcrumbEvents?.first()?.data?.get("to"))
359+
}
360+
)
361+
}
362+
339363
@Test
340364
fun `sets screen from scope as replay url`() {
341365
fixture.scope.screen = "MainActivity"

0 commit comments

Comments
 (0)