Skip to content

Handle UnauthorizedAccessExceptions in DirectoryHelper #8219

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Handle UnauthorizedAccessExceptions in DirectoryHelper #8219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DirectoryHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ internal static IEnumerable<string> GetFilteredFiles(
// This can also happen if the directory is too long for windows.
files = Array.Empty<string>();
}
catch (UnauthorizedAccessException ex)
{
logger?.LogWarning("UnauthorizedAccess: {exception}", ex.Message);
yield break;
}
catch (PathTooLongException ex)
{
logger?.LogWarning("PathTooLong: {exception}", ex.Message);
Expand Down Expand Up @@ -91,6 +96,11 @@ internal static IEnumerable<string> GetFilteredFiles(
// This can also happen if the directory is too long for windows.
directories = Array.Empty<string>();
}
catch (UnauthorizedAccessException ex)
{
logger?.LogWarning("UnauthorizedAccess: {exception}", ex.Message);
yield break;
}
catch (PathTooLongException ex)
{
logger?.LogWarning("PathTooLong: {exception}", ex.Message);
Expand Down
30 changes: 17 additions & 13 deletions .../Microsoft.AspNetCore.Razor.LanguageServer/MonitorProjectConfigurationFilePathEndpoint.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer;

internal class MonitorProjectConfigurationFilePathEndpoint : IMonitorProjectConfigurationFilePathHandler, IDisposable
{
private readonly ProjectSnapshotManagerDispatcher _projectSnapshotManagerDispatcher;
private readonly ProjectSnapshotManagerDispatcher _dispatcher;
private readonly WorkspaceDirectoryPathResolver _workspaceDirectoryPathResolver;
private readonly IEnumerable<IProjectConfigurationFileChangeListener> _listeners;
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
private readonly LanguageServerFeatureOptions _options;
private readonly ILoggerFactory _loggerFactory;
private readonly ILogger _logger;
private readonly ConcurrentDictionary<string, (string ConfigurationDirectory, IFileChangeDetector Detector)> _outputPathMonitors;
private readonly object _disposeLock;
Expand All @@ -29,21 +30,22 @@ internal class MonitorProjectConfigurationFilePathEndpoint : IMonitorProjectConf
public bool MutatesSolutionState => false;

public MonitorProjectConfigurationFilePathEndpoint(
ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
ProjectSnapshotManagerDispatcher dispatcher,
WorkspaceDirectoryPathResolver workspaceDirectoryPathResolver,
IEnumerable<IProjectConfigurationFileChangeListener> listeners,
LanguageServerFeatureOptions languageServerFeatureOptions,
LanguageServerFeatureOptions options,
ILoggerFactory loggerFactory)
{
if (languageServerFeatureOptions is null)
if (options is null)
{
throw new ArgumentNullException(nameof(languageServerFeatureOptions));
throw new ArgumentNullException(nameof(options));
}

_projectSnapshotManagerDispatcher = projectSnapshotManagerDispatcher;
_dispatcher = dispatcher;
_workspaceDirectoryPathResolver = workspaceDirectoryPathResolver;
_listeners = listeners;
_languageServerFeatureOptions = languageServerFeatureOptions;
_options = options;
_loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory));
_logger = loggerFactory.CreateLogger<MonitorProjectConfigurationFilePathEndpoint>();
_outputPathMonitors = new ConcurrentDictionary<string, (string, IFileChangeDetector)>(FilePathComparer.Instance);
_disposeLock = new object();
Expand Down Expand Up @@ -72,7 +74,7 @@ public async Task HandleNotificationAsync(MonitorProjectConfigurationFilePathPar
return;
}

if (!request.ConfigurationFilePath.EndsWith(_languageServerFeatureOptions.ProjectConfigurationFileName, StringComparison.Ordinal))
if (!request.ConfigurationFilePath.EndsWith(_options.ProjectConfigurationFileName, StringComparison.Ordinal))
{
_logger.LogError("Invalid configuration file path provided for project '{0}': '{1}'", request.ProjectFilePath, request.ConfigurationFilePath);
return;
Expand Down Expand Up @@ -188,8 +190,10 @@ public void Dispose()
}

// Protected virtual for testing
protected virtual IFileChangeDetector CreateFileChangeDetector() => new ProjectConfigurationFileChangeDetector(
_projectSnapshotManagerDispatcher,
_listeners,
_languageServerFeatureOptions);
protected virtual IFileChangeDetector CreateFileChangeDetector()
=> new ProjectConfigurationFileChangeDetector(
_dispatcher,
_listeners,
_options,
_loggerFactory);
}
52 changes: 23 additions & 29 deletions ...r/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationFileChangeDetector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer;

internal class ProjectConfigurationFileChangeDetector : IFileChangeDetector
{
private readonly ProjectSnapshotManagerDispatcher _projectSnapshotManagerDispatcher;
private readonly ProjectSnapshotManagerDispatcher _dispatcher;
private readonly IEnumerable<IProjectConfigurationFileChangeListener> _listeners;
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
private readonly ILogger? _logger;
private readonly LanguageServerFeatureOptions _options;
private readonly ILogger _logger;
private FileSystemWatcher? _watcher;

private static readonly IReadOnlyCollection<string> s_ignoredDirectories = new string[]
Expand All @@ -29,30 +29,20 @@ internal class ProjectConfigurationFileChangeDetector : IFileChangeDetector
};

public ProjectConfigurationFileChangeDetector(
ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
ProjectSnapshotManagerDispatcher dispatcher,
IEnumerable<IProjectConfigurationFileChangeListener> listeners,
LanguageServerFeatureOptions languageServerFeatureOptions,
ILoggerFactory? loggerFactory = null)
LanguageServerFeatureOptions options,
ILoggerFactory loggerFactory)
{
if (projectSnapshotManagerDispatcher is null)
if (loggerFactory is null)
{
throw new ArgumentNullException(nameof(projectSnapshotManagerDispatcher));
throw new ArgumentNullException(nameof(loggerFactory));
}

if (listeners is null)
{
throw new ArgumentNullException(nameof(listeners));
}

if (languageServerFeatureOptions is null)
{
throw new ArgumentNullException(nameof(languageServerFeatureOptions));
}

_projectSnapshotManagerDispatcher = projectSnapshotManagerDispatcher;
_listeners = listeners;
_languageServerFeatureOptions = languageServerFeatureOptions;
_logger = loggerFactory?.CreateLogger<ProjectConfigurationFileChangeDetector>();
_dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(dispatcher));
_listeners = listeners ?? throw new ArgumentNullException(nameof(listeners));
_options = options ?? throw new ArgumentNullException(nameof(options));
_logger = loggerFactory.CreateLogger<ProjectConfigurationFileChangeDetector>();
}

