tests/NLog.UnitTests/Targets/FileTargetTests.cs (3)

1019-1019: Assert files and contents to catch “first message lost”.

To ensure no event is dropped when creating directories, verify both files exist and contain “a” and “b”.

                 var logger = LogManager.GetLogger("A");
                 logger.Info("a");
                 logger.Info("b");
 
                 Assert.Equal(createDirs, Directory.Exists(tempDir));
                 if (createDirs)
-                    Assert.Equal(2, Directory.GetDirectories(tempDir).Length);
+                {
+                    Assert.Equal(2, Directory.GetDirectories(tempDir).Length);
+                    var files = Directory.GetFiles(tempDir, "log.log", SearchOption.AllDirectories);
+                    Assert.Equal(2, files.Length);
+                    var contents = files.Select(f => File.ReadAllText(f).Trim()).OrderBy(s => s).ToArray();
+                    Assert.Equal(new[] { "a", "b" }, contents);
+                }
+                else
+                {
+                    Assert.Empty(Directory.GetDirectories(tempDir));
+                    Assert.Empty(Directory.GetFiles(tempDir, "log.log", SearchOption.AllDirectories));
+                }

---

1001-1001: Restore ThrowExceptions to its prior value.

Avoid leaking global state to other tests.

-                LogManager.ThrowExceptions = false;
+                var prevThrow = LogManager.ThrowExceptions;
+                LogManager.ThrowExceptions = false;

And in finally:

-                LogManager.ThrowExceptions = true;
+                LogManager.ThrowExceptions = prevThrow;

---

1029-1030: Remove ineffective file cleanup.

logfile contains ${guid} and won’t match actual files. Directory cleanup below already removes them.

-                if (File.Exists(logfile))
-                    File.Delete(logfile);

src/NLog/Targets/FileTarget.cs (1)

1231-1259: Guard against null/empty directory names before CreateDirectory.

Path.GetDirectoryName(filePath) can be null/empty in edge cases; be defensive to avoid ArgumentNullException.

-                var directoryName = Path.GetDirectoryName(filePath);
-
-                try
-                {
-                    Directory.CreateDirectory(directoryName);
-                }
+                var directoryName = Path.GetDirectoryName(filePath);
+                try
+                {
+                    if (string.IsNullOrEmpty(directoryName))
+                        throw; // No directory part to create; rethrow original DirectoryNotFoundException
+                    Directory.CreateDirectory(directoryName);
+                }
                 catch (Exception ex)
                 {
                     // If creating a directory failed, don't retry for this message
                     throw new NLogRuntimeException($"Could not create directory {directoryName}", ex);
                 }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/NLog/Targets/FileTarget.cs (1)

1231-1258: Harden directory creation: guard null dir, preserve retry on transient IO, fix log text.

Two small robustness tweaks:

• Handle Path.GetDirectoryName returning null/empty (rare but possible) to avoid wrapping an ArgumentNullException and to let the outer retry handle it.
• If Directory.CreateDirectory throws an IOException (transient/lock/share issues), rethrow it so CreateFileStreamWithRetry’s retry-loop can kick in. Keep NLogRuntimeException for non-IO errors.
• Nit: log says “FileName”; it’s a path.

Apply:

-                InternalLogger.Debug("{0}: DirectoryNotFoundException - Attempting to create directory for FileName: {1}", this, filePath);
+                InternalLogger.Debug("{0}: DirectoryNotFoundException - Attempting to create directory for FilePath: {1}", this, filePath);

-                var directoryName = Path.GetDirectoryName(filePath);
+                var directoryName = Path.GetDirectoryName(filePath);
+                if (string.IsNullOrEmpty(directoryName))
+                {
+                    // Nothing to create; let outer retry-logic decide.
+                    throw;
+                }

                 try
                 {
-                    Directory.CreateDirectory(directoryName);
+                    Directory.CreateDirectory(directoryName);
                 }
-                catch (Exception ex)
+                catch (IOException)
                 {
-                    // If creating a directory failed, don't retry for this message
-                    throw new NLogRuntimeException($"Could not create directory {directoryName}", ex);
+                    // Allow the outer retry-loop in CreateFileStreamWithRetry to handle transient IO failures
+                    throw;
+                }
+                catch (Exception ex)
+                {
+                    // Non-IO errors won't benefit from retry; keep current behavior
+                    throw new NLogRuntimeException($"Could not create directory '{directoryName}' for '{filePath}'", ex);
                 }

Would you like me to add/adjust a unit test that asserts the very first write succeeds when the directory does not exist (with minimal race via multiple concurrent loggers)?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/NLog/Targets/FileTarget.cs (1)

1231-1258: Guard null/empty directory names and preserve original exception; improve debug context

If Path.GetDirectoryName(filePath) returns null/empty, Directory.CreateDirectory will throw ArgumentNullException, masking the original DirectoryNotFoundException. Also, logging the target directory aids diagnostics.

Apply:

-        private Stream CreateFileStreamWithDirectory(string filePath, int bufferSize, bool initialFileOpen)
+        private Stream CreateFileStreamWithDirectory(string filePath, int bufferSize, bool initialFileOpen)
         {
             try
             {
                 return OpenNewFileStream(filePath, bufferSize, initialFileOpen);
             }
-            catch (DirectoryNotFoundException)
+            catch (DirectoryNotFoundException dnfe)
             {
                 if (!CreateDirs)
                     throw;
 
-                InternalLogger.Debug("{0}: DirectoryNotFoundException - Attempting to create directory for file: {1}", this, filePath);
-
-                var directoryName = Path.GetDirectoryName(filePath);
+                var directoryName = Path.GetDirectoryName(filePath);
+                if (string.IsNullOrEmpty(directoryName))
+                {
+                    // No directory component to create; keep original stack/exception.
+                    throw;
+                }
+                InternalLogger.Debug("{0}: DirectoryNotFoundException - Attempting to create directory: {1} for file: {2}", this, directoryName, filePath);
 
                 try
                 {
                     Directory.CreateDirectory(directoryName);
                 }
                 catch (Exception ex)
                 {
-                    // If creating a directory failed, don't retry for this message
-                    throw new NLogRuntimeException($"Could not create directory {directoryName}", ex);
+                    // If creating a directory failed, don't retry for this message
+                    throw new NLogRuntimeException($"Could not create directory '{directoryName}' for file '{filePath}'", ex);
                 }
 
                 return OpenNewFileStream(filePath, bufferSize, initialFileOpen);
             }
         }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro