Severity: Info
File: src/Servy.Core/Native/NativeMethods.cs lines 779–811
Finding
public static FileIdentity GetFileIdentity(FileStream fs)
{
var identity = new FileIdentity();
try
{
if (GetFileInformationByHandle(fs.SafeFileHandle.DangerousGetHandle(), out var info))
{
identity.VolumeSerialNumber = info.VolumeSerialNumber;
identity.FileIndex = ((ulong)info.FileIndexHigh << 32) | info.FileIndexLow;
identity.IsValidHandleInfo = true;
}
}
catch
{
// Fallback or log failure here if necessary
}
try
{
long origPos = fs.Position;
fs.Seek(0, SeekOrigin.Begin);
byte[] buffer = new byte[64];
int read = fs.Read(buffer, 0, buffer.Length);
identity.PrefixHash = Convert.ToBase64String(buffer, 0, read);
fs.Seek(origPos, SeekOrigin.Begin);
}
catch
{
// Fallback or log failure here if necessary
}
return identity;
}
Two empty catch blocks, both annotated with the identical placeholder comment // Fallback or log failure here if necessary. This is a TODO that has never been actioned. GetFileIdentity is used by LogTailer to detect log rotation; if the file-handle probe or the prefix-hash probe silently fails (handle was closed, file was truncated, file was locked), the returned identity is a default-initialized struct — IsValidHandleInfo false, PrefixHash null — and the rotation-detection caller will treat it as 'no identity available' without ever knowing why. That is the textbook 3am operator finding: a tail log that mysteriously stops following after a rotation, with no log line indicating which probe failed.
GetFileInformationByHandle is a P/Invoke into kernel32; the only realistic exceptions are wrapper-induced (ObjectDisposedException from fs.SafeFileHandle, AccessViolationException). Both deserve a log line.
Suggested fix
Replace both empty catches with a debug-level log, and at minimum capture the exception type:
catch (Exception ex) when (LogAndContinue(ex, "GetFileInformationByHandle probe failed"))
{
// already logged via filter
}
…or inject IServyLogger and call _logger.Debug($"GetFileIdentity prefix-hash probe failed: {ex.GetType().Name} {ex.Message}") so the call sites at least surface a single diagnostic line per probe failure.
The TODO comment can also be removed once the decision is made — leaving placeholder TODOs in production code is itself a maintenance smell.
Severity: Info
File:
src/Servy.Core/Native/NativeMethods.cslines 779–811Finding
Two empty catch blocks, both annotated with the identical placeholder comment
// Fallback or log failure here if necessary. This is a TODO that has never been actioned.GetFileIdentityis used byLogTailerto detect log rotation; if the file-handle probe or the prefix-hash probe silently fails (handle was closed, file was truncated, file was locked), the returned identity is a default-initialized struct —IsValidHandleInfofalse,PrefixHashnull — and the rotation-detection caller will treat it as 'no identity available' without ever knowing why. That is the textbook 3am operator finding: a tail log that mysteriously stops following after a rotation, with no log line indicating which probe failed.GetFileInformationByHandleis a P/Invoke into kernel32; the only realistic exceptions are wrapper-induced (ObjectDisposedExceptionfromfs.SafeFileHandle,AccessViolationException). Both deserve a log line.Suggested fix
Replace both empty catches with a debug-level log, and at minimum capture the exception type:
…or inject
IServyLoggerand call_logger.Debug($"GetFileIdentity prefix-hash probe failed: {ex.GetType().Name} {ex.Message}")so the call sites at least surface a single diagnostic line per probe failure.The TODO comment can also be removed once the decision is made — leaving placeholder TODOs in production code is itself a maintenance smell.