From 2341545d149d67dd613830a4d0a9087e4ca7ec08 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 28 Nov 2022 17:40:36 +0100 Subject: [PATCH 1/5] ref: SentrySDK.close calls flush SentrySDK.close now calls flush and disables the client. --- CHANGELOG.md | 1 + Sources/Sentry/Public/SentryClient.h | 7 +++++ Sources/Sentry/Public/SentryHub.h | 5 ++++ Sources/Sentry/Public/SentryOptions.h | 5 ++++ Sources/Sentry/Public/SentrySDK.h | 3 +- Sources/Sentry/SentryClient.m | 9 +++++- Sources/Sentry/SentryHub.m | 8 ++++++ Sources/Sentry/SentryOptions.m | 1 + Sources/Sentry/SentrySDK.m | 5 +--- Tests/SentryTests/SentryClientTests.swift | 4 +++ Tests/SentryTests/SentryOptionsTest.m | 1 + Tests/SentryTests/SentrySDKTests.swift | 35 +++++++++++++++++++++++ 12 files changed, 78 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e73e3a0948..623935fbd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ This version adds a dependency on Swift. - Marks App hang's event stacktrace snapshot as true (#2441) - Enable user interaction tracing by default (#2442) - Remove default attachment content type (#2443) +- SentrySDK.close calls flush (#2452) ## 7.31.2 diff --git a/Sources/Sentry/Public/SentryClient.h b/Sources/Sentry/Public/SentryClient.h index ba2042b1d5..399e348b1e 100644 --- a/Sources/Sentry/Public/SentryClient.h +++ b/Sources/Sentry/Public/SentryClient.h @@ -8,6 +8,8 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryClient : NSObject SENTRY_NO_INIT +@property (nonatomic, assign, readonly) BOOL isEnabled; + @property (nonatomic, strong) SentryOptions *options; /** @@ -119,6 +121,11 @@ SENTRY_NO_INIT */ - (void)flush:(NSTimeInterval)timeout NS_SWIFT_NAME(flush(timeout:)); +/** + * Disables the client and calls flush with ``SentryOptions/shutdownTimeInterval``. + */ +- (void)close; + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/Public/SentryHub.h b/Sources/Sentry/Public/SentryHub.h index 9a5bb369b9..88609c6f4a 100644 --- a/Sources/Sentry/Public/SentryHub.h +++ b/Sources/Sentry/Public/SentryHub.h @@ -268,6 +268,11 @@ SENTRY_NO_INIT */ - (void)flush:(NSTimeInterval)timeout NS_SWIFT_NAME(flush(timeout:)); +/** + * Calls flush with ``SentryOptions/shutdownTimeInterval``. + */ +- (void)close; + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index 2dc4de2c39..c960d41e54 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -58,6 +58,11 @@ NS_SWIFT_NAME(Options) */ @property (nonatomic, assign) BOOL enabled; +/** + * Controls the flush duration when calling ``SentrySDK/close``. + */ +@property (nonatomic, assign) NSTimeInterval shutdownTimeInterval; + /** * When enabled, the SDK sends crashes to Sentry. Default value is YES. */ diff --git a/Sources/Sentry/Public/SentrySDK.h b/Sources/Sentry/Public/SentrySDK.h index 595bd6cea1..6ceee4f6c2 100644 --- a/Sources/Sentry/Public/SentrySDK.h +++ b/Sources/Sentry/Public/SentrySDK.h @@ -311,7 +311,8 @@ SENTRY_NO_INIT + (void)flush:(NSTimeInterval)timeout NS_SWIFT_NAME(flush(timeout:)); /** - * Closes the SDK and uninstalls all the integrations. + * Closes the SDK, uninstalls all the integrations, and calls flush with + * ``SentryOptions/shutdownTimeInterval``. */ + (void)close; diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 627b64fcab..4b0f6a4b1f 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -162,6 +162,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options timezone:(NSTimeZone *)timezone { if (self = [super init]) { + _isEnabled = YES; self.options = options; self.transportAdapter = transportAdapter; self.fileManager = fileManager; @@ -501,6 +502,12 @@ - (void)flush:(NSTimeInterval)timeout [self.transportAdapter flush:timeout]; } +- (void)close +{ + _isEnabled = NO; + [self flush:self.options.shutdownTimeInterval]; +} + - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event withScope:(SentryScope *)scope alwaysAttachStacktrace:(BOOL)alwaysAttachStacktrace @@ -632,7 +639,7 @@ - (BOOL)isSampled:(NSNumber *)sampleRate - (BOOL)isDisabled { - return !self.options.enabled || nil == self.options.parsedDsn; + return !_isEnabled || !self.options.enabled || nil == self.options.parsedDsn; } - (void)logDisabledMessage diff --git a/Sources/Sentry/SentryHub.m b/Sources/Sentry/SentryHub.m index 3b79664304..df5ef3e563 100644 --- a/Sources/Sentry/SentryHub.m +++ b/Sources/Sentry/SentryHub.m @@ -672,6 +672,14 @@ - (void)flush:(NSTimeInterval)timeout } } +- (void)close +{ + SentryClient *client = _client; + if (client != nil) { + [client close]; + } +} + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/SentryOptions.m b/Sources/Sentry/SentryOptions.m index b7565951b0..963047e9fd 100644 --- a/Sources/Sentry/SentryOptions.m +++ b/Sources/Sentry/SentryOptions.m @@ -47,6 +47,7 @@ - (instancetype)init { if (self = [super init]) { self.enabled = YES; + self.shutdownTimeInterval = 2.0; self.enableCrashHandler = YES; self.diagnosticLevel = kSentryLevelDebug; self.debug = NO; diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 2d036a9bb1..46dba45ed1 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -402,10 +402,7 @@ + (void)close } } [hub removeAllIntegrations]; - - // close the client - SentryClient *client = [hub getClient]; - client.options.enabled = NO; + [hub close]; [hub bindClient:nil]; [SentrySDK setCurrentHub:nil]; diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index ea4641930d..419496f6f7 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -134,6 +134,10 @@ class SentryClientTest: XCTestCase { clearTestState() } + func testClientIsEnabled() { + XCTAssertTrue(fixture.getSut().isEnabled) + } + func testCaptureMessage() { let eventId = fixture.getSut().capture(message: fixture.messageAsString) diff --git a/Tests/SentryTests/SentryOptionsTest.m b/Tests/SentryTests/SentryOptionsTest.m index c25fab5eae..710b65059e 100644 --- a/Tests/SentryTests/SentryOptionsTest.m +++ b/Tests/SentryTests/SentryOptionsTest.m @@ -78,6 +78,7 @@ - (void)testEmptyConstructorSetsDefaultValues - (void)assertDefaultValues:(SentryOptions *)options { XCTAssertEqual(YES, options.enabled); + XCTAssertEqual(2.0, options.shutdownTimeInterval); XCTAssertEqual(NO, options.debug); XCTAssertEqual(kSentryLevelDebug, options.diagnosticLevel); XCTAssertEqual(options.environment, kSentryDefaultEnvironment); diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 2012647357..f47cf2c87f 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -493,6 +493,41 @@ class SentrySDKTests: XCTestCase { assertIntegrationsInstalled(integrations: []) } + func testClose_SetsClientToNil() { + SentrySDK.start { options in + options.dsn = SentrySDKTests.dsnAsString + } + + SentrySDK.close() + + XCTAssertNil(SentrySDK.currentHub().client()) + } + + func testClose_ClosesClient() { + SentrySDK.start { options in + options.dsn = SentrySDKTests.dsnAsString + } + + let client = SentrySDK.currentHub().client() + SentrySDK.close() + + XCTAssertFalse(client?.isEnabled ?? true) + } + + func testClose_CallsFlushCorrectlyOnTransport() { + SentrySDK.start { options in + options.dsn = SentrySDKTests.dsnAsString + } + + let transport = TestTransport() + let client = SentryClient(options: fixture.options) + Dynamic(client).transportAdapter = TestTransportAdapter(transport: transport, options: fixture.options) + SentrySDK.currentHub().bindClient(client) + SentrySDK.close() + + XCTAssertEqual(Options().shutdownTimeInterval, transport.flushInvocations.first) + } + func testFlush_CallsFlushCorrectlyOnTransport() { SentrySDK.start { options in options.dsn = SentrySDKTests.dsnAsString From b3007a9e63343544cb12ff6ca206ffc7e79e9fa4 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 29 Nov 2022 10:15:42 +0100 Subject: [PATCH 2/5] Update Sources/Sentry/SentryHub.m Co-authored-by: Dhiogo Brustolin --- Sources/Sentry/SentryHub.m | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Sources/Sentry/SentryHub.m b/Sources/Sentry/SentryHub.m index df5ef3e563..aa9bd29791 100644 --- a/Sources/Sentry/SentryHub.m +++ b/Sources/Sentry/SentryHub.m @@ -674,10 +674,7 @@ - (void)flush:(NSTimeInterval)timeout - (void)close { - SentryClient *client = _client; - if (client != nil) { - [client close]; - } + [_client close]; } @end From 64e0d5e070ee81d04e50790fdd06f1c85ab7a3ef Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 29 Nov 2022 10:55:01 +0100 Subject: [PATCH 3/5] Update CHANGELOG.md Co-authored-by: Dhiogo Brustolin --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bb972e027..37df7474e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,7 +42,7 @@ This version adds a dependency on Swift. - Rename `SentryOptions.enablePreWarmedAppStartTracking` to `enablePreWarmedAppStartTracing` - Rename `SentryOptions.enableFileIOTracking` to `enableFileIOTracing` - Rename `SentryOptions.enableCoreDataTracking` to `enableCoreDataTracing` -- SentrySDK.close calls flush (#2452) +- SentrySDK.close calls flush (#2453) ## 7.31.2 From df13cec3b3c2348b0e67da14153c29765ade7568 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 1 Dec 2022 07:36:53 +0100 Subject: [PATCH 4/5] explain why blocking --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2627f71c51..e2f7944abb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ This version adds a dependency on Swift. - Rename `SentryOptions.enablePreWarmedAppStartTracking` to `enablePreWarmedAppStartTracing` - Rename `SentryOptions.enableFileIOTracking` to `enableFileIOTracing` - Rename `SentryOptions.enableCoreDataTracking` to `enableCoreDataTracing` -- SentrySDK.close calls flush (#2453) +- SentrySDK.close calls flush, which is a blocking call (#2453) ## 7.31.3 From 84d017db50159e3b0a50bc487aa7e7f7117e40c6 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 1 Dec 2022 07:48:33 +0100 Subject: [PATCH 5/5] fix linter --- Samples/iOS-Swift/iOS-Swift/ViewController.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Samples/iOS-Swift/iOS-Swift/ViewController.swift b/Samples/iOS-Swift/iOS-Swift/ViewController.swift index eb7e0aa849..2c9f13af7c 100644 --- a/Samples/iOS-Swift/iOS-Swift/ViewController.swift +++ b/Samples/iOS-Swift/iOS-Swift/ViewController.swift @@ -159,11 +159,13 @@ class ViewController: UIViewController { SentrySDK.crash() } + // swiftlint:disable force_unwrapping @IBAction func unwrapCrash(_ sender: Any) { let a: String! = nil let b: String = a! print(b) } + // swiftlint:enable force_unwrapping @IBAction func asyncCrash(_ sender: Any) { DispatchQueue.main.async {