You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Description:
Both retry helpers end with an unreachable return default;:
privateT?ExecuteWithRetry<T>(Func<T>action){for(inti=0;i<SyncMaxRetries;i++){try{returnaction();}catch(SQLiteExceptionex)when(...){if(i==SyncMaxRetries-1){ ...;throw;}
...}}returndefault;// unreachable — last iteration either returns or throws}
SyncMaxRetries and MaxRetries are both private const int = 3, so the loop always runs at least once. The terminal iteration either returns the action's result or rethrows after logging — control never falls out of the loop. The same pattern is present in ExecuteWithRetryAsync at line 104.
The return default is dead code today and only exists to satisfy the compiler's flow analysis that doesn't know the constant is non-zero. Two minor risks:
Code reviewers reading the method need to mentally prove the line is unreachable, which is friction.
Suggested fix:
Replace the dead line with an explicit throw new InvalidOperationException(\"...\"). That preserves the unreachable-by-construction guarantee for the current const = 3 case, and converts a future 0 into a loud failure instead of a silent default.
thrownewInvalidOperationException(\"Retryloop exited without returning or rethrowing — retry count must be >0.\");
Severity: Info
File:
src/Servy.Infrastructure/Data/DapperExecutor.csLines: 76 (
ExecuteWithRetry), 104 (ExecuteWithRetryAsync)Description:
Both retry helpers end with an unreachable
return default;:SyncMaxRetriesandMaxRetriesare bothprivate const int = 3, so the loop always runs at least once. The terminal iteration either returns the action's result or rethrows after logging — control never falls out of the loop. The same pattern is present inExecuteWithRetryAsyncat line 104.The
return defaultis dead code today and only exists to satisfy the compiler's flow analysis that doesn't know the constant is non-zero. Two minor risks:SyncMaxRetriesto a parameter or config value (issue [Maintainability] Timing/retry magic numbers scattered across LogTailer, ServiceHelper, DapperExecutor, ProcessHelper, RotatingStreamWriter — consolidate into AppConfig #818 is already pushing toward consolidating timing/retry magic numbers inAppConfig). The day someone passes0, callers silently get adefault(T)(null for reference types, 0 for ints) instead of a proper exception, which is a much worse failure mode than the current rethrow.Suggested fix:
Replace the dead line with an explicit
throw new InvalidOperationException(\"...\"). That preserves the unreachable-by-construction guarantee for the currentconst = 3case, and converts a future0into a loud failure instead of a silentdefault.Apply the same change at lines 76 and 104.