diff --git a/CHANGELOG.md b/CHANGELOG.md index 896e619709..55500ff97a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Ensure potential callback exceptions are caught #2123 ([#2291](https://github.com/getsentry/sentry-java/pull/2291)) - Remove verbose FrameMetricsAggregator failure logging ([#2293](https://github.com/getsentry/sentry-java/pull/2293)) - Ignore broken regex for tracePropagationTarget ([#2288](https://github.com/getsentry/sentry-java/pull/2288)) +- Fix `SentryFileWriter`/`SentryFileOutputStream` append overwrites file contents ([#2304](https://github.com/getsentry/sentry-java/pull/2304)) ### Features diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt index d0dedbf823..4a1b836886 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt @@ -100,13 +100,13 @@ class SendCachedEnvelopeIntegrationTest { @Test fun `when synchronous send times out, continues the task on a background thread`() { - val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 50) - fixture.options.startupCrashFlushTimeoutMillis = 10 + val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 1000) + fixture.options.startupCrashFlushTimeoutMillis = 100 sut.register(fixture.hub, fixture.options) // first wait until synchronous send times out and check that the logger was hit in the catch block - await.atLeast(11, MILLISECONDS) + await.atLeast(500, MILLISECONDS) verify(fixture.logger).log( eq(DEBUG), eq("Synchronous send timed out, continuing in the background.") diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java index a48e10def0..791e2dcdc3 100644 --- a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java +++ b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java @@ -24,7 +24,7 @@ public final class SentryFileOutputStream extends FileOutputStream { private final @NotNull FileIOSpanManager spanManager; public SentryFileOutputStream(final @Nullable String name) throws FileNotFoundException { - this(name != null ? new File(name) : null, HubAdapter.getInstance()); + this(name != null ? new File(name) : null, false, HubAdapter.getInstance()); } public SentryFileOutputStream(final @Nullable String name, final boolean append) @@ -33,7 +33,7 @@ public SentryFileOutputStream(final @Nullable String name, final boolean append) } public SentryFileOutputStream(final @Nullable File file) throws FileNotFoundException { - this(file, HubAdapter.getInstance()); + this(file, false, HubAdapter.getInstance()); } public SentryFileOutputStream(final @Nullable File file, final boolean append) @@ -45,9 +45,9 @@ public SentryFileOutputStream(final @NotNull FileDescriptor fdObj) { this(init(fdObj, null, HubAdapter.getInstance()), fdObj); } - SentryFileOutputStream(final @Nullable File file, final @NotNull IHub hub) + SentryFileOutputStream(final @Nullable File file, final boolean append, final @NotNull IHub hub) throws FileNotFoundException { - this(init(file, false, null, hub)); + this(init(file, append, null, hub)); } private SentryFileOutputStream( @@ -72,7 +72,7 @@ private static FileOutputStreamInitData init( throws FileNotFoundException { final ISpan span = FileIOSpanManager.startSpan(hub, "file.write"); if (delegate == null) { - delegate = new FileOutputStream(file); + delegate = new FileOutputStream(file, append); } return new FileOutputStreamInitData( file, append, span, delegate, hub.getOptions().isSendDefaultPii()); diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java index 60afc8d11c..9588984612 100644 --- a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java +++ b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java @@ -30,7 +30,8 @@ public SentryFileWriter(final @NotNull FileDescriptor fd) { super(new SentryFileOutputStream(fd)); } - SentryFileWriter(final @NotNull File file, final @NotNull IHub hub) throws FileNotFoundException { - super(new SentryFileOutputStream(file, hub)); + SentryFileWriter(final @NotNull File file, final boolean append, final @NotNull IHub hub) + throws FileNotFoundException { + super(new SentryFileOutputStream(file, append, hub)); } } diff --git a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt index b455a6642f..6753715138 100644 --- a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt +++ b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt @@ -22,13 +22,14 @@ class SentryFileOutputStreamTest { internal fun getSut( tmpFile: File? = null, activeTransaction: Boolean = true, + append: Boolean = false ): SentryFileOutputStream { whenever(hub.options).thenReturn(SentryOptions()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) if (activeTransaction) { whenever(hub.span).thenReturn(sentryTracer) } - return SentryFileOutputStream(tmpFile, hub) + return SentryFileOutputStream(tmpFile, append, hub) } } diff --git a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt index f8f8fe1ea6..99d53ba865 100644 --- a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt +++ b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt @@ -21,13 +21,14 @@ class SentryFileWriterTest { internal fun getSut( tmpFile: File, activeTransaction: Boolean = true, + append: Boolean = false ): SentryFileWriter { whenever(hub.options).thenReturn(SentryOptions()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) if (activeTransaction) { whenever(hub.span).thenReturn(sentryTracer) } - return SentryFileWriter(tmpFile, hub) + return SentryFileWriter(tmpFile, append, hub) } } @@ -36,7 +37,7 @@ class SentryFileWriterTest { private val fixture = Fixture() - private val tmpFile: File get() = tmpDir.newFile("test.txt") + private val tmpFile: File by lazy { tmpDir.newFile("test.txt") } @Test fun `captures a span`() { @@ -53,4 +54,20 @@ class SentryFileWriterTest { assertEquals(fileIOSpan.isFinished, true) assertEquals(fileIOSpan.status, OK) } + + @Test + fun `append works`() { + val writer1 = fixture.getSut(tmpFile, append = true) + writer1.use { + it.append("test") + } + + // second writer should not overwrite the file contents, but append to the existing content + val writer2 = fixture.getSut(tmpFile, append = true) + writer2.use { + it.append("test2") + } + + assertEquals("testtest2", tmpFile.readText()) + } }