fix: Simplified IFileSystem (#3641)

CHANGELOG.md

@@ -14,7 +14,7 @@
 
 ### Features
 
-- Added a flag to options `DisableFileWrite` to allow users to opt-out of all file writing operations. Note that toggling this will affect features such as offline caching and auto-session tracking and release health as these rely on some file persistency ([#3614](https://github.com/getsentry/sentry-dotnet/pull/3614))
+- Added a flag to options `DisableFileWrite` to allow users to opt-out of all file writing operations. Note that toggling this will affect features such as offline caching and auto-session tracking and release health as these rely on some file persistency ([#3614](https://github.com/getsentry/sentry-dotnet/pull/3614), [#3641](https://github.com/getsentry/sentry-dotnet/pull/3641))
 
 ### Dependencies
 

src/Sentry/GlobalSessionManager.cs

@@ -51,29 +51,42 @@ private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp
             return;
         }
 
+        if (_options.DisableFileWrite)
+        {
+            _options.LogInfo("File write has been disabled via the options. Skipping persisting session.");
+            return;
+        }
+
         try
         {
             _options.LogDebug("Creating persistence directory for session file at '{0}'.", _persistenceDirectoryPath);
 
-            var result = _options.FileSystem.CreateDirectory(_persistenceDirectoryPath);
-            if (result is not FileOperationResult.Success)
+            if (!_options.FileSystem.CreateDirectory(_persistenceDirectoryPath))
             {
-                if (result is FileOperationResult.Disabled)
-                {
-                    _options.LogInfo("Persistent directory for session file has not been created. File-write has been disabled via the options.");
-                }
-                else
-                {
-                    _options.LogError("Failed to create persistent directory for session file.");
-                }
-
+                _options.LogError("Failed to create persistent directory for session file.");
                 return;
             }
 
             var filePath = Path.Combine(_persistenceDirectoryPath, PersistedSessionFileName);
 
             var persistedSessionUpdate = new PersistedSessionUpdate(update, pauseTimestamp);
-            persistedSessionUpdate.WriteToFile(_options.FileSystem, filePath, _options.DiagnosticLogger);
+            if (!_options.FileSystem.CreateFileForWriting(filePath, out var file))
+            {
+                _options.LogError("Failed to persist session file.");
+                return;
+            }
+
+            using var writer = new Utf8JsonWriter(file);
+
+            try
+            {
+                persistedSessionUpdate.WriteTo(writer, _options.DiagnosticLogger);
+                writer.Flush();
+            }
+            finally
+            {
+                file.Dispose();
+            }
 
             _options.LogDebug("Persisted session to a file '{0}'.", filePath);
         }
@@ -91,6 +104,12 @@ private void DeletePersistedSession()
             return;
         }
 
+        if (_options.DisableFileWrite)
+        {
+            _options.LogInfo("File write has been disabled via the options. Skipping deletion of persisted session files.");
+            return;
+        }
+
         var filePath = Path.Combine(_persistenceDirectoryPath, PersistedSessionFileName);
         try
         {
@@ -108,7 +127,11 @@ private void DeletePersistedSession()
                 }
             }
 
-            _options.FileSystem.DeleteFile(filePath);
+            if (!_options.FileSystem.DeleteFile(filePath))
+            {
+                _options.LogError("Failed to delete persisted session file.");
+                return;
+            }
 
             _options.LogInfo("Deleted persisted session file '{0}'.", filePath);
         }

src/Sentry/Http/HttpTransportBase.cs

@@ -383,20 +383,24 @@ private void HandleFailure(HttpResponseMessage response, Envelope envelope)
                     persistLargeEnvelopePathEnvVar,
                     destinationDirectory);
 
+                if (_options.DisableFileWrite)
+                {
+                    _options.LogInfo("File write has been disabled via the options. Skipping persisting envelope.");
+                    return;
+                }
+
                 var destination = Path.Combine(destinationDirectory, "envelope_too_large",
                     (eventId ?? SentryId.Create()).ToString());
 
-                var createDirectoryResult = _options.FileSystem.CreateDirectory(Path.GetDirectoryName(destination)!);
-                if (createDirectoryResult is not FileOperationResult.Success)
+                if (!_options.FileSystem.CreateDirectory(Path.GetDirectoryName(destination)!))
                 {
-                    _options.LogError("Failed to create directory to store the envelope: {0}", createDirectoryResult);
+                    _options.LogError("Failed to create directory to store the envelope.");
                     return;
                 }
 
-                var result = _options.FileSystem.CreateFileForWriting(destination, out var envelopeFile);
-                if (result is not FileOperationResult.Success)
+                if (!_options.FileSystem.CreateFileForWriting(destination, out var envelopeFile))
                 {
-                    _options.LogError("Failed to create envelope file: {0}", result);
+                    _options.LogError("Failed to create envelope file.");
                     return;
                 }
 
@@ -449,20 +453,24 @@ private async Task HandleFailureAsync(HttpResponseMessage response, Envelope env
                     persistLargeEnvelopePathEnvVar,
                     destinationDirectory);
 
+                if (_options.DisableFileWrite)
+                {
+                    _options.LogInfo("File write has been disabled via the options. Skipping persisting envelope.");
+                    return;
+                }
+
                 var destination = Path.Combine(destinationDirectory, "envelope_too_large",
                     (eventId ?? SentryId.Create()).ToString());
 
-                var createDirectoryResult = _options.FileSystem.CreateDirectory(Path.GetDirectoryName(destination)!);
-                if (createDirectoryResult is not FileOperationResult.Success)
+                if (!_options.FileSystem.CreateDirectory(Path.GetDirectoryName(destination)!))
                 {
-                    _options.LogError("Failed to create directory to store the envelope: {0}", createDirectoryResult);
+                    _options.LogError("Failed to create directory to store the envelope.");
                     return;
                 }
 
-                var result = _options.FileSystem.CreateFileForWriting(destination, out var envelopeFile);
-                if (result is not FileOperationResult.Success)
+                if (!_options.FileSystem.CreateFileForWriting(destination, out var envelopeFile))
                 {
-                    _options.LogError("Failed to create envelope file: {0}", result);
+                    _options.LogError("Failed to create envelope file.");
                     return;
                 }
 

src/Sentry/ISentryJsonSerializable.cs

@@ -17,21 +17,3 @@ public interface ISentryJsonSerializable
     /// </remarks>
     void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger);
 }
-
-internal static class JsonSerializableExtensions
-{
-    public static void WriteToFile(this ISentryJsonSerializable serializable, IFileSystem fileSystem, string filePath, IDiagnosticLogger? logger)
-    {
-        var result = fileSystem.CreateFileForWriting(filePath, out var file);
-        if (result is not FileOperationResult.Success)
-        {
-            return;
-        }
-
-        using var writer = new Utf8JsonWriter(file);
-
-        serializable.WriteTo(writer, logger);
-        writer.Flush();
-        file.Dispose();
-    }
-}

src/Sentry/Internal/FileSystemBase.cs

@@ -20,10 +20,10 @@ public IEnumerable<string> EnumerateFiles(string path, string searchPattern, Sea
 
     public Stream OpenFileForReading(string path) => File.OpenRead(path);
 
-    public abstract FileOperationResult CreateDirectory(string path);
-    public abstract FileOperationResult DeleteDirectory(string path, bool recursive = false);
-    public abstract FileOperationResult CreateFileForWriting(string path, out Stream fileStream);
-    public abstract FileOperationResult WriteAllTextToFile(string path, string contents);
-    public abstract FileOperationResult MoveFile(string sourceFileName, string destFileName, bool overwrite = false);
-    public abstract FileOperationResult DeleteFile(string path);
+    public abstract bool CreateDirectory(string path);
+    public abstract bool DeleteDirectory(string path, bool recursive = false);
+    public abstract bool CreateFileForWriting(string path, out Stream fileStream);
+    public abstract bool WriteAllTextToFile(string path, string contents);
+    public abstract bool MoveFile(string sourceFileName, string destFileName, bool overwrite = false);
+    public abstract bool DeleteFile(string path);
 }

src/Sentry/Internal/Http/CachingTransport.cs

