Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle UnauthorizedAccessExceptions in DirectoryHelper #8219

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

DustinCampbell
Copy link
Member

@DustinCampbell DustinCampbell commented Feb 3, 2023

When recursively digging through directories to find project configuration files (i.e. project.razor.json), we didn't handle any UnauthorizedAccessExceptions that might be thrown. So, if some sub-directory fails due to permissions access, the entire search would fail, even if a configuration file had already been found.

Beyond the bug fix (which is small), I did a bit of clean up.

When recursively digging through directories to find project
configuration files (i.e. `project.razor.json`), we didn't handle any
`UnauthorizedAccessExceptions` that might be thrown. So, if some
sub-directory fails due to permissions access, the entire search would
fail, even if a configuration file had already been found.
@DustinCampbell DustinCampbell requested a review from a team as a code owner February 3, 2023 21:52
@@ -128,14 +118,18 @@ protected virtual void OnInitializationFinished()
// Protected virtual for testing
protected virtual IEnumerable<string> GetExistingConfigurationFiles(string workspaceDirectory)
{
var files = DirectoryHelper.GetFilteredFiles(workspaceDirectory, _languageServerFeatureOptions.ProjectConfigurationFileName, s_ignoredDirectories, logger: _logger);
using var _ = _logger.BeginScope("Searching for existing project configuration files");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want to merge #8233 first or cherry-pick it onto this PR.

It was silly of me to write it that way at the time just because we weren't using Scopes yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also of note though, most of the loggers NoOp for BeginScope, so this might not produce the useful info you imagine (though it will be nice to "come online" if we complete #8232).

@DustinCampbell DustinCampbell merged commit e92db20 into dotnet:main Feb 8, 2023
@DustinCampbell DustinCampbell deleted the handle-io-exceptions branch February 8, 2023 23:33
maryamariyan added a commit that referenced this pull request Jul 20, 2023
maryamariyan added a commit that referenced this pull request Jul 20, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants