diff --git a/CHANGELOG.md b/CHANGELOG.md index 30499ed2a5..94ad299b79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ - Allow hybrid SDK to set replay options tags information (#4710) - Add threshold to always log fatal logs (#4707) +### Improvements + +- Add error logging for invalid `cacheDirectoryPath` (#4693) ### Internal - Change macros TEST and TESTCI to SENTRY_TEST and SENTRY_TEST_CI (#4712) diff --git a/SentryTestUtils/SentryFileManager+Test.h b/SentryTestUtils/SentryFileManager+Test.h index 52010880a3..eb97708216 100644 --- a/SentryTestUtils/SentryFileManager+Test.h +++ b/SentryTestUtils/SentryFileManager+Test.h @@ -2,6 +2,9 @@ NS_ASSUME_NONNULL_BEGIN +BOOL isErrorPathTooLong(NSError *error); +BOOL createDirectoryIfNotExists(NSString *path, NSError **error); + SENTRY_EXTERN NSURL *launchProfileConfigFileURL(void); SENTRY_EXTERN NSURL *_Nullable sentryLaunchConfigFileURL; diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 07a2191139..c3d7fb573f 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -87,7 +87,7 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options dispatchQueueWrapper:dispatchQueue error:&error]; if (error != nil) { - SENTRY_LOG_ERROR(@"Cannot init filesystem."); + SENTRY_LOG_FATAL(@"Failed to initialize file system: %@", error.localizedDescription); return nil; } return [self initWithOptions:options diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 69a63c7582..92362942b0 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -20,19 +20,49 @@ NSString *const EnvelopesPathComponent = @"envelopes"; +BOOL +isErrorPathTooLong(NSError *error) +{ + NSError *underlyingError = NULL; + if (@available(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5, *)) { + underlyingError = error.underlyingErrors.firstObject; + } + if (underlyingError == NULL) { + id errorInUserInfo = [error.userInfo valueForKey:NSUnderlyingErrorKey]; + if (errorInUserInfo && [errorInUserInfo isKindOfClass:[NSError class]]) { + underlyingError = errorInUserInfo; + } + } + if (underlyingError == NULL) { + underlyingError = error; + } + BOOL isEnameTooLong + = underlyingError.domain == NSPOSIXErrorDomain && underlyingError.code == ENAMETOOLONG; + // On older OS versions the error code is NSFileWriteUnknown + // Reference: https://developer.apple.com/forums/thread/128927?answerId=631839022#631839022 + BOOL isUnknownError = underlyingError.domain == NSCocoaErrorDomain + && underlyingError.code == NSFileWriteUnknownError; + + return isEnameTooLong || isUnknownError; +} + BOOL createDirectoryIfNotExists(NSString *path, NSError **error) { - if (![[NSFileManager defaultManager] createDirectoryAtPath:path - withIntermediateDirectories:YES - attributes:nil - error:error]) { - *error = NSErrorFromSentryErrorWithUnderlyingError(kSentryErrorFileIO, - [NSString stringWithFormat:@"Failed to create the directory at path %@.", path], - *error); - return NO; + BOOL success = [[NSFileManager defaultManager] createDirectoryAtPath:path + withIntermediateDirectories:YES + attributes:nil + error:error]; + if (success) { + return YES; } - return YES; + + if (isErrorPathTooLong(*error)) { + SENTRY_LOG_FATAL(@"Failed to create directory, path is too long: %@", path); + } + *error = NSErrorFromSentryErrorWithUnderlyingError(kSentryErrorFileIO, + [NSString stringWithFormat:@"Failed to create the directory at path %@.", path], *error); + return NO; } /** @@ -45,15 +75,14 @@ { NSError *error = nil; NSFileManager *fileManager = [NSFileManager defaultManager]; - if (![fileManager removeItemAtPath:path error:&error]) { - if (error.code == NSFileNoSuchFileError) { - SENTRY_LOG_DEBUG(@"No file to delete at %@", path); - } else { - SENTRY_LOG_ERROR( - @"Error occurred while deleting file at %@ because of %@", path, error); - } - } else { + if ([fileManager removeItemAtPath:path error:&error]) { SENTRY_LOG_DEBUG(@"Successfully deleted file at %@", path); + } else if (error.code == NSFileNoSuchFileError) { + SENTRY_LOG_DEBUG(@"No file to delete at %@", path); + } else if (isErrorPathTooLong(error)) { + SENTRY_LOG_FATAL(@"Failed to remove file, path is too long: %@", path); + } else { + SENTRY_LOG_ERROR(@"Error occurred while deleting file at %@ because of %@", path, error); } } @@ -102,9 +131,12 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options [self removeFileAtPath:self.eventsPath]; if (!createDirectoryIfNotExists(self.sentryPath, error)) { + SENTRY_LOG_FATAL(@"Failed to create Sentry SDK working directory: %@", self.sentryPath); return nil; } if (!createDirectoryIfNotExists(self.envelopesPath, error)) { + SENTRY_LOG_FATAL( + @"Failed to create Sentry SDK envelopes directory: %@", self.envelopesPath); return nil; } diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 81df321cae..f753fc60be 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -1,3 +1,5 @@ +// swiftlint:disable file_length + @testable import Sentry import SentryTestUtils import XCTest @@ -69,7 +71,27 @@ class SentryFileManagerTests: XCTestCase { sut.setDelegate(delegate) return sut } - + + func getValidPath() -> String { + URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("SentryTest") + .path + } + + func getTooLongPath() -> String { + var url = URL(fileURLWithPath: NSTemporaryDirectory()) + for element in ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J"] { + url = url.appendingPathComponent(Array( + repeating: element, + count: Int(NAME_MAX) + ).joined()) + } + return url.path + } + + func getInvalidPath() -> String { + URL(fileURLWithPath: "/dev/null").path + } } private var fixture: Fixture! @@ -723,6 +745,181 @@ class SentryFileManagerTests: XCTestCase { try "garbage".write(to: URL(fileURLWithPath: sut.timezoneOffsetFilePath), atomically: true, encoding: .utf8) XCTAssertNil(sut.readTimezoneOffset()) } + + func testIsErrorPathTooLong_underlyingErrorsAvailableAndMultipleErrorsGiven_shouldUseErrorInUserInfo() throws { + // -- Arrange -- + guard #available(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5, *) else { + throw XCTSkip("This test is only for macOS 11 and above") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSMultipleUnderlyingErrorsKey: [ + NSError(domain: NSCocoaErrorDomain, code: 2, userInfo: nil) + ], + NSUnderlyingErrorKey: NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil) + ]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertTrue(result) + } + + func testIsErrorPathTooLong_underlyingErrorsAvailableAndMultipleErrorsEmpty_shouldUseErrorInUserInfo() throws { + // -- Arrange -- + guard #available(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5, *) else { + throw XCTSkip("Test is disabled for this OS version") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSMultipleUnderlyingErrorsKey: [Any](), + NSUnderlyingErrorKey: NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil) + ]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertTrue(result) + } + + func testIsErrorPathTooLong_underlyingErrorsAvailableAndMultipleErrorsNotSet_shouldUseErrorInUserInfo() throws { + // -- Arrange -- + guard #available(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5, *) else { + throw XCTSkip("Test is disabled for this OS version") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSUnderlyingErrorKey: NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil) + ]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertTrue(result) + } + + func testIsErrorPathTooLong_underlyingErrorsAvailableAndOnlyMultipleErrorsGiven_shouldUseErrorFirstError() throws { + // -- Arrange -- + guard #available(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5, *) else { + throw XCTSkip("Test is disabled for this OS version") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSMultipleUnderlyingErrorsKey: [ + NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil), + NSError(domain: NSCocoaErrorDomain, code: 2, userInfo: nil) + ] + ]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertTrue(result) + } + + func testIsErrorPathTooLong_underlyingErrorsNotAvailableAndErrorNotInUserInfo_shouldNotCheckError() throws { + // -- Arrange -- + guard #unavailable(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5) else { + throw XCTSkip("Test is disabled for this OS version") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [:]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertFalse(result) + } + + func testIsErrorPathTooLong_underlyingErrorsNotAvailableAndNonErrorInUserInfo_shouldNotCheckError() throws { + // -- Arrange -- + guard #unavailable(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5) else { + throw XCTSkip("Test is disabled for this OS version") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSUnderlyingErrorKey: "This is not an error" + ]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertFalse(result) + } + + func testIsErrorPathTooLong_underlyingErrorsNotAvailableAndErrorInUserInfo_shouldNotCheckError() throws { + // -- Arrange -- + guard #unavailable(macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5) else { + throw XCTSkip("Test is disabled for this OS version") + } + // When accessing via `underlyingErrors`, the first result is the error set with `NSUnderlyingErrorKey`. + // This test asserts if that behavior changes. + let error = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSUnderlyingErrorKey: NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil) + ]) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertTrue(result) + } + + func testIsErrorPathTooLong_errorIsEnameTooLong_shouldReturnTrue() throws { + // -- Arrange -- + let error = NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil) + // -- Act -- + let result = isErrorPathTooLong(error) + // -- Assert -- + XCTAssertTrue(result) + } + + func testCreateDirectoryIfNotExists_successful_shouldNotLogError() throws { + // -- Arrange - + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configureLog(true, diagnosticLevel: .debug) + + let path = fixture.getValidPath() + var error: NSError? + // -- Act -- + let result = createDirectoryIfNotExists(path, &error) + // -- Assert - + XCTAssertTrue(result) + XCTAssertEqual(logOutput.loggedMessages.count, 0) + } + + func testCreateDirectoryIfNotExists_pathTooLogError_shouldLogError() throws { + // -- Arrange - + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configureLog(true, diagnosticLevel: .debug) + + let path = fixture.getTooLongPath() + var error: NSError? + // -- Act -- + let result = createDirectoryIfNotExists(path, &error) + // -- Assert - + XCTAssertFalse(result) + XCTAssertEqual(error?.domain, SentryErrorDomain) + XCTAssertEqual(error?.code, 108) + XCTAssertEqual(logOutput.loggedMessages.count, 1) + } + + func testCreateDirectoryIfNotExists_otherError_shouldNotLogError() throws { + // -- Arrange - + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configureLog(true, diagnosticLevel: .debug) + + let path = fixture.getInvalidPath() + var error: NSError? + // -- Act -- + let result = createDirectoryIfNotExists(path, &error) + // -- Assert - + XCTAssertFalse(result) + XCTAssertEqual(error?.domain, SentryErrorDomain) + XCTAssertEqual(error?.code, 108) + XCTAssertEqual(logOutput.loggedMessages.count, 0) + } } #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)