public async Task StartAsync(string workspaceDirectory, CancellationToken cancellationToken)
Expand All @@ -67,7 +57,7 @@ public async Task StartAsync(string workspaceDirectory, CancellationToken cancel
workspaceDirectory = FilePathNormalizer.Normalize(workspaceDirectory);
var existingConfigurationFiles = GetExistingConfigurationFiles(workspaceDirectory);

await _projectSnapshotManagerDispatcher.RunOnDispatcherThreadAsync(() =>
await _dispatcher.RunOnDispatcherThreadAsync(() =>
{
foreach (var configurationFilePath in existingConfigurationFiles)
{
Expand All @@ -84,7 +74,7 @@ await _projectSnapshotManagerDispatcher.RunOnDispatcherThreadAsync(() =>
return;
}

_watcher = new RazorFileSystemWatcher(workspaceDirectory, _languageServerFeatureOptions.ProjectConfigurationFileName)
_watcher = new RazorFileSystemWatcher(workspaceDirectory, _options.ProjectConfigurationFileName)
{
NotifyFilter = NotifyFilters.FileName | NotifyFilters.LastWrite | NotifyFilters.CreationTime,
IncludeSubdirectories = true,
Expand All @@ -97,12 +87,12 @@ await _projectSnapshotManagerDispatcher.RunOnDispatcherThreadAsync(() =>
{
// Translate file renames into remove / add

if (args.OldFullPath.EndsWith(_languageServerFeatureOptions.ProjectConfigurationFileName, FilePathComparison.Instance))
if (args.OldFullPath.EndsWith(_options.ProjectConfigurationFileName, FilePathComparison.Instance))
{
// Renaming from project configuration file to something else. Just remove the configuration file.
FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.OldFullPath, RazorFileChangeKind.Removed);
}
else if (args.FullPath.EndsWith(_languageServerFeatureOptions.ProjectConfigurationFileName, FilePathComparison.Instance))
else if (args.FullPath.EndsWith(_options.ProjectConfigurationFileName, FilePathComparison.Instance))
{
// Renaming from a non-project configuration file file to a real one. Just add the configuration file.
FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.FullPath, RazorFileChangeKind.Added);
Expand All @@ -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

@ryanbrandenburg ryanbrandenburg Feb 6, 2023

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

@ryanbrandenburg ryanbrandenburg Feb 6, 2023

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).


return files;
return DirectoryHelper.GetFilteredFiles(
workspaceDirectory,
_options.ProjectConfigurationFileName,
s_ignoredDirectories,
logger: _logger);
}

private void FileSystemWatcher_ProjectConfigurationFileEvent_Background(string physicalFilePath, RazorFileChangeKind kind)
{
_ = _projectSnapshotManagerDispatcher.RunOnDispatcherThreadAsync(
_ = _dispatcher.RunOnDispatcherThreadAsync(
() => FileSystemWatcher_ProjectConfigurationFileEvent(physicalFilePath, kind),
CancellationToken.None);
}
Expand Down
11 changes: 7 additions & 4 deletions ...rosoft.AspNetCore.Razor.LanguageServer.Test/ProjectConfigurationFileChangeDetectorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.AspNetCore.Razor.LanguageServer.Common;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.Extensions.Logging;
using Moq;
using Xunit;
using Xunit.Abstractions;
Expand Down Expand Up @@ -40,7 +41,8 @@ public async Task StartAsync_NotifiesListenersOfExistingConfigurationFiles()
cts,
Dispatcher,
new[] { listener1.Object, listener2.Object },
existingConfigurationFiles);
existingConfigurationFiles,
LoggerFactory);

// Act
await detector.StartAsync("/some/workspace+directory", cts.Token);
Expand Down Expand Up @@ -77,10 +79,11 @@ private class TestProjectConfigurationFileChangeDetector : ProjectConfigurationF

public TestProjectConfigurationFileChangeDetector(
CancellationTokenSource cancellationTokenSource,
ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
ProjectSnapshotManagerDispatcher dispatcher,
IEnumerable<IProjectConfigurationFileChangeListener> listeners,
IReadOnlyList<string> existingConfigurationFiles)
: base(projectSnapshotManagerDispatcher, listeners, TestLanguageServerFeatureOptions.Instance)
IReadOnlyList<string> existingConfigurationFiles,
ILoggerFactory loggerFactory)
: base(dispatcher, listeners, TestLanguageServerFeatureOptions.Instance, loggerFactory)
{
_cancellationTokenSource = cancellationTokenSource;
_existingConfigurationFiles = existingConfigurationFiles;
Expand Down