fix: IOException thrown issue from HtmlPostProcessor #10637
Conversation
|
Thank you for working on this! |
|
Thanks for looking into this. |
|
(Another try to generate my API doc and this time I disabled |
There was a problem hiding this comment.
Pull Request Overview
This PR is intended to address an IOException issue from the HtmlPostProcessor by adding retry logic and additional logging to diagnose file locking problems.
- Introduces retry logic (3 attempts with increasing delays) for file creation in ManifestFileWriter.cs.
- Adds a new FileLockCheck.cs file that leverages Win32 APIs to detect processes locking a file.
- Adds a new warning code (LockedFile) to support the additional logging and updates the project file to allow unsafe code blocks.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Docfx.Common/Loggers/WarningCodes.cs | New warning constant for file locking detection |
| src/Docfx.Common/FileLockCheck.cs | New file for detecting processes locking a file using Win32 APIs |
| src/Docfx.Common/FileAbstractLayer/ManifestFileWriter.cs | Implemented retry logic on file creation with added logging |
| src/Docfx.Common/Docfx.Common.csproj | Enabled unsafe code blocks |
| Retry: | ||
| try | ||
| { | ||
| Directory.CreateDirectory( | ||
| Path.Combine(_manifestFolder, file.RemoveWorkingFolder().GetDirectoryPath())); | ||
| var result = File.Create(Path.Combine(_manifestFolder, file.RemoveWorkingFolder())); | ||
| var fileStream = File.Create(path); | ||
| entry.LinkToPath = null; | ||
| return result; | ||
| return fileStream; | ||
| } | ||
| else | ||
| catch (IOException e) when ((e.HResult & 0x0000FFFF) == 32) // ERROR_SHARING_VIOLATION: 0x80070020 | ||
| { | ||
| var path = Path.Combine(OutputFolder, file.RemoveWorkingFolder()); | ||
| Directory.CreateDirectory(Path.GetDirectoryName(path)); | ||
| var result = File.Create(path); | ||
| entry.LinkToPath = path; | ||
| return result; | ||
| // If retry failed 3 times. throw exception | ||
| if (++retryCount > 3) | ||
| throw; | ||
|
|
||
| var sleepDelay = 500 * retryCount; | ||
|
|
||
| var message = FileLockCheck.GetLockingProcessNames(path); | ||
| if (string.IsNullOrEmpty(message)) | ||
| message = "File is locked by other process"; | ||
|
|
||
| Logger.LogWarning($"{message}. Retry after {sleepDelay}[ms]", file: path, code: WarningCodes.Build.LockedFile); | ||
| Thread.Sleep(500 * retryCount); | ||
| goto Retry; |
There was a problem hiding this comment.
[nitpick] Consider refactoring the retry logic to use a loop structure instead of a goto statement to improve code readability and maintainability.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I know using goto statement is not recommended generally.
But it's more readable for simple retry patterns.
(And for complex retry scenario. It's better to use library (e.g. Polly) )
filzrev
force-pushed
the
fix-issue-8654
branch
2 times, most recently
from
7464aff to
8101b80
Compare
This PR intended to fix #8654.
And contains following changes.
1. Add retry logics when HtmlPostProcessor throw IOException
Add retry logics to confirm behaviors. (Retry 3-times with delay
500ms/1000ms/1500ms0ms/500ms/1000ms)I don't sure this retry resolve reported issues though.
But it make possible to isolate the problem. file locking problem is transient issue or not.
2. Add additional logging when IOException is thrown
Add logics to find processes that lock target file by using Win32 APIs. and add warnings.
See:
https://github.com/dotnet/runtime/issues/109927for details.It can detect when file is locked by external process (e.g. Windows Defender) or locked by self process.