@@ -78,6 +78,10 @@ private CachingTransport(ITransport innerTransport, SentryOptions options, bool
             options.TryGetProcessSpecificCacheDirectoryPath() ??
             throw new InvalidOperationException("Cache directory or DSN is not set.");
 
+        // Sanity check: This should never happen in the first place.
+        // We check for `DisableFileWrite` before creating the CachingTransport.
+        Debug.Assert(!_options.DisableFileWrite);
+
         _processingDirectoryPath = Path.Combine(_isolatedCacheDirectoryPath, ProcessingFolder);
     }
 
@@ -451,8 +455,7 @@ private async Task StoreToCacheAsync(
 
         EnsureFreeSpaceInCache();
 
-        var result = _options.FileSystem.CreateFileForWriting(envelopeFilePath, out var stream);
-        if (result is not FileOperationResult.Success)
+        if (!_options.FileSystem.CreateFileForWriting(envelopeFilePath, out var stream))
         {
             _options.LogDebug("Failed to store to cache.");
             return;

src/Sentry/Internal/IFileSystem.cs

@@ -1,14 +1,12 @@
 namespace Sentry.Internal;
 
-internal enum FileOperationResult
-{
-    Success,
-    Failure,
-    Disabled
-}
-
 internal interface IFileSystem
 {
+    // Note: You are responsible for handling success/failure when attempting to write to disk.
+    // You are required to check for `Options.FileWriteDisabled` whether you are allowed to call any writing operations.
+    // The options will automatically pick between `ReadOnly` and `ReadAndWrite` to prevent accidental file writing that
+    // could cause crashes on restricted platforms like the Nintendo Switch.
+
     // Note: This is not comprehensive.  If you need other filesystem methods, add to this interface,
     // then implement in both Sentry.Internal.FileSystem and Sentry.Testing.FakeFileSystem.
 
@@ -21,10 +19,10 @@ internal interface IFileSystem
     string? ReadAllTextFromFile(string file);
     Stream OpenFileForReading(string path);
 
-    FileOperationResult CreateDirectory(string path);
-    FileOperationResult DeleteDirectory(string path, bool recursive = false);
-    FileOperationResult CreateFileForWriting(string path, out Stream fileStream);
-    FileOperationResult WriteAllTextToFile(string path, string contents);
-    FileOperationResult MoveFile(string sourceFileName, string destFileName, bool overwrite = false);
-    FileOperationResult DeleteFile(string path);
+    bool CreateDirectory(string path);
+    bool DeleteDirectory(string path, bool recursive = false);
+    bool CreateFileForWriting(string path, out Stream fileStream);
+    bool WriteAllTextToFile(string path, string contents);
+    bool MoveFile(string sourceFileName, string destFileName, bool overwrite = false);
+    bool DeleteFile(string path);
 }

src/Sentry/Internal/InstallationIdHelper.cs

@@ -60,7 +60,7 @@ internal class InstallationIdHelper(SentryOptions options)
             var directoryPath = Path.Combine(rootPath, "Sentry", options.Dsn!.GetHashString());
             var fileSystem = options.FileSystem;
 
-            if (fileSystem.CreateDirectory(directoryPath) is not FileOperationResult.Success)
+            if (!fileSystem.CreateDirectory(directoryPath))
             {
                 options.LogDebug("Failed to create a directory for installation ID file ({0}).", directoryPath);
                 return null;
@@ -79,7 +79,7 @@ internal class InstallationIdHelper(SentryOptions options)
 
             // Generate new installation ID and store it in a file
             var id = Guid.NewGuid().ToString();
-            if (fileSystem.WriteAllTextToFile(filePath, id) is not FileOperationResult.Success)
+            if (!fileSystem.WriteAllTextToFile(filePath, id))
             {
                 options.LogDebug("Failed to write Installation ID to file ({0}).", filePath);
                 return null;

src/Sentry/Internal/ReadOnlyFilesystem.cs

@@ -2,20 +2,24 @@ namespace Sentry.Internal;
 
 internal class ReadOnlyFileSystem : FileSystemBase
 {
-    public override FileOperationResult CreateDirectory(string path) => FileOperationResult.Disabled;
+    // Note: You are responsible for handling success/failure when attempting to write to disk.
+    // You are required to check for `Options.FileWriteDisabled` whether you are allowed to call any writing operations.
+    // The options will automatically pick between `ReadOnly` and `ReadAndWrite` to prevent accidental file writing that
+    // could cause crashes on restricted platforms like the Nintendo Switch.
 
-    public override FileOperationResult DeleteDirectory(string path, bool recursive = false) => FileOperationResult.Disabled;
+    public override bool CreateDirectory(string path) => false;
 
-    public override FileOperationResult CreateFileForWriting(string path, out Stream fileStream)
+    public override bool DeleteDirectory(string path, bool recursive = false) => false;
+
+    public override bool CreateFileForWriting(string path, out Stream fileStream)
     {
         fileStream = Stream.Null;
-        return FileOperationResult.Disabled;
+        return false;
     }
 
-    public override FileOperationResult WriteAllTextToFile(string path, string contents) => FileOperationResult.Disabled;
+    public override bool WriteAllTextToFile(string path, string contents) => false;
 
-    public override FileOperationResult MoveFile(string sourceFileName, string destFileName, bool overwrite = false) =>
-        FileOperationResult.Disabled;
+    public override bool MoveFile(string sourceFileName, string destFileName, bool overwrite = false) => false;
 
-    public override FileOperationResult DeleteFile(string path) => FileOperationResult.Disabled;
+    public override bool DeleteFile(string path) => false;
 }

src/Sentry/Internal/ReadWriteFileSystem.cs

@@ -2,31 +2,36 @@ namespace Sentry.Internal;
 
 internal class ReadWriteFileSystem : FileSystemBase
 {
-    public override FileOperationResult CreateDirectory(string path)
+    // Note: You are responsible for handling success/failure when attempting to write to disk.
+    // You are required to check for `Options.FileWriteDisabled` whether you are allowed to call any writing operations.
+    // The options will automatically pick between `ReadOnly` and `ReadAndWrite` to prevent accidental file writing that
+    // could cause crashes on restricted platforms like the Nintendo Switch.
+
+    public override bool CreateDirectory(string path)
     {
         Directory.CreateDirectory(path);
-        return DirectoryExists(path) ? FileOperationResult.Success : FileOperationResult.Failure;
+        return DirectoryExists(path);
     }
 
-    public override FileOperationResult DeleteDirectory(string path, bool recursive = false)
+    public override bool DeleteDirectory(string path, bool recursive = false)
     {
         Directory.Delete(path, recursive);
-        return Directory.Exists(path) ? FileOperationResult.Failure : FileOperationResult.Success;
+        return !Directory.Exists(path);
     }
 
-    public override FileOperationResult CreateFileForWriting(string path, out Stream fileStream)
+    public override bool CreateFileForWriting(string path, out Stream fileStream)
     {
         fileStream = File.Create(path);
-        return FileOperationResult.Success;
+        return true;
     }
 
-    public override FileOperationResult WriteAllTextToFile(string path, string contents)
+    public override bool WriteAllTextToFile(string path, string contents)
     {
         File.WriteAllText(path, contents);
-        return File.Exists(path) ? FileOperationResult.Success : FileOperationResult.Failure;
+        return File.Exists(path);
     }
 
-    public override FileOperationResult MoveFile(string sourceFileName, string destFileName, bool overwrite = false)
+    public override bool MoveFile(string sourceFileName, string destFileName, bool overwrite = false)
     {
 #if NETCOREAPP3_0_OR_GREATER
         File.Move(sourceFileName, destFileName, overwrite);
@@ -44,15 +49,15 @@ public override FileOperationResult MoveFile(string sourceFileName, string destF
 
         if (File.Exists(sourceFileName) || !File.Exists(destFileName))
         {
-            return FileOperationResult.Failure;
+            return false;
         }
 
-        return FileOperationResult.Success;
+        return true;
     }
 
-    public override FileOperationResult DeleteFile(string path)
+    public override bool DeleteFile(string path)
     {
         File.Delete(path);
-        return File.Exists(path) ? FileOperationResult.Failure : FileOperationResult.Success;
+        return !File.Exists(path);
     }
 }

src/Sentry/Internal/SdkComposer.cs

@@ -32,7 +32,7 @@ private ITransport CreateTransport()
 
             if (_options.DisableFileWrite)
             {
-                _options.LogInfo("File writing is disabled, Skipping caching transport creation.");
+                _options.LogInfo("File write has been disabled via the options. Skipping caching transport creation.");
             }
             else
             {