From d03bed4aa4f93370e578f6a7bf033984ae4bcce0 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Mon, 25 Aug 2025 11:18:32 +0200 Subject: [PATCH 1/5] fix(client): Fix attachment processor dropping additional atachments --- Sources/Sentry/SentryClient.m | 4 +++- Sources/Sentry/SentryScreenshotIntegration.m | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 2ddc6c012ab..f9e6ff6e8a2 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -1134,7 +1134,9 @@ - (void)removeAttachmentProcessor:(id)attachmen NSArray *processedAttachments = attachments; for (id attachmentProcessor in self.attachmentProcessors) { - processedAttachments = [attachmentProcessor processAttachments:attachments forEvent:event]; + // Keep chaining the processed attachments so each processor works on the output of the + // previous one. This is necessary so each processor can add and remove attachments. + processedAttachments = [attachmentProcessor processAttachments:processedAttachments forEvent:event]; } return processedAttachments; diff --git a/Sources/Sentry/SentryScreenshotIntegration.m b/Sources/Sentry/SentryScreenshotIntegration.m index 4d0de6ea4e2..0a2b1e425d8 100644 --- a/Sources/Sentry/SentryScreenshotIntegration.m +++ b/Sources/Sentry/SentryScreenshotIntegration.m @@ -87,13 +87,12 @@ - (void)uninstall return attachments; } + + NSMutableArray *result = [NSMutableArray arrayWithArray:attachments]; + NSArray *screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshotDatasFromMainThread]; - NSMutableArray *result = - [NSMutableArray arrayWithCapacity:attachments.count + screenshot.count]; - [result addObjectsFromArray:attachments]; - for (int i = 0; i < screenshot.count; i++) { NSString *name = i == 0 ? @"screenshot.png" : [NSString stringWithFormat:@"screenshot-%i.png", i + 1]; From 5ebf3b52fd0aa4f34be1810601c5da4ffc289c65 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Mon, 25 Aug 2025 11:25:37 +0200 Subject: [PATCH 2/5] add notes --- Sources/Sentry/SentryClient.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index f9e6ff6e8a2..172ccf0219b 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -1136,6 +1136,10 @@ - (void)removeAttachmentProcessor:(id)attachmen for (id attachmentProcessor in self.attachmentProcessors) { // Keep chaining the processed attachments so each processor works on the output of the // previous one. This is necessary so each processor can add and remove attachments. + // + // Important: This means the order of adding processors matters and relies on the initialization order of + // the integrations. At this point in time the attachment processors are only adding attachments, therefore + // we can ignore this restriction for now. processedAttachments = [attachmentProcessor processAttachments:processedAttachments forEvent:event]; } From 5727c55ca9628b481745efe26f6e19a5a4a72ffa Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Mon, 25 Aug 2025 11:27:29 +0200 Subject: [PATCH 3/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 734c76ffbe4..59b3921832c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Don't capture replays for events dropped in `beforeSend` (#5916) - Fix linking with SentrySwiftUI on Xcode 26 for visionOS (#5823) +- Fix missing view hierachy when enabling `attachScreenshot` too (#5989) ### Improvements From 5649c95651aa85463e88244a28a120b611a182df Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Tue, 26 Aug 2025 17:11:50 +0200 Subject: [PATCH 4/5] added tests --- Sources/Sentry/SentryClient.m | 9 ++- Sources/Sentry/SentryScreenshotIntegration.m | 1 - Tests/SentryTests/SentryClientTests.swift | 82 ++++++++++++++++++-- 3 files changed, 80 insertions(+), 12 deletions(-) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 172ccf0219b..819b96ec38c 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -1137,10 +1137,11 @@ - (void)removeAttachmentProcessor:(id)attachmen // Keep chaining the processed attachments so each processor works on the output of the // previous one. This is necessary so each processor can add and remove attachments. // - // Important: This means the order of adding processors matters and relies on the initialization order of - // the integrations. At this point in time the attachment processors are only adding attachments, therefore - // we can ignore this restriction for now. - processedAttachments = [attachmentProcessor processAttachments:processedAttachments forEvent:event]; + // Important: This means the order of adding processors matters and relies on the + // initialization order of the integrations. At this point in time the attachment processors + // are only adding attachments, therefore we can ignore this restriction for now. + processedAttachments = [attachmentProcessor processAttachments:processedAttachments + forEvent:event]; } return processedAttachments; diff --git a/Sources/Sentry/SentryScreenshotIntegration.m b/Sources/Sentry/SentryScreenshotIntegration.m index 0a2b1e425d8..7ffc66c26bc 100644 --- a/Sources/Sentry/SentryScreenshotIntegration.m +++ b/Sources/Sentry/SentryScreenshotIntegration.m @@ -87,7 +87,6 @@ - (void)uninstall return attachments; } - NSMutableArray *result = [NSMutableArray arrayWithArray:attachments]; NSArray *screenshot = diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 6f380f2e405..9412e1405af 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -6,7 +6,7 @@ import XCTest // We are aware that the client has a lot of logic and we should maybe // move some of it to other classes. @available(*, deprecated, message: "This is deprecated because SentryOptions integrations is deprecated") -class SentryClientTest: XCTestCase { +class SentryClientTests: XCTestCase { private static let dsn = TestConstants.dsnAsString(username: "SentryClientTest") @@ -56,7 +56,7 @@ class SentryClientTest: XCTestCase { user.ipAddress = "127.0.0.1" let options = Options() - options.dsn = SentryClientTest.dsn + options.dsn = SentryClientTests.dsn fileManager = try XCTUnwrap(SentryFileManager(options: options, dispatchQueueWrapper: TestSentryDispatchQueueWrapper())) transaction = Transaction(trace: trace, children: []) @@ -81,7 +81,7 @@ class SentryClientTest: XCTestCase { var client: SentryClient! do { let options = try SentryOptionsInternal.initWithDict([ - "dsn": SentryClientTest.dsn + "dsn": SentryClientTests.dsn ]) options.removeAllIntegrations() configureOptions(options) @@ -167,7 +167,7 @@ class SentryClientTest: XCTestCase { SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = fixture.dispatchQueue let options = Options() - options.dsn = SentryClientTest.dsn + options.dsn = SentryClientTests.dsn // We have to put our cache into a subfolder of the default path, because on macOS we can't delete the default cache folder options.cacheDirectoryPath = "\(options.cacheDirectoryPath)/cache" _ = SentryClient(options: options) @@ -475,6 +475,74 @@ class SentryClientTest: XCTestCase { XCTAssertEqual(sentAttachments.count, 1) XCTAssertEqual(extraAttachment, sentAttachments.first) } + + func test_AttachmentProcessors_Chained_Additive() { + // -- Arrange -- + let sut = fixture.getSut() + let event = Event() + let att1 = Attachment(data: Data("one".utf8), filename: "AttachmentOne") + let att2 = Attachment(data: Data("two".utf8), filename: "AttachmentTwo") + + let p1 = TestAttachmentProcessor { atts, _ in + var out = atts + out.append(att1) + return out + } + + let p2 = TestAttachmentProcessor { atts, _ in + var out = atts + out.append(att2) + return out + } + + // Order matters; second sees the output of the first. + sut.add(p1) + sut.add(p2) + + // -- Act -- + sut.capture(event: event) + + // -- Assert -- + let sentAttachments = fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.attachments ?? [] + XCTAssertEqual(sentAttachments.count, 2) + XCTAssertEqual(sentAttachments.element(at: 0), att1) + XCTAssertEqual(sentAttachments.element(at: 1), att2) + } + + func test_AttachmentProcessors_Chained_RemovalThenAdd() { + // -- Arrange -- + let sut = fixture.getSut() + let event = Event() + + // Start with one attachment from the scope so the removal has an effect. + let initial = Attachment(data: Data("init".utf8), filename: "Initial") + let scope = Scope() + scope.addAttachment(initial) + + // First processor removes everything. + let remover = TestAttachmentProcessor { _, _ in + return [] + } + + // Second processor appends a new one; should not see the removed initial. + let added = Attachment(data: Data("new".utf8), filename: "AddedAfterRemoval") + let adder = TestAttachmentProcessor { atts, _ in + var out = atts + out.append(added) + return out + } + + sut.add(remover) + sut.add(adder) + + // -- Act -- + sut.capture(event: event, scope: scope) + + // -- Assert -- + let sentAttachments = fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.attachments ?? [] + XCTAssertEqual(sentAttachments.count, 1) + XCTAssertEqual(sentAttachments.first, added) + } func testCaptureEventWithDsnSetAfterwards() { let event = Event() @@ -483,7 +551,7 @@ class SentryClientTest: XCTestCase { options.dsn = nil }) - sut.options.dsn = SentryClientTest.dsn + sut.options.dsn = SentryClientTests.dsn let eventId = sut.capture(event: event) eventId.assertIsNotEmpty() @@ -1737,7 +1805,7 @@ class SentryClientTest: XCTestCase { SentryFileManager.prepareInitError() let options = Options() - options.dsn = SentryClientTest.dsn + options.dsn = SentryClientTests.dsn let client = SentryClient(options: options, dispatchQueue: TestSentryDispatchQueueWrapper(), deleteOldEnvelopeItems: false) XCTAssertNil(client) @@ -2177,7 +2245,7 @@ class SentryClientTest: XCTestCase { } @available(*, deprecated, message: "This is deprecated because SentryOptions integrations is deprecated") -private extension SentryClientTest { +private extension SentryClientTests { private func givenEventWithDebugMeta() -> Event { let event = Event(level: SentryLevel.fatal) let debugMeta = DebugMeta() From e3f572d687c77c1cc8ec46c4a6a9ab7e6aaed7d4 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 27 Aug 2025 12:05:28 +0200 Subject: [PATCH 5/5] fix test after refactoring --- Tests/SentryTests/SentryClientTests.swift | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 9412e1405af..95edc49cc12 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -2210,8 +2210,8 @@ class SentryClientTests: XCTestCase { XCTAssertEqual(0, fixture.transport.sentEnvelopes.count) } -#if os(macOS) func testCaptureSentryWrappedException() throws { +#if os(macOS) let exception = NSException(name: NSExceptionName("exception"), reason: "reason", userInfo: nil) // If we don't raise the exception, it won't have the callStack data let raisedException = ExceptionCatcher.try { @@ -2232,7 +2232,7 @@ class SentryClientTests: XCTestCase { // Make sure the stacktrace is not empty XCTAssertGreaterThan(actual.threads?[0].stacktrace?.frames.count ?? 0, 1) // We will need to update it if the test class / module changes - let testMangledName = "$s11SentryTests0A10ClientTestC011testCaptureA16WrappedExceptionyyKF" + let testMangledName = "$s11SentryTests0a6ClientB0C011testCaptureA16WrappedExceptionyyKF" let frameWithTestFunction = actual.threads?[0].stacktrace?.frames.first { frame in frame.function == testMangledName } @@ -2240,8 +2240,10 @@ class SentryClientTests: XCTestCase { // Last frame should always be `__exceptionPreprocess` XCTAssertEqual(actual.threads?[0].stacktrace?.frames.last?.function, "__exceptionPreprocess") + #else + throw XCTSkip("Test is disabled for this OS version") + #endif // os(macOS) } -#endif // os(macOS) } @available(*, deprecated, message: "This is deprecated because SentryOptions integrations is deprecated")