Merge d0605ac into 91ceb3f

getsentry · Jan 16, 2025 · 6dafec2 · 6dafec2
2 parents 91ceb3f + d0605ac
commit 6dafec2
Show file tree

Hide file tree

Showing 5 changed files with 183 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,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/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
@@ -20,19 +20,45 @@
 
 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;
+    }
+    if (underlyingError.domain == NSPOSIXErrorDomain && underlyingError.code == ENAMETOOLONG) {
+        return YES;
+    }
+    return NO;
+}
+
 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 +71,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 +127,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/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h
@@ -15,6 +15,12 @@ NS_ASSUME_NONNULL_BEGIN
 @class SentryOptions;
 @class SentrySession;
 
+#if SENTRY_TEST || SENTRY_TEST_CI
+BOOL isErrorPathTooLong(NSError *error);
+
+BOOL createDirectoryIfNotExists(NSString *path, NSError **error);
+#endif // SENTRY_TEST || SENTRY_TEST_CI
+
 NS_SWIFT_NAME(SentryFileManager)
 @interface SentryFileManager : NSObject
 SENTRY_NO_INIT

diff --git 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
@@ -723,6 +725,132 @@ 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.0, 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.0, 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: [],
+            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.0, 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.0, 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.0, 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.0, 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.0, 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 testIsErrrorPathTooLong_errorIsEnameTooLong_shouldReturnTrue() throws {
+        // -- Arrange --
+        let error = NSError(domain: NSPOSIXErrorDomain, code: Int(ENAMETOOLONG), userInfo: nil)
+        // -- Act --
+        let result = isErrorPathTooLong(error)
+        // -- Assert --
+        XCTAssertTrue(result)
+    }
 }
 
 #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)