-
-
Notifications
You must be signed in to change notification settings - Fork 20
fix: Stop deleting estimation files on transient errors #1721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,13 +40,22 @@ public async Task<IEnumerable<SubModuleEstimation>> GetSubModuleEstimatedTimesAs | |
| { | ||
| try | ||
| { | ||
| var name = Path.GetFileNameWithoutExtension(file.FullName).Split("-Sub-")[1]; | ||
| var fileNameWithoutExtension = Path.GetFileNameWithoutExtension(file.FullName); | ||
| var subIndex = fileNameWithoutExtension.IndexOf("-Sub-", StringComparison.Ordinal); | ||
|
|
||
| if (subIndex < 0) | ||
| { | ||
| // File doesn't match expected naming pattern - skip gracefully | ||
| return null; | ||
| } | ||
|
|
||
| var name = fileNameWithoutExtension[(subIndex + 5)..]; // 5 = length of "-Sub-" | ||
| var time = await GetEstimatedTimeAsync(file.FullName).ConfigureAwait(false); | ||
| return new SubModuleEstimation(name, time); | ||
| } | ||
| catch | ||
| catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or System.Security.SecurityException) | ||
| { | ||
| File.Delete(file.FullName); | ||
| // File access error (locked, permissions, etc.) - skip gracefully without deleting | ||
| return null; | ||
| } | ||
|
Comment on lines
41
to
60
|
||
| }) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching only IOException is too narrow and may cause unhandled exceptions. The GetEstimatedTimeAsync method on line 53 can throw FormatException (from TimeSpan.Parse on line 83), and File.ReadAllTextAsync can throw other exceptions beyond IOException such as UnauthorizedAccessException, NotSupportedException, ArgumentException, etc. Since the SafeModuleEstimatedTimeProvider wrapper already catches all exceptions and logs them, it's safer to catch Exception here to prevent any parsing or I/O error from propagating.