AppEnvironmentWrapper - Return Unknown_ProcessId when empty ProcessName - #6148

src/NLog/Internal/AppEnvironmentWrapper.cs (1)

233-242: Reuse LookupEntryAssemblyFriendlyName() instead of reimplementing it.

Lines 374-379 now duplicate the basename extraction that already lives in LookupEntryAssemblyFriendlyName(). Keeping that normalization in one helper will make future fallback tweaks less error-prone.

♻️ Proposed refactor

-                processName = LookupEntryAssemblyFileName();
-                if (!string.IsNullOrEmpty(processName))
-                {
-                    var friendlyName = Path.GetFileNameWithoutExtension(processName);
-                    return string.IsNullOrEmpty(friendlyName) ? processName : friendlyName;
-                }
+                processName = LookupEntryAssemblyFriendlyName();
+                if (!string.IsNullOrEmpty(processName))
+                    return processName;

Also applies to: 374-379

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Internal/AppEnvironmentWrapper.cs` around lines 233 - 242, Replace
the duplicated basename extraction with a call to the existing helper
LookupEntryAssemblyFriendlyName(): locate the block that manually computes
Path.GetFileNameWithoutExtension(fileName) (the duplicate logic present around
the other assembly lookup) and remove the reimplementation, invoking
LookupEntryAssemblyFriendlyName() instead and returning its result (or handling
its null) so normalization is centralized in LookupEntryAssemblyFriendlyName().

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/NLog/Internal/AppEnvironmentWrapper.cs (1)

297-304: Inconsistent fallback chain in catch block.

The try block has three fallbacks (managed → native → assembly location), but the catch block only falls back to LookupCurrentProcessFilePathNative(), missing the assembly location fallback added at line 294. This means behavior differs depending on whether the managed lookup returns empty vs throws.

Consider aligning the catch block with the try block's fallback chain for consistency:

♻️ Suggested fix

             catch (Exception ex)
             {
                 if (ex.MustBeRethrownImmediately())
                     throw;

                 InternalLogger.Debug("LookupCurrentProcessFilePath Failed - {0}", ex.Message);
-                return LookupCurrentProcessFilePathNative();
+                var processFilePath = LookupCurrentProcessFilePathNative();
+                if (string.IsNullOrEmpty(processFilePath))
+                    processFilePath = AssemblyHelpers.GetAssemblyFileLocation(System.Reflection.Assembly.GetEntryAssembly());
+                return processFilePath ?? string.Empty;
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Internal/AppEnvironmentWrapper.cs` around lines 297 - 304, The catch
block in LookupCurrentProcessFilePath currently returns
LookupCurrentProcessFilePathNative() directly, creating an inconsistent fallback
chain versus the try path (managed → native → assembly location); update the
catch to mirror the try's fallback by calling
LookupCurrentProcessFilePathNative() and if that returns null/empty then call
LookupCurrentProcessFilePathAssemblyLocation(), e.g. capture the native result
and return it unless empty, otherwise return
LookupCurrentProcessFilePathAssemblyLocation(); keep the existing
MustBeRethrownImmediately() check and InternalLogger.Debug call.

src/NLog/Internal/AppEnvironmentWrapper.cs (1)

64-64: ⚠️ Potential issue | 🟠 Major

Return the exact sentinel here.

This still appends the PID, so CurrentProcessBaseName no longer matches the PR contract and changes downstream ${processname} / fallback filename output for callers expecting the literal Unknown_ProcessId.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Internal/AppEnvironmentWrapper.cs` at line 64, The
CurrentProcessBaseName getter currently appends the PID to the sentinel,
breaking the contract; change the null-coalescing fallback so that
_currentProcessBaseName is set to the exact sentinel "Unknown_ProcessId" when
LookupCurrentProcessNameWithFallback() returns null (i.e., replace the
$"Unknown_ProcessId_{CurrentProcessId}" fallback with the exact string
"Unknown_ProcessId" in the CurrentProcessBaseName property that calls
LookupCurrentProcessNameWithFallback()).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes