diff --git a/src/Sentry/FileAttachmentContent.cs b/src/Sentry/FileAttachmentContent.cs index ed3bb4203b..9fe9fe6a5d 100644 --- a/src/Sentry/FileAttachmentContent.cs +++ b/src/Sentry/FileAttachmentContent.cs @@ -9,6 +9,8 @@ public class FileAttachmentContent : IAttachmentContent { private readonly bool _readFileAsynchronously; + private readonly bool _deleteOnClose; + /// /// The path to the file to attach. /// @@ -18,7 +20,16 @@ public class FileAttachmentContent : IAttachmentContent /// Creates a new instance of . /// /// The path to the file to attach. - public FileAttachmentContent(string filePath) : this(filePath, true) + public FileAttachmentContent(string filePath) : this(filePath, true, false) + { + } + + /// + /// Creates a new instance of . + /// + /// The path to the file to attach. + /// Whether to use async file I/O to read the file. + public FileAttachmentContent(string filePath, bool readFileAsynchronously) : this(filePath, readFileAsynchronously, false) { } @@ -27,18 +38,35 @@ public FileAttachmentContent(string filePath) : this(filePath, true) /// /// The path to the file to attach. /// Whether to use async file I/O to read the file. - public FileAttachmentContent(string filePath, bool readFileAsynchronously) + /// Whether to delete the file when it closed. + public FileAttachmentContent(string filePath, bool readFileAsynchronously, bool deleteOnClose) { FilePath = filePath; _readFileAsynchronously = readFileAsynchronously; + _deleteOnClose = deleteOnClose; } /// - public Stream GetStream() => new FileStream( - FilePath, - FileMode.Open, - FileAccess.Read, - FileShare.ReadWrite, - bufferSize: 4096, - useAsync: _readFileAsynchronously); + public Stream GetStream() + { + var options = FileOptions.None; + + if (_readFileAsynchronously) + { + options |= FileOptions.Asynchronous; + } + + if (_deleteOnClose) + { + options |= FileOptions.DeleteOnClose; + } + + return new FileStream( + FilePath, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite, + bufferSize: 4096, + options); + } } diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index d235bcc2a6..ab97ab60ae 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -759,7 +759,7 @@ internal void CaptureHeapDump(string dumpFile) Level = _options.HeapDumpOptions?.Level ?? SentryLevel.Warning, }; var hint = new SentryHint(_options); - hint.AddAttachment(dumpFile); + hint.AddAttachment(dumpFile, AttachmentType.HeapDump); CaptureEvent(evt, CurrentScope, hint); } catch (Exception e) diff --git a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs index 3c4ad20ae8..ca473789d2 100644 --- a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs +++ b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs @@ -342,6 +342,7 @@ internal static EnvelopeItem FromAttachment(SentryAttachment attachment, Stream AttachmentType.UnrealContext => "unreal.context", AttachmentType.UnrealLogs => "unreal.logs", AttachmentType.ViewHierarchy => "event.view_hierarchy", + AttachmentType.HeapDump => "event.heapdump", _ => "event.attachment" }; diff --git a/src/Sentry/SentryAttachment.cs b/src/Sentry/SentryAttachment.cs index c57dc96ed4..62e347a29b 100644 --- a/src/Sentry/SentryAttachment.cs +++ b/src/Sentry/SentryAttachment.cs @@ -36,7 +36,14 @@ public enum AttachmentType /// /// A JSON attachment containing the View Hierarchy /// - ViewHierarchy + ViewHierarchy, + + /// + /// A .gcdump file captured when a configured memory threshold is exceeded. + /// Used internally to allow the SDK to clean up the file from disk if it can't be sent to Sentry + /// (e.g. because it exceeds the attachment size limit). + /// + HeapDump } /// diff --git a/src/Sentry/SentryHint.cs b/src/Sentry/SentryHint.cs index b1b80356a2..cc126b3bda 100644 --- a/src/Sentry/SentryHint.cs +++ b/src/Sentry/SentryHint.cs @@ -71,10 +71,12 @@ public void AddAttachment( { if (_options is not null) { + var deleteOnClose = type == AttachmentType.HeapDump; + _attachments.Add( new SentryAttachment( type, - new FileAttachmentContent(filePath, _options.UseAsyncFileIO), + new FileAttachmentContent(filePath, _options.UseAsyncFileIO, deleteOnClose), Path.GetFileName(filePath), contentType)); } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt index 1b849bc057..c132faa2b7 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt @@ -9,6 +9,7 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, ViewHierarchy = 5, + HeapDump = 6, } public class BaggageHeader { @@ -109,6 +110,7 @@ namespace Sentry { public FileAttachmentContent(string filePath) { } public FileAttachmentContent(string filePath, bool readFileAsynchronously) { } + public FileAttachmentContent(string filePath, bool readFileAsynchronously, bool deleteOnClose) { } public System.IO.Stream GetStream() { } } public static class HasExtraExtensions diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index 1b849bc057..c132faa2b7 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -9,6 +9,7 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, ViewHierarchy = 5, + HeapDump = 6, } public class BaggageHeader { @@ -109,6 +110,7 @@ namespace Sentry { public FileAttachmentContent(string filePath) { } public FileAttachmentContent(string filePath, bool readFileAsynchronously) { } + public FileAttachmentContent(string filePath, bool readFileAsynchronously, bool deleteOnClose) { } public System.IO.Stream GetStream() { } } public static class HasExtraExtensions diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt index 1b849bc057..c132faa2b7 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt @@ -9,6 +9,7 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, ViewHierarchy = 5, + HeapDump = 6, } public class BaggageHeader { @@ -109,6 +110,7 @@ namespace Sentry { public FileAttachmentContent(string filePath) { } public FileAttachmentContent(string filePath, bool readFileAsynchronously) { } + public FileAttachmentContent(string filePath, bool readFileAsynchronously, bool deleteOnClose) { } public System.IO.Stream GetStream() { } } public static class HasExtraExtensions diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index 2cf30af33f..efd3961e5d 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt @@ -9,6 +9,7 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, ViewHierarchy = 5, + HeapDump = 6, } public class BaggageHeader { @@ -99,6 +100,7 @@ namespace Sentry { public FileAttachmentContent(string filePath) { } public FileAttachmentContent(string filePath, bool readFileAsynchronously) { } + public FileAttachmentContent(string filePath, bool readFileAsynchronously, bool deleteOnClose) { } public System.IO.Stream GetStream() { } } public static class HasExtraExtensions diff --git a/test/Sentry.Tests/HintTests.cs b/test/Sentry.Tests/HintTests.cs index bbcd20755f..838bdd2773 100644 --- a/test/Sentry.Tests/HintTests.cs +++ b/test/Sentry.Tests/HintTests.cs @@ -175,5 +175,48 @@ public void WithAttachments_WithICollection_ReturnsHintWithAttachments() hint.Attachments.Should().Contain(attachment2); } + [Fact] + public void AddAttachment_HeapDump_DeletesFileWhenStreamIsClosed() + { + // Arrange + var attachmentPath = Path.Combine(_testDirectory, "dump.gcdump"); + File.WriteAllText(attachmentPath, "fake heap dump"); + + var hint = new SentryHint(new SentryOptions()); + + // Act + hint.AddAttachment(attachmentPath, AttachmentType.HeapDump); + + // Assert + var attachment = Assert.Single(hint.Attachments); + using (attachment.Content.GetStream()) + { + File.Exists(attachmentPath).Should().BeTrue("the dump must survive while it is being read"); + } + + File.Exists(attachmentPath).Should().BeFalse("heap dumps are deleted once the stream is closed"); + } + + [Fact] + public void AddAttachment_NonHeapDump_LeavesFileInPlaceWhenStreamIsClosed() + { + // Arrange + var attachmentPath = Path.Combine(_testDirectory, "attachment.txt"); + File.WriteAllText(attachmentPath, "some user attachment"); + + var hint = new SentryHint(new SentryOptions()); + + // Act + hint.AddAttachment(attachmentPath, AttachmentType.Default); + + // Assert + var attachment = Assert.Single(hint.Attachments); + using (attachment.Content.GetStream()) + { + } + + File.Exists(attachmentPath).Should().BeTrue("we must never delete attachments supplied by the user"); + } + public void Dispose() => Directory.Delete(_testDirectory, true); } diff --git a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs index 4eb29f4290..f50029edab 100644 --- a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs +++ b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs @@ -79,6 +79,44 @@ public async Task WithAttachment() } } + [Fact] + public async Task WithHeapDumpAttachment_DeletesLocalFileButStillSendsIt() + { + // Arrange + string httpContent = null; + var innerTransport = new HttpTransport(_options, new HttpClient(new CallbackHttpClientHandler(async message => + { + httpContent = await message.Content!.ReadAsStringAsync(); + }))); + + await using var transport = CachingTransport.Create(innerTransport, _options, startWorker: false); + + const string dumpContent = "fake heap dump"; + var dumpFile = Path.Combine(_cacheDirectory.Path, "dump.gcdump"); + File.WriteAllText(dumpFile, dumpContent); + + var attachment = new SentryAttachment( + AttachmentType.HeapDump, + new FileAttachmentContent(dumpFile, readFileAsynchronously: true, deleteOnClose: true), + "dump.gcdump", + null); + + var envelope = Envelope.FromEvent(new SentryEvent(), attachments: new[] { attachment }); + + // Act + // The caching transport writes the envelope (including the dump) to the cache directory + // before the envelope is disposed, which is what BackgroundWorker does after each send. + await transport.SendEnvelopeAsync(envelope); + File.Exists(dumpFile).Should().BeTrue("the dump must survive until it has been cached"); + + envelope.Dispose(); + await transport.FlushAsync(); + + // Assert + File.Exists(dumpFile).Should().BeFalse("the dump should be deleted once the envelope is disposed"); + httpContent.Should().Contain(dumpContent, "the cached copy should still be sent to Sentry"); + } + [Fact] public async Task WorksInBackground() { diff --git a/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs b/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs index 6414ff48e4..c8671612b1 100644 --- a/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs +++ b/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs @@ -621,6 +621,154 @@ public async Task SendEnvelopeAsync_AttachmentTooLarge_DropsItem() actualEnvelopeSerialized.Should().NotContain("test2.txt"); } + [Fact] + public async Task SendEnvelopeAsync_HeapDumpAttachmentTooLarge_FileIsDeleted() + { + // Arrange + using var httpHandler = new RecordingHttpMessageHandler( + new FakeHttpMessageHandler()); + + var tempFilePath = Path.GetTempFileName(); + File.WriteAllBytes(tempFilePath, new byte[] { 1, 2, 3, 4, 5 }); + + var httpTransport = new HttpTransport( + new SentryOptions + { + Dsn = ValidDsn, + MaxAttachmentSize = 1, + Debug = true + }, + new HttpClient(httpHandler)); + + var heapDumpAttachment = new SentryAttachment( + AttachmentType.HeapDump, + new FileAttachmentContent(tempFilePath, readFileAsynchronously: true, deleteOnClose: true), + Path.GetFileName(tempFilePath), + null); + + using var envelope = Envelope.FromEvent( + new SentryEvent(), + null, + [heapDumpAttachment]); + + try + { + // Act + await httpTransport.SendEnvelopeAsync(envelope); + + // The oversized item is dropped before sending, so it never reaches the processed + // envelope. The original envelope still owns it, so the file survives until that + // envelope is disposed - which is what BackgroundWorker does after each send. + File.Exists(tempFilePath).Should().BeTrue(); + + envelope.Dispose(); + + // Assert + File.Exists(tempFilePath).Should().BeFalse(); + } + finally + { + if (File.Exists(tempFilePath)) + { + File.Delete(tempFilePath); + } + } + } + + [Fact] + public async Task SendEnvelopeAsync_HeapDumpAttachmentSentSuccessfully_FileIsDeleted() + { + // Arrange + using var httpHandler = new RecordingHttpMessageHandler( + new FakeHttpMessageHandler()); + + var tempFilePath = Path.GetTempFileName(); + File.WriteAllBytes(tempFilePath, new byte[] { 1, 2, 3, 4, 5 }); + + var httpTransport = new HttpTransport( + new SentryOptions + { + Dsn = ValidDsn, + // Large enough that this attachment is NOT dropped for being too big + MaxAttachmentSize = 1_000_000, + Debug = true + }, + new HttpClient(httpHandler)); + + var heapDumpAttachment = new SentryAttachment( + AttachmentType.HeapDump, + new FileAttachmentContent(tempFilePath, readFileAsynchronously: true, deleteOnClose: true), + Path.GetFileName(tempFilePath), + null); + + using var envelope = Envelope.FromEvent( + new SentryEvent(), + null, + [heapDumpAttachment]); + + try + { + // Act + await httpTransport.SendEnvelopeAsync(envelope); + + // Assert + // The file is streamed as part of the successful send, and should be + // deleted once the stream backing the envelope item is closed/disposed. + File.Exists(tempFilePath).Should().BeFalse(); + } + finally + { + if (File.Exists(tempFilePath)) + { + File.Delete(tempFilePath); + } + } + } + + [Fact] + public async Task SendEnvelopeAsync_NonHeapDumpAttachment_FileIsNotDeleted() + { + // Arrange + using var httpHandler = new RecordingHttpMessageHandler( + new FakeHttpMessageHandler()); + + var tempFilePath = Path.GetTempFileName(); + File.WriteAllBytes(tempFilePath, new byte[] { 1, 2, 3, 4, 5 }); + + var httpTransport = new HttpTransport( + new SentryOptions + { + Dsn = ValidDsn, + MaxAttachmentSize = 1_000_000, + Debug = true + }, + new HttpClient(httpHandler)); + + var normalAttachment = new SentryAttachment( + AttachmentType.Default, + new FileAttachmentContent(tempFilePath), + Path.GetFileName(tempFilePath), + null); + + using var envelope = Envelope.FromEvent( + new SentryEvent(), + null, + [normalAttachment]); + + try + { + // Act + await httpTransport.SendEnvelopeAsync(envelope); + + // Assert + File.Exists(tempFilePath).Should().BeTrue(); + } + finally + { + File.Delete(tempFilePath); + } + } + [Fact] public async Task SendEnvelopeAsync_ItemRateLimit_PromotesNextSessionWithSameId() {