feat: Validate single occurence of Sentry SDK inbinaries #5298

itaybre · 2025-05-26T21:24:49Z

📜 Description

Add a log message when Sentry SDK is found to be present twice + Added a breakpoint

💡 Motivation and Context

Fixes #4566

💚 How did you test it?

Running duplicated SDK app + Unit Tests cases

📝 Checklist

You have to check all boxes before merging:

I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
I updated the wizard if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

TODO:

See how to tests this without needing to capture app output
Define how we want to raise this error

github-actions · 2025-05-26T21:25:14Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 54a858b

codecov · 2025-05-26T21:29:08Z

Codecov Report

Attention: Patch coverage is 76.59574% with 11 lines in your changes missing coverage. Please review.

Project coverage is 92.891%. Comparing base (1148435) to head (54a858b).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
Sources/Swift/Core/Tools/LoadValidator.swift	76.086%	11 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #5298       +/-   ##
=============================================
+ Coverage   92.799%   92.891%   +0.092%     
=============================================
  Files          688       689        +1     
  Lines        86295     86428      +133     
  Branches     30098     31201     +1103     
=============================================
+ Hits         80081     80284      +203     
+ Misses        6116      6043       -73     
- Partials        98       101        +3

Files with missing lines	Coverage Δ
Sources/Sentry/SentrySDK.m	`88.770% <100.000%> (+0.030%)`	⬆️
Sources/Swift/Core/Tools/LoadValidator.swift	`76.086% <76.086%> (ø)`

... and 27 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1148435...54a858b. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-05-26T21:39:16Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1233.62 ms	1263.34 ms	29.72 ms
Size	23.76 KiB	823.12 KiB	799.36 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`90d17d3`	1261.18 ms	1278.18 ms	17.00 ms
`1cafec7`	1236.59 ms	1244.06 ms	7.47 ms
`1e065bc`	1239.69 ms	1258.18 ms	18.49 ms
`e70a2e1`	1207.54 ms	1227.17 ms	19.63 ms
`5c3fa88`	1237.17 ms	1254.33 ms	17.16 ms
`149a2d7`	1228.88 ms	1245.72 ms	16.85 ms
`e887ddc`	1234.71 ms	1244.22 ms	9.50 ms
`a0bb101`	1219.67 ms	1242.74 ms	23.07 ms
`c78683b`	1246.71 ms	1258.70 ms	11.99 ms
`e0291c9`	1242.98 ms	1253.08 ms	10.10 ms

App size

Revision	Plain	With Sentry	Diff
`90d17d3`	20.76 KiB	432.17 KiB	411.41 KiB
`1cafec7`	21.58 KiB	615.60 KiB	594.01 KiB
`1e065bc`	20.76 KiB	425.78 KiB	405.01 KiB
`e70a2e1`	21.58 KiB	655.73 KiB	634.15 KiB
`5c3fa88`	22.30 KiB	821.37 KiB	799.07 KiB
`149a2d7`	23.76 KiB	861.22 KiB	837.46 KiB
`e887ddc`	21.58 KiB	616.76 KiB	595.18 KiB
`a0bb101`	22.31 KiB	775.10 KiB	752.79 KiB
`c78683b`	20.76 KiB	435.24 KiB	414.48 KiB
`e0291c9`	22.85 KiB	413.50 KiB	390.65 KiB

Previous results on branch: itaybre/feature/duplicated_class_log

Startup times

Revision	Plain	With Sentry	Diff
`8cc696f`	1233.29 ms	1260.80 ms	27.51 ms
`ac39e43`	1224.49 ms	1246.33 ms	21.84 ms
`c0a154f`	1231.88 ms	1252.54 ms	20.66 ms

App size

Revision	Plain	With Sentry	Diff
`8cc696f`	23.76 KiB	823.13 KiB	799.37 KiB
`ac39e43`	23.76 KiB	822.67 KiB	798.91 KiB
`c0a154f`	23.76 KiB	823.13 KiB	799.37 KiB

philprime

Thanks @itaybre for working on this! Left a couple of comments.

Sources/Swift/Core/Tools/LoadValidator.swift

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

philprime · 2025-05-28T11:17:29Z

Sources/Swift/Core/Tools/LoadValidator.swift

+            imagesWhereSentryIsFound.forEach { path in
+                message.append("  - \(path)")
+            }
+            print(message.joined(separator: "\n"))


m: Couldn't we use SentryLog.fatal(...) here? It would also allow us to assert log messages using TestLogOutput.

Because of how I am running it, I cannot check SentryLog output later.

Check out e.g. testSetUserBeforeStartingSDK_LogsFatalMessage in case you hadn't seen that. In any case, we probably shouldn't use print, but also what happens with the logger if there are multiple copes of the sentry framework loaded 🤔

