diff --git a/CHANGELOG.md b/CHANGELOG.md index dc53197421a..090e054f10c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Add null-handling for parsed DSN in SentryHTTPTransport (#5800) + ### Features - Structured Logs: Flush logs on SDK flush/close (#5834) diff --git a/Sources/Sentry/SentryHttpTransport.m b/Sources/Sentry/SentryHttpTransport.m index fa9bbec9c12..d861f116d05 100644 --- a/Sources/Sentry/SentryHttpTransport.m +++ b/Sources/Sentry/SentryHttpTransport.m @@ -33,7 +33,8 @@ @interface SentryHttpTransport () @property (nonatomic, strong) SentryFileManager *fileManager; @property (nonatomic, strong) id requestManager; @property (nonatomic, strong) SentryNSURLRequestBuilder *requestBuilder; -@property (nonatomic, strong) SentryOptions *options; +@property (nonatomic, strong) SentryDsn *dsn; +@property (nonatomic) BOOL sendClientReports; @property (nonatomic, strong) id rateLimits; @property (nonatomic, strong) SentryEnvelopeRateLimit *envelopeRateLimit; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue; @@ -67,7 +68,8 @@ @interface SentryHttpTransport () @implementation SentryHttpTransport -- (id)initWithOptions:(SentryOptions *)options +- (id)initWithDsn:(SentryDsn *)dsn + sendClientReports:(BOOL)sendClientReports cachedEnvelopeSendDelay:(NSTimeInterval)cachedEnvelopeSendDelay dateProvider:(id)dateProvider fileManager:(SentryFileManager *)fileManager @@ -78,7 +80,8 @@ - (id)initWithOptions:(SentryOptions *)options dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper { if (self = [super init]) { - self.options = options; + self.dsn = dsn; + self.sendClientReports = sendClientReports; _cachedEnvelopeSendDelay = cachedEnvelopeSendDelay; self.requestManager = requestManager; self.requestBuilder = requestBuilder; @@ -162,7 +165,7 @@ - (void)recordLostEvent:(SentryDataCategory)category reason:(SentryDiscardReason)reason quantity:(NSUInteger)quantity { - if (!self.options.sendClientReports) { + if (!self.sendClientReports) { return; } @@ -276,7 +279,7 @@ - (void)envelopeItemDeleted:(SentryEnvelopeItem *)envelopeItem - (SentryEnvelope *)addClientReportTo:(SentryEnvelope *)envelope { - if (!self.options.sendClientReports) { + if (!self.sendClientReports) { return envelope; } @@ -357,14 +360,14 @@ - (void)sendAllCachedEnvelopes // We must set sentAt as close as possible to the transmission of the envelope to Sentry. rateLimitedEnvelope.header.sentAt = [self.dateProvider date]; - NSError *requestError = nil; + NSError *_Nullable requestError = nil; NSURLRequest *request = [self.requestBuilder createEnvelopeRequest:rateLimitedEnvelope - dsn:self.options.parsedDsn + dsn:self.dsn didFailWithError:&requestError]; if (nil == request || nil != requestError) { if (nil != requestError) { - SENTRY_LOG_DEBUG(@"Failed to build request: %@.", requestError); + SENTRY_LOG_FATAL(@"Failed to build request to send envelope: %@.", requestError); } [self recordLostEventFor:rateLimitedEnvelope.items]; [self deleteEnvelopeAndSendNext:envelopeFilePath]; diff --git a/Sources/Sentry/SentryTransportFactory.m b/Sources/Sentry/SentryTransportFactory.m index 14bd283efae..aef6c29a30d 100644 --- a/Sources/Sentry/SentryTransportFactory.m +++ b/Sources/Sentry/SentryTransportFactory.m @@ -3,6 +3,8 @@ #import "SentryEnvelopeRateLimit.h" #import "SentryHttpDateParser.h" #import "SentryHttpTransport.h" +#import "SentryInternalDefines.h" +#import "SentryLogC.h" #import "SentryNSURLRequestBuilder.h" #import "SentryOptions.h" #import "SentryQueueableRequestManager.h" @@ -27,34 +29,37 @@ @implementation SentryTransportFactory sentryFileManager:(SentryFileManager *)sentryFileManager rateLimits:(id)rateLimits { - NSURLSession *session; - - if (options.urlSession) { - session = options.urlSession; - } else { - NSURLSessionConfiguration *configuration = - [NSURLSessionConfiguration ephemeralSessionConfiguration]; - session = [NSURLSession sessionWithConfiguration:configuration - delegate:options.urlSessionDelegate - delegateQueue:nil]; - } + NSMutableArray> *transports = [NSMutableArray array]; + NSURLSession *session = [self getUrlSession:options]; id requestManager = [[SentryQueueableRequestManager alloc] initWithSession:session]; - SentryEnvelopeRateLimit *envelopeRateLimit = [[SentryEnvelopeRateLimit alloc] initWithRateLimits:rateLimits]; - - dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( - DISPATCH_QUEUE_SERIAL, DISPATCH_QUEUE_PRIORITY_LOW, 0); - SentryDispatchQueueWrapper *dispatchQueueWrapper = - [[SentryDispatchQueueWrapper alloc] initWithName:"io.sentry.http-transport" - attributes:attributes]; + SentryDispatchQueueWrapper *dispatchQueueWrapper = [self createDispatchQueueWrapper]; SentryNSURLRequestBuilder *requestBuilder = [[SentryNSURLRequestBuilder alloc] init]; - SentryHttpTransport *httpTransport = - [[SentryHttpTransport alloc] initWithOptions:options + if (options.enableSpotlight) { + SENTRY_LOG_DEBUG(@"Spotlight is enabled, creating Spotlight transport."); + SentrySpotlightTransport *spotlightTransport = + [[SentrySpotlightTransport alloc] initWithOptions:options + requestManager:requestManager + requestBuilder:requestBuilder + dispatchQueueWrapper:dispatchQueueWrapper]; + + [transports addObject:spotlightTransport]; + } else { + SENTRY_LOG_DEBUG(@"Spotlight is disabled in options, not adding Spotlight transport."); + } + + if (options.parsedDsn) { + SENTRY_LOG_DEBUG(@"Options contain parsed DSN, creating HTTP transport."); + SentryDsn *_Nonnull dsn = SENTRY_UNWRAP_NULLABLE(SentryDsn, options.parsedDsn); + + SentryHttpTransport *httpTransport = + [[SentryHttpTransport alloc] initWithDsn:dsn + sendClientReports:options.sendClientReports cachedEnvelopeSendDelay:0.1 dateProvider:dateProvider fileManager:sentryFileManager @@ -64,18 +69,39 @@ @implementation SentryTransportFactory envelopeRateLimit:envelopeRateLimit dispatchQueueWrapper:dispatchQueueWrapper]; - if (options.enableSpotlight) { - SentrySpotlightTransport *spotlightTransport = - [[SentrySpotlightTransport alloc] initWithOptions:options - requestManager:requestManager - requestBuilder:requestBuilder - dispatchQueueWrapper:dispatchQueueWrapper]; - return @[ httpTransport, spotlightTransport ]; + [transports addObject:httpTransport]; } else { - return @[ httpTransport ]; + SENTRY_LOG_WARN( + @"Failed to create HTTP transport because the SentryOptions does not contain " + @"a parsed DSN."); } + + return transports; } ++ (NSURLSession *)getUrlSession:(SentryOptions *_Nonnull)options +{ + if (options.urlSession) { + SENTRY_LOG_DEBUG(@"Using URL session provided in SDK options for HTTP transport."); + return SENTRY_UNWRAP_NULLABLE(NSURLSession, options.urlSession); + } + + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration ephemeralSessionConfiguration]; + return [NSURLSession sessionWithConfiguration:configuration + delegate:options.urlSessionDelegate + delegateQueue:nil]; +} + ++ (SentryDispatchQueueWrapper *)createDispatchQueueWrapper +{ + dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( + DISPATCH_QUEUE_SERIAL, DISPATCH_QUEUE_PRIORITY_LOW, 0); + SentryDispatchQueueWrapper *dispatchQueueWrapper = + [[SentryDispatchQueueWrapper alloc] initWithName:"io.sentry.http-transport" + attributes:attributes]; + return dispatchQueueWrapper; +} @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryHttpTransport.h b/Sources/Sentry/include/SentryHttpTransport.h index 8b279603f5a..c27bf1ac5bc 100644 --- a/Sources/Sentry/include/SentryHttpTransport.h +++ b/Sources/Sentry/include/SentryHttpTransport.h @@ -7,7 +7,7 @@ @class SentryDispatchQueueWrapper; @class SentryNSURLRequestBuilder; -@class SentryOptions; +@class SentryDsn; @protocol SentryCurrentDateProvider; NS_ASSUME_NONNULL_BEGIN @@ -16,7 +16,8 @@ NS_ASSUME_NONNULL_BEGIN : NSObject SENTRY_NO_INIT -- (id)initWithOptions:(SentryOptions *)options +- (id)initWithDsn:(SentryDsn *)dsn + sendClientReports:(BOOL)sendClientReports cachedEnvelopeSendDelay:(NSTimeInterval)cachedEnvelopeSendDelay dateProvider:(id)dateProvider fileManager:(SentryFileManager *)fileManager diff --git a/Tests/SentryTests/Networking/SentryHttpTransportFlushIntegrationTests.swift b/Tests/SentryTests/Networking/SentryHttpTransportFlushIntegrationTests.swift index 4de4af09631..95441997b4a 100644 --- a/Tests/SentryTests/Networking/SentryHttpTransportFlushIntegrationTests.swift +++ b/Tests/SentryTests/Networking/SentryHttpTransportFlushIntegrationTests.swift @@ -164,7 +164,8 @@ final class SentryHttpTransportFlushIntegrationTests: XCTestCase { let dispatchQueueWrapper = SentryDispatchQueueWrapper() return (SentryHttpTransport( - options: options, + dsn: try XCTUnwrap(options.parsedDsn), + sendClientReports: options.sendClientReports, cachedEnvelopeSendDelay: 0.0, dateProvider: SentryDefaultCurrentDateProvider(), fileManager: fileManager, diff --git a/Tests/SentryTests/Networking/SentryHttpTransportTests.swift b/Tests/SentryTests/Networking/SentryHttpTransportTests.swift index 6444f2b07d1..707fe6ccf69 100644 --- a/Tests/SentryTests/Networking/SentryHttpTransportTests.swift +++ b/Tests/SentryTests/Networking/SentryHttpTransportTests.swift @@ -136,9 +136,10 @@ class SentryHttpTransportTests: XCTestCase { func getSut( fileManager: SentryFileManager? = nil, dispatchQueueWrapper: SentryDispatchQueueWrapper? = nil - ) -> SentryHttpTransport { + ) throws -> SentryHttpTransport { return SentryHttpTransport( - options: options, + dsn: try XCTUnwrap(options.parsedDsn), + sendClientReports: options.sendClientReports, cachedEnvelopeSendDelay: 0.0, dateProvider: currentDateProvider, fileManager: fileManager ?? self.fileManager, @@ -163,13 +164,13 @@ class SentryHttpTransportTests: XCTestCase { private var fixture: Fixture! private var sut: SentryHttpTransport! - override func setUp() { + override func setUpWithError() throws { super.setUp() fixture = Fixture() fixture.fileManager.deleteAllEnvelopes() fixture.requestManager.returnResponse(response: HTTPURLResponse()) - sut = fixture.getSut() + sut = try fixture.getSut() } override func tearDown() { @@ -179,14 +180,14 @@ class SentryHttpTransportTests: XCTestCase { clearTestState() } - func testInitSendsCachedEnvelopes() { + func testInitSendsCachedEnvelopes() throws { givenNoInternetConnection() sendEventAsync() assertEnvelopesStored(envelopeCount: 1) waitForAllRequests() givenOkResponse() - let sut = fixture.getSut() + let sut = try fixture.getSut() XCTAssertNotNil(sut) waitForAllRequests() @@ -520,8 +521,8 @@ class SentryHttpTransportTests: XCTestCase { func testFailureToStoreEvenlopeEventStillSendsRequest() throws { let fileManger = try TestFileManager(options: fixture.options) fileManger.storeEnvelopePathNil = true // Failure to store envelope returns nil path - let sut = fixture.getSut(fileManager: fileManger) - + let sut = try fixture.getSut(fileManager: fileManger) + sut.send(envelope: fixture.eventEnvelope) XCTAssertEqual(fileManger.storeEnvelopeInvocations.count, 1) @@ -750,13 +751,13 @@ class SentryHttpTransportTests: XCTestCase { fixture.dispatchQueueWrapper.dispatchAfterExecutesBlock = false // Interact with sut in extra function so ARC deallocates it - func getSut() { - let sut = fixture.getSut() + func getSut() throws { + let sut = try fixture.getSut() sut.send(envelope: fixture.eventEnvelope) waitForAllRequests() } - getSut() - + try getSut() + for dispatchAfterBlock in fixture.dispatchQueueWrapper.dispatchAfterInvocations.invocations { dispatchAfterBlock.block() } @@ -841,8 +842,9 @@ class SentryHttpTransportTests: XCTestCase { assertClientReportStoredInMemory() } - func testSendClientReportsDisabled_DoesNotRecordLostEvents() { + func testSendClientReportsDisabled_DoesNotRecordLostEvents() throws { fixture.options.sendClientReports = false + sut = try fixture.getSut() givenErrorResponse() sendEvent() @@ -850,12 +852,13 @@ class SentryHttpTransportTests: XCTestCase { assertClientReportNotStoredInMemory() } - func testSendClientReportsDisabled_DoesSendClientReport() { + func testSendClientReportsDisabled_DoesSendClientReport() throws { givenErrorResponse() sendEvent() givenOkResponse() fixture.options.sendClientReports = false + sut = try fixture.getSut() sendEvent() assertEventIsSentAsEnvelope() @@ -917,18 +920,18 @@ class SentryHttpTransportTests: XCTestCase { XCTAssertEqual(2, fixture.requestManager.requests.count) } - func testDealloc_StopsReachabilityMonitoring() { - func deallocSut() { - _ = fixture.getSut() + func testDealloc_StopsReachabilityMonitoring() throws { + func deallocSut() throws { + _ = try fixture.getSut() } - deallocSut() + try deallocSut() XCTAssertEqual(1, fixture.reachability.stopMonitoringInvocations.count) } - func testDealloc_TriggerNetworkReachable_NoCrash() { - _ = fixture.getSut() - + func testDealloc_TriggerNetworkReachable_NoCrash() throws { + _ = try fixture.getSut() + fixture.reachability.triggerNetworkReachable() } #endif // !os(watchOS) diff --git a/Tests/SentryTests/Networking/SentryTransportFactoryTests.swift b/Tests/SentryTests/Networking/SentryTransportFactoryTests.swift index ec0d3ac0f6f..3530d94101a 100644 --- a/Tests/SentryTests/Networking/SentryTransportFactoryTests.swift +++ b/Tests/SentryTests/Networking/SentryTransportFactoryTests.swift @@ -3,10 +3,10 @@ import Sentry import XCTest class SentryTransportFactoryTests: XCTestCase { - private static let dsnAsString = TestConstants.dsnAsString(username: "SentryTransportFactoryTests") func testIntegration_UrlSessionDelegate_PassedToRequestManager() throws { + // -- Arrange -- let urlSessionDelegateSpy = UrlSessionDelegateSpy() let expect = expectation(description: "UrlSession Delegate of Options called in RequestManager") @@ -19,6 +19,8 @@ class SentryTransportFactoryTests: XCTestCase { options.urlSessionDelegate = urlSessionDelegateSpy let fileManager = try! SentryFileManager(options: options, dispatchQueueWrapper: TestSentryDispatchQueueWrapper()) + + // -- Act -- let transports = TransportInitializer.initTransports( options, dateProvider: SentryDependencyContainer.sharedInstance().dateProvider, @@ -32,11 +34,13 @@ class SentryTransportFactoryTests: XCTestCase { let request = URLRequest(url: imgUrl) requestManager.add(request) { _, _ in /* We don't care about the result */ } + + // -- Assert -- wait(for: [expect], timeout: 10) } func testShouldReturnTransports_WhenURLSessionPassed() throws { - + // -- Arrange -- let urlSessionDelegateSpy = UrlSessionDelegateSpy() let expect = expectation(description: "UrlSession Delegate of Options called in RequestManager") @@ -46,9 +50,12 @@ class SentryTransportFactoryTests: XCTestCase { } let options = Options() + options.dsn = SentryTransportFactoryTests.dsnAsString options.urlSession = sessionConfiguration let fileManager = try! SentryFileManager(options: options, dispatchQueueWrapper: TestSentryDispatchQueueWrapper()) + + // -- Act -- let transports = TransportInitializer.initTransports( options, dateProvider: SentryDependencyContainer.sharedInstance().dateProvider, @@ -63,29 +70,77 @@ class SentryTransportFactoryTests: XCTestCase { let request = URLRequest(url: imgUrl) requestManager.add(request) { _, _ in /* We don't care about the result */ } + + // -- Assert -- wait(for: [expect], timeout: 10) } func testShouldReturnTwoTransports_WhenSpotlightEnabled() throws { + // -- Arrange -- let options = Options() + options.dsn = SentryTransportFactoryTests.dsnAsString options.enableSpotlight = true + + // -- Act -- let transports = TransportInitializer.initTransports( options, dateProvider: SentryDependencyContainer.sharedInstance().dateProvider, sentryFileManager: try SentryFileManager(options: options), rateLimits: rateLimiting() ) - + + // -- Assert -- + XCTAssertEqual(transports.count, 2) XCTAssert(transports.contains { $0.isKind(of: SentrySpotlightTransport.self) }) - XCTAssert(transports.contains { $0.isKind(of: SentryHttpTransport.self) }) } + func testInitTransports_whenOptionsParsedDsnNilAndSpotlightDisabled_shouldReturnEmptyTransports() throws { + // -- Arrange -- + let options = Options() + options.dsn = nil + options.enableSpotlight = false + + // -- Act -- + let transports = TransportInitializer.initTransports( + options, + dateProvider: SentryDependencyContainer.sharedInstance().dateProvider, + sentryFileManager: try SentryFileManager(options: options), + rateLimits: rateLimiting() + ) + + // -- Assert -- + XCTAssertEqual(transports.count, 0) + } + + func testInitTransports_whenOptionsParsedDsnNilAndSpotlightEnabled_shouldReturnSpotlightTransport() throws { + // -- Arrange -- + let options = Options() + options.dsn = nil + options.enableSpotlight = true + + // -- Act -- + let transports = TransportInitializer.initTransports( + options, + dateProvider: SentryDependencyContainer.sharedInstance().dateProvider, + sentryFileManager: try SentryFileManager(options: options), + rateLimits: rateLimiting() + ) + + // -- Assert -- + XCTAssertEqual(transports.count, 1) + XCTAssert(transports.contains { + $0.isKind(of: SentrySpotlightTransport.self) + }) + } + + // MARK: - Helpers + private func rateLimiting() -> RateLimits { let dateProvider = TestCurrentDateProvider() let retryAfterHeaderParser = RetryAfterHeaderParser(httpDateParser: HttpDateParser(), currentDateProvider: dateProvider) @@ -93,5 +148,4 @@ class SentryTransportFactoryTests: XCTestCase { return DefaultRateLimits(retryAfterHeaderParser: retryAfterHeaderParser, andRateLimitParser: rateLimitParser, currentDateProvider: dateProvider) } - }