From e8cd524503c4a4619e273d672265a5c14b58918f Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 21 Aug 2026 13:46:23 -0500 Subject: [PATCH 1/6] feat: [SDK-5047] drive iOS logging from remote params iOS parsed logging_config.log_level but never used it to control the logging lifecycle, so remote crash/OTel logging could not be turned on or adjusted server-side the way it can on Android. Adds OSRemoteLoggingConfiguration (Android's RemoteLoggingConfigModel: a persisted level plus an enabled flag derived from the server sending a valid level) and OSRemoteLoggingConfigEvaluator (Android's OtelConfigEvaluator: a pure old-vs-new diff yielding Enable/Disable/UpdateLogLevel/NoChange). The controller now applies those actions instead of only filtering log lines. Two behavioral consequences worth noting. NONE now means enabled-but-exporting -nothing, matching Android's hydrate path, so the crash handler stays armed rather than collapsing to fully disabled. And a level change rebuilds the logger, as Android does in startLogging, because the platform provider handed to KMP is built alongside the logger; reusing the instance would keep reporting the previous level into KMP and would never re-run the crash uploader after a level escalates away from NONE. The cache also stops round-tripping through a synthesized params-shaped dictionary and owns its own format, so it no longer depends on the shape of the params API response. Co-authored-by: Cursor --- .../OneSignal.xcodeproj/project.pbxproj | 8 + .../Source/OSRemoteLoggingConfiguration.swift | 189 +++++++++++++++++ .../Source/OSRemoteLoggingController.swift | 133 +++--------- .../OSRemoteLoggingControllerTests.swift | 198 +++++++++++++++++- 4 files changed, 421 insertions(+), 107 deletions(-) create mode 100644 iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj index ab6fc07bd..3c6c97464 100644 --- a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj +++ b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj @@ -79,6 +79,9 @@ B96A3B6BA8CC49EE4796D9BF /* OSRemoteLoggingController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A72F938F8A3808AC1FF7F3C /* OSRemoteLoggingController.swift */; }; 25898119922BDCDA7AF0B9CC /* OSRemoteLoggingController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A72F938F8A3808AC1FF7F3C /* OSRemoteLoggingController.swift */; }; 9EAF92032D0429FA35E04417 /* OSRemoteLoggingController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A72F938F8A3808AC1FF7F3C /* OSRemoteLoggingController.swift */; }; + A91B2C3D4E5F60718293A4B5 /* OSRemoteLoggingConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C91A2B0D84F1E9A3C5B6D8E /* OSRemoteLoggingConfiguration.swift */; }; + B82C3D4E5F60718293A4B5C6 /* OSRemoteLoggingConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C91A2B0D84F1E9A3C5B6D8E /* OSRemoteLoggingConfiguration.swift */; }; + C73D4E5F60718293A4B5C6D7 /* OSRemoteLoggingConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C91A2B0D84F1E9A3C5B6D8E /* OSRemoteLoggingConfiguration.swift */; }; ACE2175908241BB46C9F1829 /* OSRemoteLoggingControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 658E6E9E6BC6BBF702BCBD33 /* OSRemoteLoggingControllerTests.swift */; }; 7732574D325D34CC7C498199 /* OSRemoteLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6972EE491A57C79EFE56D4C8 /* OSRemoteLogger.swift */; }; 698F58A488FCE503DFD5247F /* OSLogCrashHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = ACD3284210A7DF2597594778 /* OSLogCrashHandler.swift */; }; @@ -1800,6 +1803,7 @@ 3C14E3AE2FAE54C006ED053 /* OSLoggerAdaptersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSLoggerAdaptersTests.swift; sourceTree = ""; }; 3B6A59620B83538CEFF77269 /* OSLogCrashHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSLogCrashHandlerTests.swift; sourceTree = ""; }; 8A72F938F8A3808AC1FF7F3C /* OSRemoteLoggingController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRemoteLoggingController.swift; sourceTree = ""; }; + 7C91A2B0D84F1E9A3C5B6D8E /* OSRemoteLoggingConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRemoteLoggingConfiguration.swift; sourceTree = ""; }; C0462F96E1AADF655F3B3765 /* OSRemoteLoggingController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = OSRemoteLoggingController.h; sourceTree = ""; }; 658E6E9E6BC6BBF702BCBD33 /* OSRemoteLoggingControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRemoteLoggingControllerTests.swift; sourceTree = ""; }; 6972EE491A57C79EFE56D4C8 /* OSRemoteLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRemoteLogger.swift; sourceTree = ""; }; @@ -2718,6 +2722,7 @@ DEC08AFF2947D4E900C81DA3 /* OneSignalSwiftInterface.swift */, C0462F96E1AADF655F3B3765 /* OSRemoteLoggingController.h */, 8A72F938F8A3808AC1FF7F3C /* OSRemoteLoggingController.swift */, + 7C91A2B0D84F1E9A3C5B6D8E /* OSRemoteLoggingConfiguration.swift */, 3CC9A6352AFA26E7008F68FD /* PrivacyInfo.xcprivacy */, ); path = Source; @@ -4403,6 +4408,7 @@ 9124121E1E73342200E41FD7 /* OneSignalJailbreakDetection.m in Sources */, 3C14E3A42AFAE54C006ED053 /* OneSignalSwiftInterface.swift in Sources */, B96A3B6BA8CC49EE4796D9BF /* OSRemoteLoggingController.swift in Sources */, + A91B2C3D4E5F60718293A4B5 /* OSRemoteLoggingConfiguration.swift in Sources */, 912412471E73369600E41FD7 /* OneSignalHelper.m in Sources */, CA8E19062193C76D009DA223 /* OSInAppMessagingHelpers.m in Sources */, 7AAA60682485D0420004FADE /* OSMigrationController.m in Sources */, @@ -4557,6 +4563,7 @@ 7AECE59F23675F6300537907 /* OSFocusTimeProcessorFactory.m in Sources */, DEC08B012947D4E900C81DA3 /* OneSignalSwiftInterface.swift in Sources */, 25898119922BDCDA7AF0B9CC /* OSRemoteLoggingController.swift in Sources */, + B82C3D4E5F60718293A4B5C6 /* OSRemoteLoggingConfiguration.swift in Sources */, CA1A6E7120DC2E73001C41B9 /* OneSignalDialogRequest.m in Sources */, 912412331E73342200E41FD7 /* OneSignalTracker.m in Sources */, 7A674F1C2360D82E001F9ACD /* OSBaseFocusTimeProcessor.m in Sources */, @@ -4684,6 +4691,7 @@ 4529DEEA1FA8360C00CEAB1D /* UIApplicationOverrider.m in Sources */, DEC08B022947D4E900C81DA3 /* OneSignalSwiftInterface.swift in Sources */, 9EAF92032D0429FA35E04417 /* OSRemoteLoggingController.swift in Sources */, + C73D4E5F60718293A4B5C6D7 /* OSRemoteLoggingConfiguration.swift in Sources */, 7A93269E25AF4F0300BBEC27 /* OSPendingCallbacks.m in Sources */, 7AECE59823674AB700537907 /* OSUnattributedFocusTimeProcessor.m in Sources */, 7A5A818224897693002E07C8 /* MigrationTests.m in Sources */, diff --git a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift new file mode 100644 index 000000000..e5f771a00 --- /dev/null +++ b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift @@ -0,0 +1,189 @@ +/* + Modified MIT License + + Copyright 2026 OneSignal + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + 10| copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + 1. The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + 2. All copies of substantial portions of the Software may only be used in connection + with services provided by OneSignal. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + 20| IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + */ + +import Foundation +import OneSignalCore + +/// Persisted remote-logging params from the params API, mirroring Android's +/// `RemoteLoggingConfigModel` (`logLevel` + `isEnabled`). +struct OSRemoteLoggingConfiguration: Equatable { + static let loggingConfigKey = "logging_config" + static let logLevelKey = "log_level" + static let cachedAppIdKey = "app_id" + static let cachedLogLevelKey = "log_level" + static let cachedIsEnabledKey = "is_enabled" + + /// Minimum log level to export remotely. `nil` when the backend omitted a valid `log_level`. + let logLevel: ONE_S_LOG_LEVEL? + + /// Set true when the server sends a valid `log_level` (including `NONE`), false otherwise. + let isEnabled: Bool + + var logLevelName: String? { + logLevel.map(Self.levelName) + } + + static var current: OSRemoteLoggingConfiguration { + let params = OSRemoteParamController.shared().remoteParams as? [String: Any] ?? [:] + return OSRemoteLoggingConfiguration(remoteParams: params) + } + + static let disabled = OSRemoteLoggingConfiguration(logLevel: nil, isEnabled: false) + + init(logLevel: ONE_S_LOG_LEVEL?, isEnabled: Bool) { + self.logLevel = logLevel + self.isEnabled = isEnabled + } + + init(remoteParams: [String: Any]) { + let loggingConfig = remoteParams[Self.loggingConfigKey] as? [String: Any] + let parsed = (loggingConfig?[Self.logLevelKey] as? String) + .map { $0.uppercased() } + .flatMap(Self.oneSignalLevel) + self.init(logLevel: parsed, isEnabled: parsed != nil) + } + + init(cached: [AnyHashable: Any]) { + let parsed = (cached[Self.cachedLogLevelKey] as? String) + .map { $0.uppercased() } + .flatMap(Self.oneSignalLevel) + let isEnabled = (cached[Self.cachedIsEnabledKey] as? Bool) ?? (parsed != nil) + self.init(logLevel: parsed, isEnabled: isEnabled) + } + + func cachePayload(appId: String) -> [String: Any] { + var cached: [String: Any] = [ + Self.cachedAppIdKey: appId, + Self.cachedIsEnabledKey: isEnabled + ] + if let logLevelName { + cached[Self.cachedLogLevelKey] = logLevelName + } + return cached + } + + func allows(_ level: ONE_S_LOG_LEVEL) -> Bool { + guard isEnabled, let logLevel, logLevel != .LL_NONE else { + return false + } + return level != .LL_NONE && level.rawValue <= logLevel.rawValue + } + + func matches(_ other: OSRemoteLoggingConfiguration) -> Bool { + logLevel == other.logLevel && isEnabled == other.isEnabled + } + + static func levelName(_ level: ONE_S_LOG_LEVEL) -> String { + switch level { + case .LL_FATAL: + return "FATAL" + case .LL_ERROR: + return "ERROR" + case .LL_WARN: + return "WARN" + case .LL_INFO: + return "INFO" + case .LL_DEBUG: + return "DEBUG" + case .LL_VERBOSE: + return "VERBOSE" + default: + return "NONE" + } + } + + static func oneSignalLevel(_ value: String) -> ONE_S_LOG_LEVEL? { + switch value { + case "NONE": + return .LL_NONE + case "FATAL": + return .LL_FATAL + case "ERROR": + return .LL_ERROR + case "WARN", "WARNING": + return .LL_WARN + case "INFO": + return .LL_INFO + case "DEBUG": + return .LL_DEBUG + case "VERBOSE", "TRACE": + return .LL_VERBOSE + default: + return nil + } + } +} + +/// Snapshot used by [OSRemoteLoggingConfigEvaluator], matching Android's `OtelConfig`. +struct OSRemoteLoggingConfig: Equatable { + let isEnabled: Bool + let logLevel: ONE_S_LOG_LEVEL? + + init(_ configuration: OSRemoteLoggingConfiguration) { + isEnabled = configuration.isEnabled + logLevel = configuration.logLevel + } + + init(isEnabled: Bool, logLevel: ONE_S_LOG_LEVEL?) { + self.isEnabled = isEnabled + self.logLevel = logLevel + } + + static let disabled = OSRemoteLoggingConfig(isEnabled: false, logLevel: nil) +} + +enum OSRemoteLoggingConfigAction: Equatable { + case noChange + case enable(ONE_S_LOG_LEVEL) + case updateLogLevel(old: ONE_S_LOG_LEVEL, new: ONE_S_LOG_LEVEL) + case disable +} + +/// Pure diff of old vs new remote-logging config, mirroring Android's `OtelConfigEvaluator`. +enum OSRemoteLoggingConfigEvaluator { + static func evaluate( + old: OSRemoteLoggingConfig?, + new: OSRemoteLoggingConfig + ) -> OSRemoteLoggingConfigAction { + let wasEnabled = old?.isEnabled == true + let isNowEnabled = new.isEnabled + + switch (wasEnabled, isNowEnabled) { + case (false, true): + return .enable(new.logLevel ?? .LL_ERROR) + case (true, false): + return .disable + case (true, true) where old?.logLevel != new.logLevel: + return .updateLogLevel( + old: old?.logLevel ?? .LL_ERROR, + new: new.logLevel ?? .LL_ERROR + ) + default: + return .noChange + } + } +} diff --git a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift index 55e452656..3ac1b8853 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift +++ b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift @@ -31,81 +31,6 @@ import OneSignalCore @_spi(OneSignalInternal) import OneSignalUser import UIKit -struct OSRemoteLoggingConfiguration { - private static let loggingConfigKey = "logging_config" - private static let logLevelKey = "log_level" - - private let threshold: ONE_S_LOG_LEVEL? - - var logLevel: String? { - threshold.map(Self.levelName) - } - - var isRemoteLoggingEnabled: Bool { - threshold != nil && threshold != .LL_NONE - } - - static var current: OSRemoteLoggingConfiguration { - let params = OSRemoteParamController.shared().remoteParams as? [String: Any] ?? [:] - return OSRemoteLoggingConfiguration(remoteParams: params) - } - - init(remoteParams: [String: Any]) { - let loggingConfig = remoteParams[Self.loggingConfigKey] as? [String: Any] - let parsedThreshold = (loggingConfig?[Self.logLevelKey] as? String) - .map { $0.uppercased() } - .flatMap(Self.oneSignalLevel) - - threshold = parsedThreshold - } - - func allows(_ level: ONE_S_LOG_LEVEL) -> Bool { - guard isRemoteLoggingEnabled, let threshold else { - return false - } - return level != .LL_NONE && level.rawValue <= threshold.rawValue - } - - static func levelName(_ level: ONE_S_LOG_LEVEL) -> String { - switch level { - case .LL_FATAL: - return "FATAL" - case .LL_ERROR: - return "ERROR" - case .LL_WARN: - return "WARN" - case .LL_INFO: - return "INFO" - case .LL_DEBUG: - return "DEBUG" - case .LL_VERBOSE: - return "VERBOSE" - default: - return "NONE" - } - } - - private static func oneSignalLevel(_ value: String) -> ONE_S_LOG_LEVEL? { - switch value { - case "FATAL": - return .LL_FATAL - case "ERROR": - return .LL_ERROR - case "WARN", "WARNING": - return .LL_WARN - case "INFO": - return .LL_INFO - case "DEBUG": - return .LL_DEBUG - case "VERBOSE", "TRACE": - return .LL_VERBOSE - default: - return nil - } - } - -} - @objc(OSRemoteLoggingController) final class OSRemoteLoggingController: NSObject, OSInternalLogSink { typealias RemoteLoggerFactory = (OSRemoteLoggerProviders) -> OSStructuredRemoteLoggerProtocol @@ -113,8 +38,6 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { private static let shared = OSRemoteLoggingController() private static let installIdKey = "PREFS_OS_INSTALL_ID" private static let cachedConfigurationKey = "PREFS_OS_REMOTE_LOGGING_CONFIGURATION" - private static let cachedAppIdKey = "app_id" - private static let cachedLogLevelKey = "log_level" private static let backgroundTaskPrefix = "com.onesignal.logger.flush." private static let installId: String = { let defaults = OneSignalUserDefaults.initShared() @@ -133,7 +56,7 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { private let usesScenes: () -> Bool private let beginBackgroundTask: (String) -> Void private let endBackgroundTask: (String) -> Void - private var configuration = OSRemoteLoggingConfiguration(remoteParams: [:]) + private var configuration = OSRemoteLoggingConfiguration.disabled private var configurationGeneration = 0 private var remoteLogger: OSStructuredRemoteLoggerProtocol? private var appState = "unknown" @@ -177,15 +100,11 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { forKey: cachedConfigurationKey, defaultValue: nil ), - cached[cachedAppIdKey] as? String == appId else { + cached[OSRemoteLoggingConfiguration.cachedAppIdKey] as? String == appId else { shared.shutdown() return } - let logLevel = cached[cachedLogLevelKey] as? String - let remoteParams = logLevel.map { - ["logging_config": ["log_level": $0]] - } ?? [:] - shared.configure(with: OSRemoteLoggingConfiguration(remoteParams: remoteParams)) + shared.configure(with: OSRemoteLoggingConfiguration(cached: cached)) } @objc class func reset() { @@ -196,13 +115,9 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { guard let appId = OneSignalIdentifiers.currentAppId else { return } - var cached: [String: Any] = [cachedAppIdKey: appId] - if let logLevel = configuration.logLevel { - cached[cachedLogLevelKey] = logLevel - } OneSignalUserDefaults.initStandard().saveDictionary( forKey: cachedConfigurationKey, - withValue: cached + withValue: configuration.cachePayload(appId: appId) ) } @@ -256,19 +171,33 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { stateQueue.sync { self.configurationGeneration += 1 let generation = self.configurationGeneration - let previousLogLevel = self.configuration.logLevel + let action = OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(self.configuration), + new: OSRemoteLoggingConfig(newConfiguration) + ) self.configuration = newConfiguration - guard newConfiguration.isRemoteLoggingEnabled else { - self.stopRemoteLogging() - return - } - if previousLogLevel != newConfiguration.logLevel { + switch action { + case .disable: + self.stopRemoteLogging() + case .updateLogLevel: + // Android rebuilds remote telemetry on a level change rather than + // swapping a filter on the live instance: `startLogging` shuts the + // previous one down before constructing a new one. Rebuilding matters + // for more than parity here — the platform provider handed to KMP is + // built alongside the logger, so keeping the old instance would go on + // reporting the previous level into KMP, and the crash uploader would + // never re-run after a level escalates away from NONE. self.stopRemoteLogging() - } - - if self.remoteLogger == nil { startGeneration = generation + case .enable, .noChange: + guard newConfiguration.isEnabled else { + self.stopRemoteLogging() + break + } + if self.remoteLogger == nil { + startGeneration = generation + } } } @@ -353,7 +282,7 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { pushSubscriptionId: { OneSignalUserManagerImpl.sharedInstance.pushSubscriptionId }, appState: { [weak self] in self?.currentAppState ?? "unknown" }, featureFlags: { [] }, - remoteLogLevel: { configuration.logLevel }, + remoteLogLevel: { configuration.logLevelName }, exporterLoggingEnabled: { false } ) } @@ -435,12 +364,6 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { } -private extension OSRemoteLoggingConfiguration { - func matches(_ other: OSRemoteLoggingConfiguration) -> Bool { - logLevel == other.logLevel - } -} - struct OSRemoteLoggerProviders { let installId: () -> String let onesignalId: () -> String? diff --git a/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift b/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift index 523cc5394..b948322cf 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift +++ b/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift @@ -44,7 +44,8 @@ final class OSRemoteLoggingControllerTests: XCTestCase { let enabled = OSRemoteLoggingConfiguration( remoteParams: ["logging_config": ["log_level": "warn"]] ) - XCTAssertTrue(enabled.isRemoteLoggingEnabled) + XCTAssertTrue(enabled.isEnabled) + XCTAssertEqual(enabled.logLevel, .LL_WARN) XCTAssertTrue(enabled.allows(.LL_ERROR)) XCTAssertTrue(enabled.allows(.LL_WARN)) XCTAssertFalse(enabled.allows(.LL_INFO)) @@ -52,7 +53,137 @@ final class OSRemoteLoggingControllerTests: XCTestCase { let invalidLevel = OSRemoteLoggingConfiguration( remoteParams: ["logging_config": ["log_level": "OFF"]] ) - XCTAssertFalse(invalidLevel.isRemoteLoggingEnabled) + XCTAssertFalse(invalidLevel.isEnabled) + XCTAssertNil(invalidLevel.logLevel) + } + + func testIosParamsPayloadEnablesVerboseLevelLogging() { + let configuration = OSRemoteLoggingConfiguration(remoteParams: Self.iosParamsPayload) + + XCTAssertTrue(configuration.isEnabled) + XCTAssertEqual(configuration.logLevel, .LL_VERBOSE) + XCTAssertEqual(configuration.logLevelName, "VERBOSE") + + // VERBOSE is the most permissive level, so every severity is exported. + for level in [ONE_S_LOG_LEVEL.LL_FATAL, .LL_ERROR, .LL_WARN, .LL_INFO, .LL_DEBUG, .LL_VERBOSE] { + XCTAssertTrue(configuration.allows(level)) + } + XCTAssertFalse(configuration.allows(.LL_NONE)) + + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(.disabled), + new: OSRemoteLoggingConfig(configuration) + ), + .enable(.LL_VERBOSE) + ) + + let restored = OSRemoteLoggingConfiguration( + cached: configuration.cachePayload(appId: "app-id") + ) + XCTAssertEqual(restored.logLevel, .LL_VERBOSE) + XCTAssertTrue(restored.isEnabled) + } + + func testIosParamsPayloadRoutesEverySeverityThroughController() { + let telemetry = RemoteTelemetrySpy() + telemetry.emitExpectation = expectation(description: "routes verbose from ios_params payload") + telemetry.emitExpectation?.expectedFulfillmentCount = 2 + let controller = makeController(remoteLoggerFactory: { _ in telemetry }) + + controller.configure(remoteParams: Self.iosParamsPayload) + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "verbose is exported") + OneSignalLog.onesignalLog(.LL_ERROR, message: "error is exported") + + wait(for: [telemetry.emitExpectation!], timeout: 2) + XCTAssertEqual(telemetry.messages, ["verbose is exported", "error is exported"]) + XCTAssertEqual(telemetry.levels, ["VERBOSE", "ERROR"]) + } + + func testNoneLogLevelEnablesRemoteLoggingButDoesNotSend() { + let none = OSRemoteLoggingConfiguration( + remoteParams: ["logging_config": ["log_level": "NONE"]] + ) + XCTAssertTrue(none.isEnabled) + XCTAssertEqual(none.logLevel, .LL_NONE) + XCTAssertFalse(none.allows(.LL_FATAL)) + XCTAssertFalse(none.allows(.LL_ERROR)) + } + + func testCachePersistsLogLevelAndEnabledFlag() { + let enabled = OSRemoteLoggingConfiguration( + remoteParams: ["logging_config": ["log_level": "ERROR"]] + ) + let payload = enabled.cachePayload(appId: "app-id") + XCTAssertEqual(payload["app_id"] as? String, "app-id") + XCTAssertEqual(payload["log_level"] as? String, "ERROR") + XCTAssertEqual(payload["is_enabled"] as? Bool, true) + + let restored = OSRemoteLoggingConfiguration(cached: payload) + XCTAssertTrue(restored.isEnabled) + XCTAssertEqual(restored.logLevel, .LL_ERROR) + } + + func testLegacyCacheWithoutIsEnabledUsesLogLevelPresence() { + let legacyEnabled = OSRemoteLoggingConfiguration(cached: ["log_level": "WARN"]) + XCTAssertTrue(legacyEnabled.isEnabled) + XCTAssertEqual(legacyEnabled.logLevel, .LL_WARN) + + let legacyDisabled = OSRemoteLoggingConfiguration(cached: ["app_id": "app-id"]) + XCTAssertFalse(legacyDisabled.isEnabled) + XCTAssertNil(legacyDisabled.logLevel) + } + + func testEvaluatorMirrorsAndroidOtelConfigEvaluator() { + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: nil, + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) + ), + .enable(.LL_WARN) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: nil, + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: nil) + ), + .enable(.LL_ERROR) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: nil, + new: OSRemoteLoggingConfig.disabled + ), + .noChange + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig.disabled, + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_INFO) + ), + .enable(.LL_INFO) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), + new: OSRemoteLoggingConfig.disabled + ), + .disable + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) + ), + .updateLogLevel(old: .LL_ERROR, new: .LL_WARN) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR) + ), + .noChange + ) } func testControllerRoutesLogsAndFlushesOnBackground() { @@ -138,6 +269,52 @@ final class OSRemoteLoggingControllerTests: XCTestCase { XCTAssertTrue(telemetry.messages.isEmpty) } + func testNoneLogLevelStartsLoggerButDoesNotSend() { + let telemetry = RemoteTelemetrySpy() + telemetry.emitExpectation = expectation(description: "does not route at NONE") + telemetry.emitExpectation?.isInverted = true + let controller = makeController(remoteLoggerFactory: { _ in telemetry }) + controller.configure(remoteParams: Self.remoteParams(level: "NONE")) + + OneSignalLog.onesignalLog(.LL_ERROR, message: "not uploaded") + + wait(for: [telemetry.emitExpectation!], timeout: 0.2) + XCTAssertEqual(telemetry.startCount, 1) + XCTAssertEqual(telemetry.shutdownCount, 0) + XCTAssertTrue(telemetry.messages.isEmpty) + } + + func testLogLevelUpdateRebuildsLoggerWithTheNewLevel() { + var loggers: [RemoteTelemetrySpy] = [] + var levelsReportedToKmp: [String?] = [] + let controller = makeController { providers in + levelsReportedToKmp.append(providers.remoteLogLevel()) + let logger = RemoteTelemetrySpy() + loggers.append(logger) + return logger + } + + controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Self.remoteParams(level: "WARN")) + + // Android's updateLogLevel shuts the previous telemetry down and builds a new + // one. Rebuilding is what keeps the level reported into KMP in step with the + // configured level, which a live-instance filter swap would leave stale. + XCTAssertEqual(loggers.count, 2) + XCTAssertEqual(levelsReportedToKmp, ["ERROR", "WARN"]) + XCTAssertEqual(loggers[0].shutdownCount, 1) + XCTAssertEqual(loggers[1].startCount, 1) + + let current = loggers[1] + current.emitExpectation = expectation(description: "routes warn after level update") + OneSignalLog.onesignalLog(.LL_WARN, message: "uploaded") + OneSignalLog.onesignalLog(.LL_INFO, message: "not uploaded") + + wait(for: [current.emitExpectation!], timeout: 2) + XCTAssertEqual(current.messages, ["uploaded"]) + XCTAssertEqual(current.levels, ["WARN"]) + } + func testTerminationFlushesBeforeShutdown() { let notificationCenter = NotificationCenter() let telemetry = RemoteTelemetrySpy() @@ -252,6 +429,23 @@ final class OSRemoteLoggingControllerTests: XCTestCase { private static func remoteParams(level: String) -> [String: Any] { ["logging_config": ["log_level": level]] } + + /// Verbatim ios_params response, so parsing stays honest about the real shape + /// rather than only the trimmed dictionaries the other tests use. + private static let iosParamsPayload: [String: Any] = [ + "fba": true, + "uses_provisional_auth": true, + "outcomes": [ + "direct": ["enabled": true], + "indirect": [ + "notification_attribution": ["minutes_since_displayed": 1440, "limit": 10], + "enabled": true + ], + "unattributed": ["enabled": true] + ], + "receive_receipts_enable": true, + "logging_config": ["log_level": "VERBOSE"] + ] } private final class ReentrantLogListener: NSObject, OSLogListener { From 710ec6970c02cc60cd2fe0b842d548cf6bf9e191 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 21 Aug 2026 13:46:35 -0500 Subject: [PATCH 2/6] fix: [SDK-5047] correct KMP boundary threading and teardown Three pre-existing defects in the KMP adapter, all found while verifying the remote-logging lifecycle end to end. Kotlin/Native only permits calling exported suspend functions from the main thread, but all three crossings (LogLoggingHelper.log, ILogTelemetry .forceFlush, LogCrashUploader.start) were reached from the controller's serial queue and from URLSession callbacks, which trips a runtime failure as soon as the backend enables logging. They are now marshalled to main. The hop runs inline when already on main so a caller that blocks waiting on the completion cannot deadlock itself. telemetry.shutdown() drains buffered records under runBlocking with a five second cap, and was reachable from initialize: and from app-id changes through stateQueue.sync, so a slow network could stall the UI for that long. Only the drain is deferred to a background queue; unregistering the crash handler stays synchronous, because OSLogCrashHandler.initialize() bails when another handler is still registered and deferring it would silently disable crash capture after an app-id change. Because the drain is now asynchronous, start() also has to reject once shutdown has begun rather than once it has finished -- otherwise a logger told to shut down could still install a crash handler that nothing would ever unregister, permanently blocking later loggers. Finally, the HTTP exporter's diagnostics logged through OneSignalLog, whose sink is invoked unconditionally, so at verbose levels each export POST created a new exported record and guaranteed another POST. They now log console-only. Co-authored-by: Cursor --- .../Source/Logging/OSRemoteLogger.swift | 101 +++++++++++++----- .../OSLoggerAdaptersTests.swift | 22 ++++ 2 files changed, 98 insertions(+), 25 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift index 6323e9e88..b840d185d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift @@ -54,13 +54,17 @@ public extension OSRemoteLoggerProtocol { @_implementationOnly import OneSignalKMP -private final class OSRemoteLoggerLifecycle { +final class OSRemoteLoggerLifecycle { private let lock = NSLock() private var isStarted = false private var isShuttingDown = false private var isShutdown = false - var canStartUploader: Bool { + var canStartUploader: Bool { canEmit } + + /// Records stop being accepted the moment shutdown begins, rather than once the + /// final drain finishes, so teardown never lets a late log escape. + var canEmit: Bool { lock.lock() defer { lock.unlock() } return isStarted && !isShuttingDown && !isShutdown @@ -76,10 +80,13 @@ private final class OSRemoteLoggerLifecycle { return true } + /// Rejects once shutdown has *begun*, not just once it has finished. The final + /// drain is asynchronous, so a logger told to shut down can otherwise still be + /// started afterwards and install a crash handler nothing will ever unregister. func start() -> Bool { lock.lock() defer { lock.unlock() } - guard !isStarted, !isShutdown else { + guard !isStarted, !isShuttingDown, !isShutdown else { return false } isStarted = true @@ -170,6 +177,9 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { private let lifecycleOperationLock = NSLock() private let uploaderOwner = UUID() + /// Serial so overlapping teardowns cannot stack several bounded drains at once. + private static let teardownQueue = DispatchQueue(label: "com.onesignal.logger.remote-teardown") + public convenience init( installIdProvider: @escaping () -> String, onesignalIdProvider: @escaping () -> String?, @@ -237,9 +247,12 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { let crashLogger = OSCrashLogger() let lifecycle = OSRemoteLoggerLifecycle() let fileStore = FileLogStore(rootPath: provider.crashStoragePath) + // Console-only logger on purpose. Exporter diagnostics describe the POST that + // ships log records, so routing them through OneSignalLog would feed each POST + // back into the export queue as a new record and never settle. let httpSender = Self.makeHttpSender( requestSender: requestSenderOverride, - logger: logger, + logger: crashLogger, isDiagnosticsEnabled: exporterLoggingEnabledProvider, lifecycle: lifecycle ) @@ -309,19 +322,32 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { let logger = self.logger let lifecycle = self.lifecycle OSCrashUploaderCoordinator.shared.enqueue(owner: owner) { - guard lifecycle.canStartUploader else { - OSCrashUploaderCoordinator.shared.finish(owner: owner) - return - } - crashUploader.start { error in - if let error { - logger.error(message: "LogCrashUploader failed: \(error.localizedDescription)") + Self.onMain { + guard lifecycle.canStartUploader else { + OSCrashUploaderCoordinator.shared.finish(owner: owner) + return + } + crashUploader.start { error in + if let error { + logger.error(message: "LogCrashUploader failed: \(error.localizedDescription)") + } + OSCrashUploaderCoordinator.shared.finish(owner: owner) } - OSCrashUploaderCoordinator.shared.finish(owner: owner) } } } + /// Kotlin/Native only supports calling exported `suspend` functions from the main + /// thread, so every crossing into KMP has to be marshalled here. Callers reach this + /// class from the logging controller's serial queue and from URLSession callbacks. + private static func onMain(_ work: @escaping () -> Void) { + if Thread.isMainThread { + work() + } else { + DispatchQueue.main.async(execute: work) + } + } + public var kmpVersion: String { LoggerBuildInfo.shared.KMP_VERSION } @@ -347,32 +373,57 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { exceptionMessage: String?, exceptionStacktrace: String? ) { - LogLoggingHelper.shared.log( - telemetry: telemetry, - level: level, - message: message, - exceptionType: exceptionType, - exceptionMessage: exceptionMessage, - exceptionStacktrace: exceptionStacktrace, - completionHandler: { _ in } - ) + guard lifecycle.canEmit else { + return + } + let telemetry = self.telemetry + let lifecycle = self.lifecycle + Self.onMain { + // Re-checked here because the hop is asynchronous: teardown can begin + // between the caller-side check above and the crossing into KMP. + guard lifecycle.canEmit else { + return + } + LogLoggingHelper.shared.log( + telemetry: telemetry, + level: level, + message: message, + exceptionType: exceptionType, + exceptionMessage: exceptionMessage, + exceptionStacktrace: exceptionStacktrace, + completionHandler: { _ in } + ) + } } public func forceFlush(completion: @escaping () -> Void) { - telemetry.forceFlush(completionHandler: { _ in completion() }) + let telemetry = self.telemetry + Self.onMain { + telemetry.forceFlush(completionHandler: { _ in completion() }) + } } public func shutdown() { lifecycleOperationLock.lock() - defer { lifecycleOperationLock.unlock() } guard lifecycle.beginShutdown() else { + lifecycleOperationLock.unlock() return } OSCrashUploaderCoordinator.shared.cancel(owner: uploaderOwner) crashHandler.unregister() - telemetry.shutdown() - lifecycle.finishShutdown() + lifecycleOperationLock.unlock() + + // `telemetry.shutdown()` blocks for up to five seconds draining buffered + // records, and callers reach here from app launch and app-id changes, where + // that would stall the UI. `beginShutdown()` has already closed the door on + // new records, so the drain can finish on its own thread. Unregistering the + // crash handler stays synchronous above: a later logger cannot install its + // handler while this one is still registered. + Self.teardownQueue.async { [self] in + telemetry.shutdown() + lifecycle.finishShutdown() + } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift index 878bdc639..ba930d0dd 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift @@ -44,6 +44,28 @@ final class OSLoggerAdaptersTests: XCTestCase { try? FileManager.default.removeItem(at: temporaryDirectory) } + func testLifecycleRejectsStartOnceShutdownHasBegun() { + let lifecycle = OSRemoteLoggerLifecycle() + + XCTAssertTrue(lifecycle.beginShutdown()) + // The final drain is deferred, so `isShutdown` is still false at this point. + // Starting anyway would register a crash handler that nothing unregisters, + // permanently blocking every later logger from installing its own. + XCTAssertFalse(lifecycle.start()) + XCTAssertFalse(lifecycle.canEmit) + } + + func testLifecycleStopsAcceptingRecordsWhenShutdownBegins() { + let lifecycle = OSRemoteLoggerLifecycle() + + XCTAssertTrue(lifecycle.start()) + XCTAssertTrue(lifecycle.canEmit) + + XCTAssertTrue(lifecycle.beginShutdown()) + XCTAssertFalse(lifecycle.canEmit) + XCTAssertFalse(lifecycle.start()) + } + func testFileStoreSynchronouslySavesAndListsPayload() throws { let store = FileLogStore(rootPath: temporaryDirectory.path) let payload = makeKotlinBytes([1, 2, 3, 255]) From 86b73638177a706c719f6cf6f6351f6766c58332 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 21 Aug 2026 15:31:59 -0500 Subject: [PATCH 3/6] fix: [SDK-5047] address review feedback on logging lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Serializes forceFlush against teardown. The deferred drain in shutdown() also flushes the same telemetry, so entering KMP from both at once was unsafe; the flush is now skipped once teardown begins. Its completion still runs in every case, because callers end a UIBackgroundTask in it and swallowing it would leak that task. The gating predicate is renamed isActive and now covers emission, uploader start, and flushes alike, since all three cross the same boundary. Normalizes a cached config that claims to be enabled without a level this SDK can parse — possible when a newer build wrote the cache. Android falls back to ERROR when enabling without a level; matching that avoids starting a logger that is enabled yet can never export and reports no level into KMP. Also strips line-number artifacts that leaked into the new file's license header, and splits the test file's configuration cases into their own suite so the controller suite stops exceeding the type_body_length error threshold. That was the sole error-severity swiftlint violation in the repo and the reason the lint job failed; the remaining violations in these files are file_length warnings, which the linter tolerates. Co-authored-by: Cursor --- .../Source/Logging/OSRemoteLogger.swift | 26 +- .../OSLoggerAdaptersTests.swift | 10 +- .../Source/OSRemoteLoggingConfiguration.swift | 9 +- .../Source/OSRemoteLoggingController.swift | 89 ++--- .../OSRemoteLoggingControllerTests.swift | 325 ++++++++++-------- 5 files changed, 261 insertions(+), 198 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift index b840d185d..a4769a7b9 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift @@ -60,11 +60,14 @@ final class OSRemoteLoggerLifecycle { private var isShuttingDown = false private var isShutdown = false - var canStartUploader: Bool { canEmit } - - /// Records stop being accepted the moment shutdown begins, rather than once the - /// final drain finishes, so teardown never lets a late log escape. - var canEmit: Bool { + var canStartUploader: Bool { isActive } + + /// True while the transport is usable and teardown has not begun. Gates record + /// emission, uploader start, and explicit flushes alike: the drain inside + /// `shutdown()` also crosses into KMP, and two concurrent crossings are unsafe. + /// Keyed on shutdown *beginning* rather than finishing, because that drain is + /// asynchronous. + var isActive: Bool { lock.lock() defer { lock.unlock() } return isStarted && !isShuttingDown && !isShutdown @@ -373,7 +376,7 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { exceptionMessage: String?, exceptionStacktrace: String? ) { - guard lifecycle.canEmit else { + guard lifecycle.isActive else { return } let telemetry = self.telemetry @@ -381,7 +384,7 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { Self.onMain { // Re-checked here because the hop is asynchronous: teardown can begin // between the caller-side check above and the crossing into KMP. - guard lifecycle.canEmit else { + guard lifecycle.isActive else { return } LogLoggingHelper.shared.log( @@ -398,7 +401,16 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { public func forceFlush(completion: @escaping () -> Void) { let telemetry = self.telemetry + let lifecycle = self.lifecycle Self.onMain { + // Skipped once teardown has begun, because `shutdown()`'s deferred drain + // flushes the same telemetry and both would cross into KMP at once. The + // completion still has to run either way: callers end a background task + // in it, and swallowing it would leak that task. + guard lifecycle.isActive else { + completion() + return + } telemetry.forceFlush(completionHandler: { _ in completion() }) } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift index ba930d0dd..6cd8a263f 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift @@ -52,17 +52,21 @@ final class OSLoggerAdaptersTests: XCTestCase { // Starting anyway would register a crash handler that nothing unregisters, // permanently blocking every later logger from installing its own. XCTAssertFalse(lifecycle.start()) - XCTAssertFalse(lifecycle.canEmit) + XCTAssertFalse(lifecycle.isActive) } func testLifecycleStopsAcceptingRecordsWhenShutdownBegins() { let lifecycle = OSRemoteLoggerLifecycle() XCTAssertTrue(lifecycle.start()) - XCTAssertTrue(lifecycle.canEmit) + XCTAssertTrue(lifecycle.isActive) + XCTAssertTrue(lifecycle.canStartUploader) + // Gates emission, uploader start, and explicit flushes together, so the + // deferred drain in shutdown() is the only thing still crossing into KMP. XCTAssertTrue(lifecycle.beginShutdown()) - XCTAssertFalse(lifecycle.canEmit) + XCTAssertFalse(lifecycle.isActive) + XCTAssertFalse(lifecycle.canStartUploader) XCTAssertFalse(lifecycle.start()) } diff --git a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift index e5f771a00..ba40a53f3 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift +++ b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift @@ -7,7 +7,7 @@ of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - 10| copies of the Software, and to permit persons to whom the Software is + copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: 1. The above copyright notice and this permission notice shall be included in @@ -17,7 +17,7 @@ with services provided by OneSignal. THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - 20| IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, @@ -72,7 +72,10 @@ struct OSRemoteLoggingConfiguration: Equatable { .map { $0.uppercased() } .flatMap(Self.oneSignalLevel) let isEnabled = (cached[Self.cachedIsEnabledKey] as? Bool) ?? (parsed != nil) - self.init(logLevel: parsed, isEnabled: isEnabled) + // A cache written by a newer SDK can name a level this one cannot parse. + // Android falls back to ERROR when enabling without a level; matching that + // avoids starting a logger that is enabled yet can never export anything. + self.init(logLevel: parsed ?? (isEnabled ? .LL_ERROR : nil), isEnabled: isEnabled) } func cachePayload(appId: String) -> [String: Any] { diff --git a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift index 3ac1b8853..c983d1957 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift +++ b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift @@ -87,14 +87,14 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { super.init() } - @objc class func configure() { + @objc static func configure() { let configuration = OSRemoteLoggingConfiguration.current cache(configuration: configuration) shared.configure(with: configuration) } @objc(configureFromCacheForAppId:) - class func configureFromCache(appId: String?) { + static func configureFromCache(appId: String?) { guard let appId, let cached = OneSignalUserDefaults.initStandard().getSavedDictionary( forKey: cachedConfigurationKey, @@ -107,11 +107,11 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { shared.configure(with: OSRemoteLoggingConfiguration(cached: cached)) } - @objc class func reset() { + @objc static func reset() { shared.shutdown() } - private class func cache(configuration: OSRemoteLoggingConfiguration) { + private static func cache(configuration: OSRemoteLoggingConfiguration) { guard let appId = OneSignalIdentifiers.currentAppId else { return } @@ -167,6 +167,45 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { private func configure(with newConfiguration: OSRemoteLoggingConfiguration) { updateAppState(Self.currentApplicationState()) + guard let startGeneration = applyConfiguration(newConfiguration) else { + return + } + + let providers = makeProviders(configuration: newConfiguration) + let newRemoteLogger = Self.onMain { + remoteLoggerFactory(providers) + } + var installed = false + stateQueue.sync { + guard self.configurationGeneration == startGeneration, + self.configuration.matches(newConfiguration), + self.remoteLogger == nil else { + return + } + self.remoteLogger = newRemoteLogger + installed = true + } + guard installed else { + newRemoteLogger.shutdown() + return + } + + newRemoteLogger.start() + logStartupDiagnostic(remoteLogger: newRemoteLogger) + stateQueue.sync { + guard self.remoteLogger === newRemoteLogger, + self.configuration.matches(newConfiguration) else { + return + } + OneSignalLog.__setInternalLogSink(self) + self.registerLifecycleObservers() + } + } + + /// Applies `newConfiguration` and tears down the running logger when the evaluated + /// action calls for it. Returns the generation a replacement logger should be built + /// for, or nil when the current state already satisfies the configuration. + private func applyConfiguration(_ newConfiguration: OSRemoteLoggingConfiguration) -> Int? { var startGeneration: Int? stateQueue.sync { self.configurationGeneration += 1 @@ -200,40 +239,7 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { } } } - - guard let startGeneration else { - return - } - - let providers = makeProviders(configuration: newConfiguration) - let newRemoteLogger = Self.onMain { - remoteLoggerFactory(providers) - } - var installed = false - stateQueue.sync { - guard self.configurationGeneration == startGeneration, - self.configuration.matches(newConfiguration), - self.remoteLogger == nil else { - return - } - self.remoteLogger = newRemoteLogger - installed = true - } - guard installed else { - newRemoteLogger.shutdown() - return - } - - newRemoteLogger.start() - logStartupDiagnostic(remoteLogger: newRemoteLogger) - stateQueue.sync { - guard self.remoteLogger === newRemoteLogger, - self.configuration.matches(newConfiguration) else { - return - } - OneSignalLog.__setInternalLogSink(self) - self.registerLifecycleObservers() - } + return startGeneration } private func stopRemoteLogging() { @@ -245,7 +251,12 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { activeRemoteLogger?.shutdown() } - private func registerLifecycleObservers() { +} + +// MARK: - App state, lifecycle observers, and thread helpers + +private extension OSRemoteLoggingController { + func registerLifecycleObservers() { if usesScenes() { observe(Notification.Name("UISceneDidActivateNotification"), appState: "foreground") observe(Notification.Name("UISceneWillDeactivateNotification"), appState: "unknown") diff --git a/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift b/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift index b948322cf..a96dac7d0 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift +++ b/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift @@ -40,58 +40,13 @@ final class OSRemoteLoggingControllerTests: XCTestCase { super.tearDown() } - func testConfigurationUsesRemoteLogLevel() { - let enabled = OSRemoteLoggingConfiguration( - remoteParams: ["logging_config": ["log_level": "warn"]] - ) - XCTAssertTrue(enabled.isEnabled) - XCTAssertEqual(enabled.logLevel, .LL_WARN) - XCTAssertTrue(enabled.allows(.LL_ERROR)) - XCTAssertTrue(enabled.allows(.LL_WARN)) - XCTAssertFalse(enabled.allows(.LL_INFO)) - - let invalidLevel = OSRemoteLoggingConfiguration( - remoteParams: ["logging_config": ["log_level": "OFF"]] - ) - XCTAssertFalse(invalidLevel.isEnabled) - XCTAssertNil(invalidLevel.logLevel) - } - - func testIosParamsPayloadEnablesVerboseLevelLogging() { - let configuration = OSRemoteLoggingConfiguration(remoteParams: Self.iosParamsPayload) - - XCTAssertTrue(configuration.isEnabled) - XCTAssertEqual(configuration.logLevel, .LL_VERBOSE) - XCTAssertEqual(configuration.logLevelName, "VERBOSE") - - // VERBOSE is the most permissive level, so every severity is exported. - for level in [ONE_S_LOG_LEVEL.LL_FATAL, .LL_ERROR, .LL_WARN, .LL_INFO, .LL_DEBUG, .LL_VERBOSE] { - XCTAssertTrue(configuration.allows(level)) - } - XCTAssertFalse(configuration.allows(.LL_NONE)) - - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(.disabled), - new: OSRemoteLoggingConfig(configuration) - ), - .enable(.LL_VERBOSE) - ) - - let restored = OSRemoteLoggingConfiguration( - cached: configuration.cachePayload(appId: "app-id") - ) - XCTAssertEqual(restored.logLevel, .LL_VERBOSE) - XCTAssertTrue(restored.isEnabled) - } - func testIosParamsPayloadRoutesEverySeverityThroughController() { let telemetry = RemoteTelemetrySpy() telemetry.emitExpectation = expectation(description: "routes verbose from ios_params payload") telemetry.emitExpectation?.expectedFulfillmentCount = 2 let controller = makeController(remoteLoggerFactory: { _ in telemetry }) - controller.configure(remoteParams: Self.iosParamsPayload) + controller.configure(remoteParams: Fixtures.iosParamsPayload) OneSignalLog.onesignalLog(.LL_VERBOSE, message: "verbose is exported") OneSignalLog.onesignalLog(.LL_ERROR, message: "error is exported") @@ -100,92 +55,6 @@ final class OSRemoteLoggingControllerTests: XCTestCase { XCTAssertEqual(telemetry.levels, ["VERBOSE", "ERROR"]) } - func testNoneLogLevelEnablesRemoteLoggingButDoesNotSend() { - let none = OSRemoteLoggingConfiguration( - remoteParams: ["logging_config": ["log_level": "NONE"]] - ) - XCTAssertTrue(none.isEnabled) - XCTAssertEqual(none.logLevel, .LL_NONE) - XCTAssertFalse(none.allows(.LL_FATAL)) - XCTAssertFalse(none.allows(.LL_ERROR)) - } - - func testCachePersistsLogLevelAndEnabledFlag() { - let enabled = OSRemoteLoggingConfiguration( - remoteParams: ["logging_config": ["log_level": "ERROR"]] - ) - let payload = enabled.cachePayload(appId: "app-id") - XCTAssertEqual(payload["app_id"] as? String, "app-id") - XCTAssertEqual(payload["log_level"] as? String, "ERROR") - XCTAssertEqual(payload["is_enabled"] as? Bool, true) - - let restored = OSRemoteLoggingConfiguration(cached: payload) - XCTAssertTrue(restored.isEnabled) - XCTAssertEqual(restored.logLevel, .LL_ERROR) - } - - func testLegacyCacheWithoutIsEnabledUsesLogLevelPresence() { - let legacyEnabled = OSRemoteLoggingConfiguration(cached: ["log_level": "WARN"]) - XCTAssertTrue(legacyEnabled.isEnabled) - XCTAssertEqual(legacyEnabled.logLevel, .LL_WARN) - - let legacyDisabled = OSRemoteLoggingConfiguration(cached: ["app_id": "app-id"]) - XCTAssertFalse(legacyDisabled.isEnabled) - XCTAssertNil(legacyDisabled.logLevel) - } - - func testEvaluatorMirrorsAndroidOtelConfigEvaluator() { - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: nil, - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) - ), - .enable(.LL_WARN) - ) - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: nil, - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: nil) - ), - .enable(.LL_ERROR) - ) - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: nil, - new: OSRemoteLoggingConfig.disabled - ), - .noChange - ) - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig.disabled, - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_INFO) - ), - .enable(.LL_INFO) - ) - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), - new: OSRemoteLoggingConfig.disabled - ), - .disable - ) - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) - ), - .updateLogLevel(old: .LL_ERROR, new: .LL_WARN) - ) - XCTAssertEqual( - OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR) - ), - .noChange - ) - } - func testControllerRoutesLogsAndFlushesOnBackground() { let notificationCenter = NotificationCenter() let telemetry = RemoteTelemetrySpy() @@ -225,7 +94,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { let telemetry = RemoteTelemetrySpy() telemetry.emitExpectation = expectation(description: "routes warning") let controller = makeController(remoteLoggerFactory: { _ in telemetry }) - controller.configure(remoteParams: Self.remoteParams(level: "WARN")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "WARN")) OneSignalLog.onesignalLog(.LL_WARN, message: "warning body") @@ -238,7 +107,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { let telemetry = RemoteTelemetrySpy() telemetry.emitExpectation = expectation(description: "routes structured exception") let controller = makeController(remoteLoggerFactory: { _ in telemetry }) - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) controller.captureLog( with: .LL_ERROR, @@ -257,7 +126,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { func testDisablingConfigurationStopsRemoteLogging() { let telemetry = RemoteTelemetrySpy() let controller = makeController(remoteLoggerFactory: { _ in telemetry }) - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) controller.configure(remoteParams: [:]) telemetry.emitExpectation = expectation(description: "does not route after disable") telemetry.emitExpectation?.isInverted = true @@ -274,7 +143,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { telemetry.emitExpectation = expectation(description: "does not route at NONE") telemetry.emitExpectation?.isInverted = true let controller = makeController(remoteLoggerFactory: { _ in telemetry }) - controller.configure(remoteParams: Self.remoteParams(level: "NONE")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "NONE")) OneSignalLog.onesignalLog(.LL_ERROR, message: "not uploaded") @@ -294,8 +163,8 @@ final class OSRemoteLoggingControllerTests: XCTestCase { return logger } - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) - controller.configure(remoteParams: Self.remoteParams(level: "WARN")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "WARN")) // Android's updateLogLevel shuts the previous telemetry down and builds a new // one. Rebuilding is what keeps the level reported into KMP in step with the @@ -324,7 +193,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { notificationCenter: notificationCenter, remoteLoggerFactory: { _ in telemetry } ) - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) notificationCenter.post(name: UIApplication.willTerminateNotification, object: nil) @@ -340,7 +209,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { usesScenes: { true }, remoteLoggerFactory: { _ in telemetry } ) - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) telemetry.flushExpectation = expectation(description: "ignores application background") telemetry.flushExpectation?.isInverted = true @@ -363,7 +232,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { } DispatchQueue.global().async { - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) configured.fulfill() } @@ -379,12 +248,12 @@ final class OSRemoteLoggingControllerTests: XCTestCase { loggers.append(logger) if !didReenter { didReenter = true - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) } return logger } - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) XCTAssertEqual(loggers.count, 2) XCTAssertEqual(loggers.map(\.startCount).reduce(0, +), 1) @@ -402,7 +271,7 @@ final class OSRemoteLoggingControllerTests: XCTestCase { OneSignalLog.debug().__add(listener) defer { OneSignalLog.debug().__remove(listener) } - controller.configure(remoteParams: Self.remoteParams(level: "ERROR")) + controller.configure(remoteParams: Fixtures.remoteParams(level: "ERROR")) wait(for: [reset], timeout: 2) XCTAssertEqual(telemetry.shutdownCount, 1) @@ -426,13 +295,17 @@ final class OSRemoteLoggingControllerTests: XCTestCase { return controller } - private static func remoteParams(level: String) -> [String: Any] { +} + +/// Shared by the configuration and controller suites. +private enum Fixtures { + static func remoteParams(level: String) -> [String: Any] { ["logging_config": ["log_level": level]] } /// Verbatim ios_params response, so parsing stays honest about the real shape /// rather than only the trimmed dictionaries the other tests use. - private static let iosParamsPayload: [String: Any] = [ + static let iosParamsPayload: [String: Any] = [ "fba": true, "uses_provisional_auth": true, "outcomes": [ @@ -448,6 +321,166 @@ final class OSRemoteLoggingControllerTests: XCTestCase { ] } +final class OSRemoteLoggingConfigurationTests: XCTestCase { + func testConfigurationUsesRemoteLogLevel() { + let enabled = OSRemoteLoggingConfiguration( + remoteParams: ["logging_config": ["log_level": "warn"]] + ) + XCTAssertTrue(enabled.isEnabled) + XCTAssertEqual(enabled.logLevel, .LL_WARN) + XCTAssertTrue(enabled.allows(.LL_ERROR)) + XCTAssertTrue(enabled.allows(.LL_WARN)) + XCTAssertFalse(enabled.allows(.LL_INFO)) + + let invalidLevel = OSRemoteLoggingConfiguration( + remoteParams: ["logging_config": ["log_level": "OFF"]] + ) + XCTAssertFalse(invalidLevel.isEnabled) + XCTAssertNil(invalidLevel.logLevel) + } + + func testIosParamsPayloadEnablesVerboseLevelLogging() { + let configuration = OSRemoteLoggingConfiguration(remoteParams: Fixtures.iosParamsPayload) + + XCTAssertTrue(configuration.isEnabled) + XCTAssertEqual(configuration.logLevel, .LL_VERBOSE) + XCTAssertEqual(configuration.logLevelName, "VERBOSE") + + // VERBOSE is the most permissive level, so every severity is exported. + for level in [ONE_S_LOG_LEVEL.LL_FATAL, .LL_ERROR, .LL_WARN, .LL_INFO, .LL_DEBUG, .LL_VERBOSE] { + XCTAssertTrue(configuration.allows(level)) + } + XCTAssertFalse(configuration.allows(.LL_NONE)) + + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(.disabled), + new: OSRemoteLoggingConfig(configuration) + ), + .enable(.LL_VERBOSE) + ) + + let restored = OSRemoteLoggingConfiguration( + cached: configuration.cachePayload(appId: "app-id") + ) + XCTAssertEqual(restored.logLevel, .LL_VERBOSE) + XCTAssertTrue(restored.isEnabled) + } + + func testNoneLogLevelEnablesRemoteLoggingButDoesNotSend() { + let none = OSRemoteLoggingConfiguration( + remoteParams: ["logging_config": ["log_level": "NONE"]] + ) + XCTAssertTrue(none.isEnabled) + XCTAssertEqual(none.logLevel, .LL_NONE) + XCTAssertFalse(none.allows(.LL_FATAL)) + XCTAssertFalse(none.allows(.LL_ERROR)) + } + + func testCachePersistsLogLevelAndEnabledFlag() { + let enabled = OSRemoteLoggingConfiguration( + remoteParams: ["logging_config": ["log_level": "ERROR"]] + ) + let payload = enabled.cachePayload(appId: "app-id") + XCTAssertEqual(payload["app_id"] as? String, "app-id") + XCTAssertEqual(payload["log_level"] as? String, "ERROR") + XCTAssertEqual(payload["is_enabled"] as? Bool, true) + + let restored = OSRemoteLoggingConfiguration(cached: payload) + XCTAssertTrue(restored.isEnabled) + XCTAssertEqual(restored.logLevel, .LL_ERROR) + } + + func testLegacyCacheWithoutIsEnabledUsesLogLevelPresence() { + let legacyEnabled = OSRemoteLoggingConfiguration(cached: ["log_level": "WARN"]) + XCTAssertTrue(legacyEnabled.isEnabled) + XCTAssertEqual(legacyEnabled.logLevel, .LL_WARN) + + let legacyDisabled = OSRemoteLoggingConfiguration(cached: ["app_id": "app-id"]) + XCTAssertFalse(legacyDisabled.isEnabled) + XCTAssertNil(legacyDisabled.logLevel) + } + + /// A cache written by a newer SDK can name a level this one cannot parse. Without + /// normalizing, the config would be enabled with a nil level, which starts a + /// logger that can never export and reports no level into KMP. + func testCachedEnabledWithUnparseableLevelFallsBackToError() { + let configuration = OSRemoteLoggingConfiguration( + cached: ["app_id": "app-id", "log_level": "TRACE_ALL_THE_THINGS", "is_enabled": true] + ) + + XCTAssertTrue(configuration.isEnabled) + XCTAssertEqual(configuration.logLevel, .LL_ERROR) + XCTAssertTrue(configuration.allows(.LL_ERROR)) + XCTAssertFalse(configuration.allows(.LL_WARN)) + + // Absent entirely, rather than unparseable, behaves the same way. + let missingLevel = OSRemoteLoggingConfiguration( + cached: ["app_id": "app-id", "is_enabled": true] + ) + XCTAssertEqual(missingLevel.logLevel, .LL_ERROR) + + // But an enabled flag of false must not manufacture a level. + let disabled = OSRemoteLoggingConfiguration( + cached: ["app_id": "app-id", "is_enabled": false] + ) + XCTAssertFalse(disabled.isEnabled) + XCTAssertNil(disabled.logLevel) + } + + func testEvaluatorMirrorsAndroidOtelConfigEvaluator() { + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: nil, + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) + ), + .enable(.LL_WARN) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: nil, + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: nil) + ), + .enable(.LL_ERROR) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: nil, + new: OSRemoteLoggingConfig.disabled + ), + .noChange + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig.disabled, + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_INFO) + ), + .enable(.LL_INFO) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), + new: OSRemoteLoggingConfig.disabled + ), + .disable + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) + ), + .updateLogLevel(old: .LL_ERROR, new: .LL_WARN) + ) + XCTAssertEqual( + OSRemoteLoggingConfigEvaluator.evaluate( + old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), + new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR) + ), + .noChange + ) + } +} + private final class ReentrantLogListener: NSObject, OSLogListener { private let onLog: () -> Void From e9007323bf78ef300f83dba7c9b5c69f6c1f7306 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 24 Aug 2026 09:43:23 -0500 Subject: [PATCH 4/6] fix: [SDK-5047] serialize teardown against an in-flight flush Gating forceFlush on a flag was not enough. The flush is asynchronous, so shutdown could begin after the check passed and run its drain over the same telemetry while the flush was still crossing into KMP. Flushes now claim a slot on the lifecycle, and shutdown waits for outstanding slots before draining. `beginFlush` refuses new flushes once shutdown starts, so the set the drain waits on can only shrink. The wait is bounded at five seconds, matching the cap KMP already puts on its own drain, so a wedged flush cannot stop teardown from completing. Blocking is safe there because it runs on the teardown queue while the KMP completion that releases the slot resumes on main. Co-authored-by: Cursor --- .../Source/Logging/OSRemoteLogger.swift | 62 ++++++++++++++++--- .../OSLoggerAdaptersTests.swift | 35 +++++++++++ 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift index a4769a7b9..42580b020 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift @@ -55,10 +55,12 @@ public extension OSRemoteLoggerProtocol { @_implementationOnly import OneSignalKMP final class OSRemoteLoggerLifecycle { - private let lock = NSLock() + /// A condition rather than a plain lock so teardown can wait on in-flight flushes. + private let lock = NSCondition() private var isStarted = false private var isShuttingDown = false private var isShutdown = false + private var activeFlushes = 0 var canStartUploader: Bool { isActive } @@ -96,6 +98,38 @@ final class OSRemoteLoggerLifecycle { return true } + /// Claims a flush slot, so teardown can tell a flush is still crossing into KMP. + /// Returns false once shutdown has begun, meaning the caller must not cross. + func beginFlush() -> Bool { + lock.lock() + defer { lock.unlock() } + guard isStarted, !isShuttingDown, !isShutdown else { + return false + } + activeFlushes += 1 + return true + } + + func endFlush() { + lock.lock() + activeFlushes -= 1 + if activeFlushes == 0 { + lock.broadcast() + } + lock.unlock() + } + + /// Blocks until flushes admitted before shutdown began have finished, so the + /// teardown drain never overlaps one. `beginFlush` already refuses new flushes by + /// this point, so the set can only shrink. Bounded, because a wedged flush must + /// not stop teardown from completing. + func waitForFlushesToDrain(timeout: TimeInterval) { + let deadline = Date().addingTimeInterval(timeout) + lock.lock() + defer { lock.unlock() } + while activeFlushes > 0, lock.wait(until: deadline) {} + } + func beginShutdown() -> Bool { lock.lock() defer { lock.unlock() } @@ -183,6 +217,10 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { /// Serial so overlapping teardowns cannot stack several bounded drains at once. private static let teardownQueue = DispatchQueue(label: "com.onesignal.logger.remote-teardown") + /// Matches the bound KMP puts on its own shutdown drain, so a wedged flush delays + /// teardown by no more than the drain itself already can. + private static let flushDrainTimeout: TimeInterval = 5 + public convenience init( installIdProvider: @escaping () -> String, onesignalIdProvider: @escaping () -> String?, @@ -403,15 +441,20 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { let telemetry = self.telemetry let lifecycle = self.lifecycle Self.onMain { - // Skipped once teardown has begun, because `shutdown()`'s deferred drain - // flushes the same telemetry and both would cross into KMP at once. The - // completion still has to run either way: callers end a background task - // in it, and swallowing it would leak that task. - guard lifecycle.isActive else { + // Claiming a slot rather than just testing a flag: the flush below is + // asynchronous, so shutdown could otherwise begin after the check passed + // and drain the same telemetry concurrently. `shutdown()` waits for the + // slot to be released. The completion still has to run on every path — + // callers end a background task in it, and swallowing it would leak that + // task. + guard lifecycle.beginFlush() else { completion() return } - telemetry.forceFlush(completionHandler: { _ in completion() }) + telemetry.forceFlush(completionHandler: { _ in + lifecycle.endFlush() + completion() + }) } } @@ -433,6 +476,11 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { // crash handler stays synchronous above: a later logger cannot install its // handler while this one is still registered. Self.teardownQueue.async { [self] in + // A flush admitted just before `beginShutdown()` may still be crossing + // into KMP; the drain below would otherwise run alongside it. Safe to + // block here: this is a background queue, and the KMP completion that + // releases the slot resumes on main. + lifecycle.waitForFlushesToDrain(timeout: Self.flushDrainTimeout) telemetry.shutdown() lifecycle.finishShutdown() } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift index 6cd8a263f..cd9205e53 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift @@ -55,6 +55,41 @@ final class OSLoggerAdaptersTests: XCTestCase { XCTAssertFalse(lifecycle.isActive) } + func testLifecycleShutdownWaitsForInFlightFlush() { + let lifecycle = OSRemoteLoggerLifecycle() + XCTAssertTrue(lifecycle.start()) + XCTAssertTrue(lifecycle.beginFlush()) + XCTAssertTrue(lifecycle.beginShutdown()) + + // A flush admitted before shutdown began still holds its slot, so teardown + // must not proceed to the drain while it is outstanding. + let drained = DispatchSemaphore(value: 0) + DispatchQueue.global().async { + lifecycle.waitForFlushesToDrain(timeout: 5) + drained.signal() + } + XCTAssertEqual(drained.wait(timeout: .now() + 0.3), .timedOut) + + lifecycle.endFlush() + XCTAssertEqual(drained.wait(timeout: .now() + 2), .success) + } + + func testLifecycleRefusesNewFlushOnceShutdownBegins() { + let lifecycle = OSRemoteLoggerLifecycle() + XCTAssertTrue(lifecycle.start()) + XCTAssertTrue(lifecycle.beginShutdown()) + + // Nothing new may cross into KMP, so the drain has a shrinking set to wait on. + XCTAssertFalse(lifecycle.beginFlush()) + + let drained = DispatchSemaphore(value: 0) + DispatchQueue.global().async { + lifecycle.waitForFlushesToDrain(timeout: 5) + drained.signal() + } + XCTAssertEqual(drained.wait(timeout: .now() + 2), .success) + } + func testLifecycleStopsAcceptingRecordsWhenShutdownBegins() { let lifecycle = OSRemoteLoggerLifecycle() From 020dbceec94454009e1189a620297b3f1b4031f4 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 24 Aug 2026 15:59:50 -0500 Subject: [PATCH 5/6] refactor: [SDK-5047] diff the config directly instead of a snapshot type OSRemoteLoggingConfig mirrored two fields of OSRemoteLoggingConfiguration and existed only to be constructed at the one call site that invokes the evaluator. The near-identical name made the pair easy to confuse for no benefit. Android keeps OtelConfig separate because its config lives in a persistence -backed Model that cannot be compared directly. OSRemoteLoggingConfiguration is already an Equatable value type, so the evaluator now diffs it as-is. Co-authored-by: Cursor --- .../Source/OSRemoteLoggingConfiguration.swift | 25 ++++-------------- .../Source/OSRemoteLoggingController.swift | 4 +-- .../OSRemoteLoggingControllerTests.swift | 26 +++++++++---------- 3 files changed, 20 insertions(+), 35 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift index ba40a53f3..b105f3127 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift +++ b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingConfiguration.swift @@ -141,24 +141,6 @@ struct OSRemoteLoggingConfiguration: Equatable { } } -/// Snapshot used by [OSRemoteLoggingConfigEvaluator], matching Android's `OtelConfig`. -struct OSRemoteLoggingConfig: Equatable { - let isEnabled: Bool - let logLevel: ONE_S_LOG_LEVEL? - - init(_ configuration: OSRemoteLoggingConfiguration) { - isEnabled = configuration.isEnabled - logLevel = configuration.logLevel - } - - init(isEnabled: Bool, logLevel: ONE_S_LOG_LEVEL?) { - self.isEnabled = isEnabled - self.logLevel = logLevel - } - - static let disabled = OSRemoteLoggingConfig(isEnabled: false, logLevel: nil) -} - enum OSRemoteLoggingConfigAction: Equatable { case noChange case enable(ONE_S_LOG_LEVEL) @@ -167,10 +149,13 @@ enum OSRemoteLoggingConfigAction: Equatable { } /// Pure diff of old vs new remote-logging config, mirroring Android's `OtelConfigEvaluator`. +/// Android diffs a separate `OtelConfig` snapshot because its config lives in a +/// persistence-backed `Model`; `OSRemoteLoggingConfiguration` is already a value type, +/// so it is diffed directly. enum OSRemoteLoggingConfigEvaluator { static func evaluate( - old: OSRemoteLoggingConfig?, - new: OSRemoteLoggingConfig + old: OSRemoteLoggingConfiguration?, + new: OSRemoteLoggingConfiguration ) -> OSRemoteLoggingConfigAction { let wasEnabled = old?.isEnabled == true let isNowEnabled = new.isEnabled diff --git a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift index c983d1957..5e89ba7fd 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift +++ b/iOS_SDK/OneSignalSDK/Source/OSRemoteLoggingController.swift @@ -211,8 +211,8 @@ final class OSRemoteLoggingController: NSObject, OSInternalLogSink { self.configurationGeneration += 1 let generation = self.configurationGeneration let action = OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(self.configuration), - new: OSRemoteLoggingConfig(newConfiguration) + old: self.configuration, + new: newConfiguration ) self.configuration = newConfiguration diff --git a/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift b/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift index a96dac7d0..7ba395a26 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift +++ b/iOS_SDK/OneSignalSDK/UnitTests/OSRemoteLoggingControllerTests.swift @@ -354,8 +354,8 @@ final class OSRemoteLoggingConfigurationTests: XCTestCase { XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(.disabled), - new: OSRemoteLoggingConfig(configuration) + old: OSRemoteLoggingConfiguration.disabled, + new: configuration ), .enable(.LL_VERBOSE) ) @@ -432,49 +432,49 @@ final class OSRemoteLoggingConfigurationTests: XCTestCase { XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( old: nil, - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) + new: OSRemoteLoggingConfiguration(logLevel: .LL_WARN, isEnabled: true) ), .enable(.LL_WARN) ) XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( old: nil, - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: nil) + new: OSRemoteLoggingConfiguration(logLevel: nil, isEnabled: true) ), .enable(.LL_ERROR) ) XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( old: nil, - new: OSRemoteLoggingConfig.disabled + new: OSRemoteLoggingConfiguration.disabled ), .noChange ) XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig.disabled, - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_INFO) + old: OSRemoteLoggingConfiguration.disabled, + new: OSRemoteLoggingConfiguration(logLevel: .LL_INFO, isEnabled: true) ), .enable(.LL_INFO) ) XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), - new: OSRemoteLoggingConfig.disabled + old: OSRemoteLoggingConfiguration(logLevel: .LL_ERROR, isEnabled: true), + new: OSRemoteLoggingConfiguration.disabled ), .disable ) XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_WARN) + old: OSRemoteLoggingConfiguration(logLevel: .LL_ERROR, isEnabled: true), + new: OSRemoteLoggingConfiguration(logLevel: .LL_WARN, isEnabled: true) ), .updateLogLevel(old: .LL_ERROR, new: .LL_WARN) ) XCTAssertEqual( OSRemoteLoggingConfigEvaluator.evaluate( - old: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR), - new: OSRemoteLoggingConfig(isEnabled: true, logLevel: .LL_ERROR) + old: OSRemoteLoggingConfiguration(logLevel: .LL_ERROR, isEnabled: true), + new: OSRemoteLoggingConfiguration(logLevel: .LL_ERROR, isEnabled: true) ), .noChange ) From 7771667ae6a89884bf357db897ceef14a60ceee9 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 24 Aug 2026 16:06:50 -0500 Subject: [PATCH 6/6] refactor: [SDK-5047] call KMP directly instead of marshalling onto main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The KMP module sets objcExportSuspendFunctionLaunchThreadRestriction=none, so exported suspend functions can be called from any thread. The hop these calls went through was unnecessary; the exception that prompted it almost certainly came from an XCFramework built before that flag landed. Removing it also closes the check-then-cross window in log(), since the lifecycle guard and the KMP call are now sequential on one thread, and drops a main-queue dispatch per exported record — which added up at verbose levels. A new test calls an exported suspend function from a background queue and asserts it completes, so this fails loudly if the flag is ever dropped. forceFlush keeps its slot tracking: the flush completes asynchronously even when started inline, so shutdown's drain could still overlap it. Also drops canStartUploader, which had become a second name for isActive with a single reader. Co-authored-by: Cursor --- .../Source/Logging/OSRemoteLogger.swift | 96 +++++++------------ .../OSLoggerAdaptersTests.swift | 29 +++++- 2 files changed, 59 insertions(+), 66 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift index 42580b020..74fbddcf1 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Logging/OSRemoteLogger.swift @@ -62,13 +62,10 @@ final class OSRemoteLoggerLifecycle { private var isShutdown = false private var activeFlushes = 0 - var canStartUploader: Bool { isActive } - /// True while the transport is usable and teardown has not begun. Gates record - /// emission, uploader start, and explicit flushes alike: the drain inside - /// `shutdown()` also crosses into KMP, and two concurrent crossings are unsafe. - /// Keyed on shutdown *beginning* rather than finishing, because that drain is - /// asynchronous. + /// emission, uploader start, and explicit flushes alike, so nothing new is accepted + /// once the SDK has been told to stop. Keyed on shutdown *beginning* rather than + /// finishing, because the final drain is asynchronous. var isActive: Bool { lock.lock() defer { lock.unlock() } @@ -363,32 +360,19 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { let logger = self.logger let lifecycle = self.lifecycle OSCrashUploaderCoordinator.shared.enqueue(owner: owner) { - Self.onMain { - guard lifecycle.canStartUploader else { - OSCrashUploaderCoordinator.shared.finish(owner: owner) - return - } - crashUploader.start { error in - if let error { - logger.error(message: "LogCrashUploader failed: \(error.localizedDescription)") - } - OSCrashUploaderCoordinator.shared.finish(owner: owner) + guard lifecycle.isActive else { + OSCrashUploaderCoordinator.shared.finish(owner: owner) + return + } + crashUploader.start { error in + if let error { + logger.error(message: "LogCrashUploader failed: \(error.localizedDescription)") } + OSCrashUploaderCoordinator.shared.finish(owner: owner) } } } - /// Kotlin/Native only supports calling exported `suspend` functions from the main - /// thread, so every crossing into KMP has to be marshalled here. Callers reach this - /// class from the logging controller's serial queue and from URLSession callbacks. - private static func onMain(_ work: @escaping () -> Void) { - if Thread.isMainThread { - work() - } else { - DispatchQueue.main.async(execute: work) - } - } - public var kmpVersion: String { LoggerBuildInfo.shared.KMP_VERSION } @@ -417,45 +401,33 @@ public final class OSRemoteLogger: OSRemoteLoggerProtocol { guard lifecycle.isActive else { return } - let telemetry = self.telemetry - let lifecycle = self.lifecycle - Self.onMain { - // Re-checked here because the hop is asynchronous: teardown can begin - // between the caller-side check above and the crossing into KMP. - guard lifecycle.isActive else { - return - } - LogLoggingHelper.shared.log( - telemetry: telemetry, - level: level, - message: message, - exceptionType: exceptionType, - exceptionMessage: exceptionMessage, - exceptionStacktrace: exceptionStacktrace, - completionHandler: { _ in } - ) - } + LogLoggingHelper.shared.log( + telemetry: telemetry, + level: level, + message: message, + exceptionType: exceptionType, + exceptionMessage: exceptionMessage, + exceptionStacktrace: exceptionStacktrace, + completionHandler: { _ in } + ) } public func forceFlush(completion: @escaping () -> Void) { - let telemetry = self.telemetry - let lifecycle = self.lifecycle - Self.onMain { - // Claiming a slot rather than just testing a flag: the flush below is - // asynchronous, so shutdown could otherwise begin after the check passed - // and drain the same telemetry concurrently. `shutdown()` waits for the - // slot to be released. The completion still has to run on every path — - // callers end a background task in it, and swallowing it would leak that - // task. - guard lifecycle.beginFlush() else { - completion() - return - } - telemetry.forceFlush(completionHandler: { _ in - lifecycle.endFlush() - completion() - }) + // Claiming a slot rather than just testing a flag: the flush completes + // asynchronously even when started inline, so shutdown could otherwise begin + // after the check passed and drain the same telemetry concurrently. + // `shutdown()` waits for the slot to be released. The completion still has to + // run on every path — callers end a background task in it, and swallowing it + // would leak that task. + guard lifecycle.beginFlush() else { + completion() + return } + let lifecycle = self.lifecycle + telemetry.forceFlush(completionHandler: { _ in + lifecycle.endFlush() + completion() + }) } public func shutdown() { diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift index cd9205e53..0b2f2c433 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSLoggerAdaptersTests.swift @@ -95,13 +95,11 @@ final class OSLoggerAdaptersTests: XCTestCase { XCTAssertTrue(lifecycle.start()) XCTAssertTrue(lifecycle.isActive) - XCTAssertTrue(lifecycle.canStartUploader) - // Gates emission, uploader start, and explicit flushes together, so the - // deferred drain in shutdown() is the only thing still crossing into KMP. + // Gates emission, uploader start, and explicit flushes together, so nothing + // new is accepted once the SDK has been told to stop. XCTAssertTrue(lifecycle.beginShutdown()) XCTAssertFalse(lifecycle.isActive) - XCTAssertFalse(lifecycle.canStartUploader) XCTAssertFalse(lifecycle.start()) } @@ -307,6 +305,29 @@ final class OSLoggerAdaptersTests: XCTestCase { XCTAssertTrue(lines.isEmpty) } + /// KMP #15 set `objcExportSuspendFunctionLaunchThreadRestriction=none`, lifting + /// Kotlin/Native's rule that exported `suspend` functions may only be called from + /// the main thread. Asserts that actually holds for the framework we link, so the + /// Swift side does not have to marshal every crossing onto main — and fails loudly + /// if that flag is ever dropped. + func testKmpSuspendCallSucceedsOffMainThread() { + let store = FileLogStore(rootPath: temporaryDirectory.path) + let telemetry = LoggerFactory.shared.createCrashLocalTelemetry( + platformProvider: makePlatformProvider(), + fileStore: store + ) + let completed = expectation(description: "KMP suspend call completes off main") + + DispatchQueue.global().async { + XCTAssertFalse(Thread.isMainThread) + telemetry.forceFlush { _ in + completed.fulfill() + } + } + + wait(for: [completed], timeout: 5) + } + func testKmpPipelineInvokesSwiftAdapters() throws { let listener = LoggerAdapterListener() OneSignalLog.debug().__add(listener)