Merge branch 'main' into fixSwizzlingNullability

Author: philipphofmann

.github/workflows/create-issue-for-unreferenced-pr.yml

@@ -55,7 +55,8 @@ jobs:
             const prNumber = pr.number;
 
             // Regex for GitHub issue references (e.g., #123, fixes #456)
-            const issueRegexGitHub = /(?:(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\s*)?#\d+/i;
+            // https://regex101.com/r/eDiGrQ/1
+            const issueRegexGitHub = /(?:(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved):?\s*)?(#\d+|https:\/\/github\.com\/getsentry\/[\w-]+\/issues\/\d+)/i;
 
             // Regex for Linear issue references (e.g., ENG-123, resolves ENG-456)
             const issueRegexLinear = /(?:(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\s*)?[A-Z]+-\d+/i;

.github/workflows/ui-tests-critical.yml

@@ -102,6 +102,13 @@ jobs:
         shell: bash
         run: ./scripts/ci-enable-permissions.sh
 
+      - name: Ensure required runtime is loaded
+        # Ideally we will not need this, but CI sometimes is failing to load some runtimes, this will ensure they are loaded
+        timeout-minutes: 5 # 5 minutes timeout
+        env:
+          OS_VERSION: "18.4"
+        run: ./scripts/ci-ensure-runtime-loaded.sh --os-version "$OS_VERSION"
+
       - name: Boot simulator
         run: ./scripts/ci-boot-simulator.sh
 

.github/workflows/ui-tests.yml

@@ -195,7 +195,7 @@ jobs:
         ui-tests-swift6,
         ready-to-merge-gate,
       ]
-    name: UI Tests
+    name: UI Tests - Check
     # This is necessary since a failed/skipped dependent job would cause this job to be skipped
     if: always()
     runs-on: ubuntu-latest

AGENTS.md

@@ -37,6 +37,68 @@ This file provides comprehensive guidance for AI coding agents working with the
 - Fix any test or type errors until the whole suite is green.
 - Add or update tests for the code you change, even if nobody asked.
 
+#### Testing Error Handling Paths
+
+When testing error handling code paths, follow these guidelines:
+
+**Testable Error Paths:**
+
+Many system call errors can be reliably tested:
+
+- **File operation failures**: Use invalid/non-existent paths, closed file descriptors, or permission-restricted paths
+- **Directory operation failures**: Use invalid directory paths
+- **Network operation failures**: Use invalid addresses or closed sockets
+
+**Example test pattern:**
+
+```objc
+- (void)testFunction_HandlesOperationFailure
+{
+    // -- Arrange --
+    // This test verifies that functionName handles errors correctly when operation() fails.
+    //
+    // The error handling code path exists in SourceFile.c and correctly handles
+    // the error condition. The code change itself is correct and verified through code review.
+    
+    // Setup to trigger error (e.g., invalid path, closed fd, etc.)
+    
+    // -- Act --
+    bool result = functionName(/* parameters that will cause error */);
+    
+    // -- Assert --
+    // Verify the function fails gracefully (error handling path executes)
+    // This verifies that the error handling code path executes correctly.
+    XCTAssertFalse(result, @"functionName should fail with error condition");
+}
+```
+
+**Untestable Error Paths:**
+
+Some error paths cannot be reliably tested in a test environment:
+
+- **System calls with hardcoded valid parameters**: Cannot pass invalid parameters to trigger failures
+- **Resource exhaustion scenarios**: System limits may not be enforceable in test environments
+- **Function interposition limitations**: `DYLD_INTERPOSE` only works for dynamically linked symbols; statically linked system calls cannot be reliably mocked
+
+**Documenting Untestable Error Paths:**
+
+When an error path cannot be reliably tested:
+
+1. **Remove the test** if one was attempted but couldn't be made to work
+2. **Add documentation** in the test file explaining:
+   - Why there's no test for the error path
+   - Approaches that were tried and why they failed
+   - That the error handling code path exists and is correct (verified through code review)
+3. **Add a comment** in the source code at the error handling location explaining why it cannot be tested
+4. **Update PR description** to document untestable error paths in the "How did you test it?" section
+
+**Test Comment Best Practices:**
+
+- **Avoid line numbers** in test comments - they become outdated when code changes
+- **Reference function names and file names** instead of line numbers
+- **Document the error condition** being tested (e.g., "when open() fails")
+- **Explain verification approach** - verify that the error handling path executes correctly rather than capturing implementation details
+
 ### Commit Guidelines
 
 - **Pre-commit Hooks**: This repository uses pre-commit hooks. If a commit fails because files were changed during the commit process (e.g., by formatting hooks), automatically retry the commit. Pre-commit hooks may modify files (like formatting), and the commit should be retried with the updated files.

Sources/Sentry/SentryAsyncSafeLog.h

@@ -30,6 +30,16 @@
 
 #define SENTRY_ASYNC_SAFE_LOG_C_BUFFER_SIZE 1024
 
+/**
+ * Buffer size for thread-safe strerror_r operations.
+ *
+ * POSIX doesn't specify a minimum buffer size for strerror_r. We use 1024 bytes to match
+ * glibc's implementation, which uses a 1024-byte buffer for strerror() to ensure sufficient
+ * space for error messages across all locales and systems. This provides a safe upper bound
+ * while being reasonable for stack allocation since it's allocated per macro expansion.
+ */
+#define SENTRY_STRERROR_R_BUFFER_SIZE 1024
+
 /**
  * In addition to writing to file, we can also write to the console. This is not safe to do from
  * actual async contexts, but can be helpful while running with the debugger attached in certain
@@ -43,7 +53,10 @@
 extern "C" {
 #endif
 
+#include <errno.h>
 #include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
 
 static char g_logFilename[1024];
 
@@ -148,6 +161,36 @@ int sentry_asyncLogSetFileName(const char *filename, bool overwrite);
 #    define SENTRY_ASYNC_SAFE_LOG_TRACE(FMT, ...)
 #endif
 
+/**
+ * Thread-safe version of strerror using strerror_r.
+ * On macOS/iOS, strerror_r follows XSI-compliant version which returns int.
+ * This macro evaluates to a pointer to a buffer containing the error string.
+ *
+ * The buffer size is defined by SENTRY_STRERROR_R_BUFFER_SIZE (1024 bytes, matching glibc).
+ *
+ * The macro uses a GCC statement expression ({ ... }) which allows a block of statements
+ * to be used as an expression. The last expression in the block (__strerror_buf;) becomes
+ * the value of the entire expression, allowing the macro to be used directly in function
+ * calls like: SENTRY_ASYNC_SAFE_LOG_ERROR("Error: %s", SENTRY_STRERROR_R(errno));
+ *
+ * IMPORTANT: Uses thread-local storage to ensure the pointer remains valid after the macro
+ * completes while maintaining thread safety. This is necessary because the macro is used
+ * as a function argument, and stack-allocated buffers would be deallocated before the
+ * function (e.g., vsnprintf) reads them. Thread-local storage ensures each thread has
+ * its own buffer, preventing race conditions.
+ *
+ * @param ERRNUM The error number (e.g., errno).
+ * @return Pointer to a thread-local buffer containing the error string.
+ */
+#define SENTRY_STRERROR_R(ERRNUM)                                                                  \
+    ({                                                                                             \
+        static __thread char __strerror_buf[SENTRY_STRERROR_R_BUFFER_SIZE];                        \
+        if (strerror_r((ERRNUM), __strerror_buf, sizeof(__strerror_buf)) != 0) {                   \
+            snprintf(__strerror_buf, sizeof(__strerror_buf), "Unknown error %d", (ERRNUM));        \
+        }                                                                                          \
+        __strerror_buf;                                                                            \
+    })
+
 /**
  * If @c errno is set to a non-zero value after @c statement finishes executing,
  * the error value is logged, and the original return value of @c statement is
@@ -160,7 +203,7 @@ int sentry_asyncLogSetFileName(const char *filename, bool overwrite);
         const int __log_errnum = errno;                                                            \
         if (__log_errnum != 0) {                                                                   \
             SENTRY_ASYNC_SAFE_LOG_ERROR("%s failed with code: %d, description: %s", #statement,    \
-                __log_errnum, strerror(__log_errnum));                                             \
+                __log_errnum, SENTRY_STRERROR_R(__log_errnum));                                    \
         }                                                                                          \
         __log_rv;                                                                                  \
     })

Sources/Sentry/SentrySessionReplaySyncC.c

@@ -56,8 +56,8 @@ sentrySessionReplaySync_writeInfo(void)
     int fd = open(crashReplay.path, O_RDWR | O_CREAT | O_TRUNC, 0644);
 
     if (fd < 1) {
-        SENTRY_ASYNC_SAFE_LOG_ERROR(
-            "Could not open replay info crash for file %s: %s", crashReplay.path, strerror(errno));
+        SENTRY_ASYNC_SAFE_LOG_ERROR("Could not open replay info crash for file %s: %s",
+            crashReplay.path, SENTRY_STRERROR_R(errno));
         return;
     }
 
@@ -85,7 +85,7 @@ sentrySessionReplaySync_readInfo(SentryCrashReplay *output, const char *const pa
     int fd = open(path, O_RDONLY);
     if (fd < 0) {
         SENTRY_ASYNC_SAFE_LOG_ERROR(
-            "Could not open replay info crash file %s: %s", path, strerror(errno));
+            "Could not open replay info crash file %s: %s", path, SENTRY_STRERROR_R(errno));
         return false;
     }
 

Sources/Sentry/SentryViewHierarchyProviderHelper.m

@@ -33,7 +33,7 @@ + (BOOL)saveViewHierarchy:(NSString *)filePath
     const char *path = [filePath UTF8String];
     int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
     if (fd < 0) {
-        SENTRY_LOG_DEBUG(@"Could not open file %s for writing: %s", path, strerror(errno));
+        SENTRY_LOG_DEBUG(@"Could not open file %s for writing: %s", path, SENTRY_STRERROR_R(errno));
         return NO;
     }
 

Sources/Sentry/include/SentryLogC.h

@@ -1,3 +1,4 @@
+#import "SentryAsyncSafeLog.h"
 #import "SentryDefines.h"
 
 #ifdef __cplusplus
@@ -45,7 +46,7 @@ void logFatal(const char file[], int line, NSString *format, ...);
         const int __log_errnum = errno;                                                            \
         if (__log_errnum != 0) {                                                                   \
             SENTRY_LOG_ERROR(@"%s failed with code: %d, description: %s", #statement,              \
-                __log_errnum, strerror(__log_errnum));                                             \
+                __log_errnum, SENTRY_STRERROR_R(__log_errnum));                                    \
         }                                                                                          \
         __log_rv;                                                                                  \
     })

Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_AppState.c

@@ -270,8 +270,17 @@ saveState(const char *const path)
 {
     int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
     if (fd < 0) {
+        // Error handling path: Uses SENTRY_STRERROR_R(errno) for thread-safe error message
+        // retrieval. This error path cannot be reliably tested because:
+        // - saveState is a static function, so it cannot be called directly from tests
+        // - While open() failures can be triggered with invalid paths or permissions, testing
+        //   this function requires calling it indirectly through app state monitoring, which
+        //   makes it difficult to control the exact error conditions
+        // - System calls cannot be easily mocked in C without function interposition, which has
+        //   limitations for statically linked symbols
+        // The error handling code path exists and is correct (verified through code review).
         SENTRY_ASYNC_SAFE_LOG_ERROR(
-            "Could not open file %s for writing: %s", path, strerror(errno));
+            "Could not open file %s for writing: %s", path, SENTRY_STRERROR_R(errno));
         return false;
     }
 

Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c

@@ -500,15 +500,33 @@ installExceptionHandler(void)
     pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
     error = pthread_create(&g_secondaryPThread, &attr, &handleExceptions, kThreadSecondary);
     if (error != 0) {
-        SENTRY_ASYNC_SAFE_LOG_ERROR("pthread_create_suspended_np: %s", strerror(error));
+        // Error handling path: Uses SENTRY_STRERROR_R(error) for thread-safe error message
+        // retrieval. This error path cannot be reliably tested because:
+        // - installExceptionHandler is a static function, so it cannot be called directly from
+        // tests
+        // - pthread_create failures are difficult to force in a test environment (resource limits,
+        //   system constraints, etc. may not reliably trigger failures)
+        // - System calls cannot be easily mocked in C without function interposition, which has
+        //   limitations for statically linked symbols
+        // The error handling code path exists and is correct (verified through code review).
+        SENTRY_ASYNC_SAFE_LOG_ERROR("pthread_create_suspended_np: %s", SENTRY_STRERROR_R(error));
         goto failed;
     }
     g_secondaryMachThread = pthread_mach_thread_np(g_secondaryPThread);
 
     SENTRY_ASYNC_SAFE_LOG_DEBUG("Creating primary exception thread.");
     error = pthread_create(&g_primaryPThread, &attr, &handleExceptions, kThreadPrimary);
     if (error != 0) {
-        SENTRY_ASYNC_SAFE_LOG_ERROR("pthread_create: %s", strerror(error));
+        // Error handling path: Uses SENTRY_STRERROR_R(error) for thread-safe error message
+        // retrieval. This error path cannot be reliably tested because:
+        // - installExceptionHandler is a static function, so it cannot be called directly from
+        // tests
+        // - pthread_create failures are difficult to force in a test environment (resource limits,
+        //   system constraints, etc. may not reliably trigger failures)
+        // - System calls cannot be easily mocked in C without function interposition, which has
+        //   limitations for statically linked symbols
+        // The error handling code path exists and is correct (verified through code review).
+        SENTRY_ASYNC_SAFE_LOG_ERROR("pthread_create: %s", SENTRY_STRERROR_R(error));
         goto failed;
     }
     pthread_attr_destroy(&attr);