Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
- Deprecate `sendDefaultPii` in favor of `dataCollection` ahead of its removal in 9.0 ([#6158](https://github.com/getsentry/sentry-java/pull/6158))
- Make the tombstone merge time threshold configurable via `SentryAndroidOptions.setTombstoneMergeTimeThresholdMillis` and the `io.sentry.tombstone.merge-time-threshold-millis` manifest option ([#6154](https://github.com/getsentry/sentry-java/pull/6154))

### Fixes

- Keep the videos of already captured session replay segments when the replay stops, so the final segments are no longer missing ([#6171](https://github.com/getsentry/sentry-java/pull/6171))

## 8.58.0

### Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ dependencies {
androidTestImplementation(libs.awaitility3.kotlin)
androidTestImplementation(libs.kotlin.test.junit)
androidTestImplementation(libs.leakcanary.instrumentation)
androidTestImplementation(libs.msgpack)
androidTestImplementation(libs.okhttp.mockwebserver)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
-dontwarn org.conscrypt.**
-dontwarn org.bouncycastle.**
-dontwarn org.openjsse.**
-dontwarn sun.nio.ch.DirectBuffer
-dontwarn org.opentest4j.AssertionFailedError
-dontwarn org.mockito.internal.**
-dontwarn org.jetbrains.annotations.**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package io.sentry.uitest.android

import androidx.lifecycle.Lifecycle
import androidx.test.core.app.launchActivity
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.RootMatchers.isDialog
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.sentry.Sentry
import io.sentry.android.core.R
import io.sentry.android.core.SentryAndroidOptions
import io.sentry.uitest.android.mockservers.REPLAY_VIDEO_PART
import io.sentry.uitest.android.mockservers.replayVideoParts
import java.io.File
import java.util.concurrent.TimeUnit.SECONDS
import kotlin.test.Test
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
import okhttp3.mockwebserver.MockResponse
import org.awaitility.kotlin.await
import org.junit.runner.RunWith

/**
* Reproduces a customer flow that combines Session Replay with the user feedback form: opening the
* form flushes the replay, and discarding the form stops it. Stopping deletes the replay cache
* directory, while the segments that the flush produced can still be waiting in the transport
* queue. Their videos are read from disk only when the envelope is written to the socket, so a
* deleted directory turns a queued segment into a segment without a video, which relay discards as
* `invalid_replay_video`.
*/
@RunWith(AndroidJUnit4::class)
class ReplayFeedbackDialogTest : BaseUiTest() {

@Test
fun discardedFeedbackKeepsVideoOfBufferedSegments() {
runDiscardedFeedbackFlow { it.sessionReplay.onErrorSampleRate = 1.0 }
}

@Test
fun discardedFeedbackKeepsVideoOfSessionSegments() {
runDiscardedFeedbackFlow { it.sessionReplay.sessionSampleRate = 1.0 }
}

private fun runDiscardedFeedbackFlow(configureReplay: (SentryAndroidOptions) -> Unit) {
initSentry { options ->
configureReplay(options)
// The transport is held open on purpose below; do not let it time out while it waits.
options.readTimeoutMillis = SECONDS.toMillis(TRANSPORT_HOLD_SECONDS + 10).toInt()
// What the customer does when the user discards the form.
options.feedbackOptions.onFormClose = Runnable {
Sentry.replay().stop()
Sentry.replay().startBuffering()
}
}

val scenario = launchActivity<EmptyActivity>()
scenario.moveToState(Lifecycle.State.RESUMED)
// A segment without frames produces no video at all, so let the recorder fill one first.
await.atMost(20, SECONDS).until { replayFrameCount() >= 2 }

// Hold the transport on an unrelated envelope so that the replay segments are still queued when
// the form is discarded. On a real device the same window is opened by a slow upload.
relay.addResponse { MockResponse().setHeadersDelay(TRANSPORT_HOLD_SECONDS, SECONDS) }
Sentry.captureMessage("occupies the transport thread")

// Opening the form flushes the replay (SentryUserFeedbackForm calls captureReplay).
scenario.onActivity { Sentry.feedback().show() }
onView(withId(R.id.sentry_dialog_user_feedback_layout))
.inRoot(isDialog())
.check(matches(isDisplayed()))

// Discarding the form runs onFormClose, which stops the replay.
onView(withId(R.id.sentry_dialog_user_feedback_btn_cancel)).inRoot(isDialog()).perform(click())

await.atMost(TRANSPORT_HOLD_SECONDS + 30, SECONDS).untilAsserted {
relay.assert {
val segments = peekEnvelopes { it.replayVideoParts() != null }
assertTrue(segments.isNotEmpty(), "No replay segment reached the server")
segments.forEach { envelope ->
val video = envelope.replayVideoParts()!![REPLAY_VIDEO_PART]
assertNotNull(
video,
"Segment ${envelope.header.eventId} was sent without its video, " +
"so relay discards it as invalid_replay_video",
)
assertTrue(video.isNotEmpty(), "Segment ${envelope.header.eventId} has an empty video")
}
}
}
}

/** Number of frame screenshots that the recorder has written to the replay cache. */
private fun replayFrameCount(): Int {
val cacheDirPath = Sentry.getCurrentScopes().options.cacheDirPath ?: return 0
return File(cacheDirPath)
.listFiles { file -> file.isDirectory && file.name.startsWith("replay_") }
.orEmpty()
.sumOf { replayDir ->
replayDir.listFiles { file -> file.name.endsWith(".jpg") }.orEmpty().size
}
}

companion object {
private const val TRANSPORT_HOLD_SECONDS = 10L
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ class RelayAsserter(private val unassertedEnvelopes: MutableList<RelayResponse>)
return unassertedEnvelopes.removeAt(relayResponseIndex)
}

/**
* Returns every envelope received so far that satisfies [filter], without consuming it. Use this
* while polling for envelopes that are still in flight: a consuming lookup would drop the
* envelopes that an earlier, failed attempt already found.
*/
fun peekEnvelopes(
filter: (envelope: SentryEnvelope) -> Boolean = { true }
): List<SentryEnvelope> = originalUnassertedEnvelopes.mapNotNull { it.envelope }.filter(filter)

/** Asserts no other envelopes were sent. */
fun assertNoOtherEnvelopes() {
if (unassertedEnvelopes.isNotEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.sentry.uitest.android.mockservers

import io.sentry.SentryEnvelope
import io.sentry.SentryItemType
import org.msgpack.core.MessagePack

/** Key of the video part inside a `replay_video` envelope item. */
const val REPLAY_VIDEO_PART = "replay_video"

/**
* Decodes the `replay_video` envelope item into its msgpack parts (`replay_event`,
* `replay_recording` and [REPLAY_VIDEO_PART]), or returns null when the envelope holds no such
* item.
*
* Relay discards a segment as `invalid_replay_video` when [REPLAY_VIDEO_PART] is absent or empty.
* The SDK writes that part lazily, on the transport thread, so only the bytes that reach the server
* show whether a segment is usable. A `SentryReplayEvent` seen in `beforeSendReplay` does not.
*/
fun SentryEnvelope.replayVideoParts(): Map<String, ByteArray>? {
val item = items.firstOrNull { it.header.type == SentryItemType.ReplayVideo } ?: return null
return unpackMsgpackMap(item.data)
}

private fun unpackMsgpackMap(bytes: ByteArray): Map<String, ByteArray> =
MessagePack.newDefaultUnpacker(bytes).use { unpacker ->
(0 until unpacker.unpackMapHeader()).associate {
val key = unpacker.unpackString()
key to unpacker.readPayload(unpacker.unpackBinaryHeader())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
* @param frameTimestamp the timestamp when the frame screenshot was taken
*/
internal fun addFrame(bitmap: Bitmap, frameTimestamp: Long, screen: String? = null) {
if (replayCacheDir == null || bitmap.isRecycled) {
if (replayCacheDir == null || bitmap.isRecycled || isClosed.get()) {
return
}
replayCacheDir?.mkdirs()
Expand Down Expand Up @@ -101,7 +101,19 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
*/
public fun addFrame(screenshot: File, frameTimestamp: Long, screen: String? = null) {
val frame = ReplayFrame(screenshot, frameTimestamp, screen)
framesLock.acquire().use { frames += frame }
val added =
framesLock.acquire().use {
if (isClosed.get()) {
false
} else {
frames += frame
true
}
}
if (!added) {
// the replay has stopped, so nothing will encode or delete this frame
deleteFile(screenshot)
}
}

/** Returns the timestamp of the first frame if available in a thread-safe manner. */
Expand Down Expand Up @@ -271,12 +283,21 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
return screen
}

/**
* Releases the encoder and deletes the frames and segment state of this cache. Segment videos are
* kept: a captured segment's video belongs to the send path.
*/
override fun close() {
try {
encoder?.release()
encoder = null
} finally {
isClosed.set(true)
framesLock.acquire().use {
frames.forEach { deleteFile(it.screenshot) }
frames.clear()
}
lock.acquire().use { replayCacheDir?.let { File(it, ONGOING_SEGMENT).delete() } }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import io.sentry.android.replay.ReplayLifecycleState.STOPPED
import io.sentry.android.replay.capture.BufferCaptureStrategy
import io.sentry.android.replay.capture.CaptureStrategy
import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
import io.sentry.android.replay.capture.ReplaySegmentHint
import io.sentry.android.replay.capture.SessionCaptureStrategy
import io.sentry.android.replay.gestures.GestureRecorder
import io.sentry.android.replay.gestures.TouchRecorderCallback
Expand Down Expand Up @@ -333,30 +334,25 @@ public class ReplayIntegration(
return
}

var activeStrategy: CaptureStrategy = strategy
capture(strategy) { newTimestamp ->
enqueueOnMainThread {
val latest = state.get()
// The flush completes asynchronously; ignore it if this replay was stopped, restarted,
// or handed to another strategy in the meantime.
if (
latest.matches(expectedGeneration, expectedReplayId) &&
latest.captureStrategy === activeStrategy
) {
activeStrategy.currentSegment++
activeStrategy.segmentTimestamp = newTimestamp
activeStrategy.isFlushed = true
}
}
}
activeStrategy = strategy.convert()
val replayId: SentryId? = activeStrategy.currentReplayId
// Convert before capturing, so the flush callback knows which strategy continues this replay.
val activeStrategy = strategy.convert()
state.set(
current.copy(
replayId = replayId ?: SentryId.EMPTY_ID,
replayId = activeStrategy.currentReplayId ?: SentryId.EMPTY_ID,
captureStrategy = activeStrategy,
)
)
capture(strategy) { newTimestamp ->
// Runs on the replay thread right after the flush segment was captured, before any segment
// work queued behind it (pause, stop, frames). `strategy.currentSegment` is exactly the id
// the flush segment used (the buffer flush does not increment its own counter), so deriving
// the next id from it, rather than incrementing activeStrategy's possibly-stale id, ensures
// no later segment reuses the flushed id even if a queued segment ran on `strategy` first.
// A restarted replay uses a new strategy instance, so this cannot touch it.
activeStrategy.currentSegment = strategy.currentSegment + 1
activeStrategy.segmentTimestamp = newTimestamp
activeStrategy.isFlushed = true
}
}

override fun getReplayId(): SentryId = state.get().replayId
Expand Down Expand Up @@ -701,7 +697,7 @@ public class ReplayIntegration(
)

if (segment is ReplaySegment.Created) {
val hint = HintUtils.createWithTypeCheckHint(PreviousReplayHint())
val hint = HintUtils.createWithTypeCheckHint(PreviousReplayHint(segment.replay.videoFile))
segment.capture(scopes, hint)
}
cleanupReplays(
Expand Down Expand Up @@ -757,7 +753,7 @@ public class ReplayIntegration(
isRecording && this.generation == generation && this.replayId == replayId
}

private class PreviousReplayHint : Backfillable {
private class PreviousReplayHint(videoFile: File?) : ReplaySegmentHint(videoFile), Backfillable {
override fun shouldEnrich(): Boolean = false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import io.sentry.android.replay.util.ReplayRunnable
import io.sentry.clientreport.DiscardReason.RATELIMIT_BACKOFF
import io.sentry.protocol.SentryId
import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.FileUtils
import java.io.File
import java.util.Date
import java.util.concurrent.ScheduledExecutorService
Expand Down Expand Up @@ -77,10 +76,11 @@ internal class BufferCaptureStrategy(
}

override fun stop() {
val replayCacheDir = cache?.replayCacheDir
replayExecutor.submit(
ReplayRunnable("$TAG.stop") {
FileUtils.deleteRecursively(replayCacheDir)
// Buffered segments were never captured, so nothing else will delete their videos.
bufferedSegments.forEach { deleteFile(it.replay.videoFile) }
bufferedSegments.clear()
currentSegment = -1
}
)
Expand Down Expand Up @@ -171,9 +171,16 @@ internal class BufferCaptureStrategy(
return this
}
// we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of
// execution
// execution, and the cache so that its frames and files keep a single owner
val captureStrategy =
SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, persistingExecutor)
SessionCaptureStrategy(
options,
scopes,
dateProvider,
replayExecutor,
persistingExecutor,
replayCacheProvider = cache?.let { current -> { _ -> current } },
)
captureStrategy.recorderConfig = recorderConfig
captureStrategy.start(
segmentId = currentSegment,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.sentry.rrweb.RRWebEvent
import io.sentry.rrweb.RRWebMetaEvent
import io.sentry.rrweb.RRWebOptionsEvent
import io.sentry.rrweb.RRWebVideoEvent
import io.sentry.util.HintUtils
import java.io.File
import java.util.Date
import java.util.Deque
Expand Down Expand Up @@ -273,7 +274,10 @@ internal interface CaptureStrategy {

data class Created(val replay: SentryReplayEvent, val recording: ReplayRecording) :
ReplaySegment() {
fun capture(scopes: IScopes?, hint: Hint = Hint()) {
fun capture(
scopes: IScopes?,
hint: Hint = HintUtils.createWithTypeCheckHint(ReplaySegmentHint(replay.videoFile)),
) {
scopes?.captureReplay(replay, hint.apply { replayRecording = recording })
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.sentry.android.replay.capture

import io.sentry.hints.DiscardNotification
import java.io.File

/**
* Hint for a captured replay segment. The send path owns the segment video: it deletes the video
* after reading it, and this hint deletes it when the envelope is dropped before that.
*/
internal open class ReplaySegmentHint(private val videoFile: File?) : DiscardNotification {
override fun markDiscarded() {
videoFile?.delete()
}
}
Loading
Loading