philprime · 2025-05-28T11:20:20Z

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

+    func testDuplicatedLoadMessageOnSDKInit() throws {
+
+      let output = captureStandardOutput {
+        SentrySDK.start(options: Options())
+      }
+
+      XCTAssertTrue(output.contains("❌ Sentry SDK was loaded multiple times in the binary ❌"))
+    }


m: See my comments above, if we use SentryLog we can assert the logs like this:

Suggested change

func testDuplicatedLoadMessageOnSDKInit() throws {

let output = captureStandardOutput {

SentrySDK.start(options: Options())

}

XCTAssertTrue(output.contains("❌ Sentry SDK was loaded multiple times in the binary ❌"))

}

func testDuplicatedLoadMessageOnSDKInit() throws {

// -- Arrange --

let logOutput = TestLogOutput()

SentryLog.setLogOutput(logOutput)

SentryLog.configureLog(true, diagnosticLevel: .debug)

// -- Act --

SentrySDK.start(options: Options())

// -- Assert --

XCTAssertTrue(logOutput.loggedMessages.contains { line in

line.contains("❌ Sentry SDK was loaded multiple times in the binary ❌")

})

}

Because the test for this uses the xcframework (because we need to link it twice) I didn't have access to the internal classes.
I can try setting it up without the xcframework and see if it links twice somehow

(I was using SentryLog initially if you see the history)

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

Tests/DuplicatedSDKTest/DuplicatedSDKTest.xctestplan

noahsmartin · 2025-05-31T03:06:59Z

Sources/Swift/Core/Tools/LoadValidator.swift

+            print(message.joined(separator: "\n"))
+#if DEBUG
+            // Raise a debugger breakpoint
+            raise(SIGTRAP)


This might end up being very noisy since some apps have no choice but to include the SDK twice when they use third party frameworks that themselves pull in the SDK

The other thing is that DEBUG as a compiler directive may not correlate to SentryOptions.debug or to a debugger even being attached (we have facilities to check for that btw). I assumed the "debug" being referred to here was Sentry debug mode:

noahsmartin · 2025-05-31T03:07:52Z

Sources/Swift/Core/Tools/LoadValidator.swift

+            }
+
+            var classCount: UInt32 = 0
+            if let classNames = objc_copyClassNamesForImage(cImageName, &classCount) {


Should this whole thing be done only in debug builds? Just to prevent overhead for release builds

feat: Add class loaded twice validator

e851f10

itaybre added 3 commits May 27, 2025 00:51

Add basic test capturing print output

65dd818

Update changelog

e9f8da4

Update min target version

f9c5c53

itaybre changed the title ~~feat: Add class loaded twice validator~~ feat: Validate single occurence of Sentry SDK inbinaries May 27, 2025

itaybre marked this pull request as ready for review May 27, 2025 14:25

itaybre requested review from philipphofmann, armcknight, philprime, noahsmartin, trevor-e and NicoHinderling as code owners May 27, 2025 14:25

philprime reviewed May 28, 2025

View reviewed changes

Remove headers

54a858b

noahsmartin reviewed May 31, 2025

View reviewed changes

-    func testDuplicatedLoadMessageOnSDKInit() throws {
-      let output = captureStandardOutput {
-        SentrySDK.start(options: Options())
-      }
-      XCTAssertTrue(output.contains("❌ Sentry SDK was loaded multiple times in the binary ❌"))
-    }
+    func testDuplicatedLoadMessageOnSDKInit() throws {
+        // -- Arrange --
+        let logOutput = TestLogOutput()
+        SentryLog.setLogOutput(logOutput)
+        SentryLog.configureLog(true, diagnosticLevel: .debug)
+        // -- Act --
+        SentrySDK.start(options: Options())
+        // -- Assert --
+        XCTAssertTrue(logOutput.loggedMessages.contains { line in
+            line.contains("❌ Sentry SDK was loaded multiple times in the binary ❌")
+        })
+    }

Uh oh!

feat: Validate single occurence of Sentry SDK inbinaries #5298

Are you sure you want to change the base?

feat: Validate single occurence of Sentry SDK inbinaries #5298

Uh oh!

Conversation

itaybre commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

TODO:

Uh oh!

github-actions bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: itaybre/feature/duplicated_class_log

Startup times

App size

Uh oh!

philprime left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

itaybre commented May 26, 2025 •

edited

Loading

github-actions bot commented May 26, 2025 •

edited

Loading

codecov bot commented May 26, 2025 •

edited

Loading

github-actions bot commented May 26, 2025 •

edited

Loading