Skip to content

Prevent EditAndContinueFeedbackDiagnosticFileProvider from throwing an exception in Razor scenarios #68619

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 3 commits into from
Jun 14, 2023
Merged

Prevent EditAndContinueFeedbackDiagnosticFileProvider from throwing an exception in Razor scenarios #68619

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01406ff
Don't try to create FileSystemWatcher in Razor scenarios
allisonchou Jun 14, 2023
3801cc8
Add condition
allisonchou Jun 14, 2023
dd2858e
Add condition
allisonchou Jun 14, 2023
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
12 changes: 10 additions & 2 deletions src/VisualStudio/Core/Def/EditAndContinue/EditAndContinueFeedbackDiagnosticFileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ internal sealed class EditAndContinueFeedbackDiagnosticFileProvider : IFeedbackD
/// Watching the file is currently the only way to detect the feedback session.
/// </summary>
private readonly string _vsFeedbackSemaphoreFullPath;
private readonly FileSystemWatcher _vsFeedbackSemaphoreFileWatcher;
private readonly FileSystemWatcher? _vsFeedbackSemaphoreFileWatcher;

private readonly int _vsProcessId;
private readonly DateTime _vsProcessStartTime;
Expand All @@ -65,6 +65,12 @@ public EditAndContinueFeedbackDiagnosticFileProvider(
var vsFeedbackTempDir = Path.Combine(_tempDir, VSFeedbackSemaphoreDir);
_vsFeedbackSemaphoreFullPath = Path.Combine(vsFeedbackTempDir, VSFeedbackSemaphoreFileName);

// Directory may not exist in scenarios such as Razor integration tests
if (!Directory.Exists(vsFeedbackTempDir))
Copy link
Member

@tmat tmat Jun 14, 2023

Choose a reason for hiding this comment

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

File watcher throws when the directory does not exist? What is the exact exception trace?
That'd be unfortunate since the intent was to detect when the file in that directory is created. The directory indeed might not exist at this time because VS Feedback didn't created it yet.

Copy link
Contributor Author

@allisonchou allisonchou Jun 14, 2023

Choose a reason for hiding this comment

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

Integration tests are struggling to run locally on my machine right now (trying to figure it out) so I don't have the exact stack trace at the moment, but the exception was:

ArgumentException: The directory name C:\Users\allichou\AppData\Local\Temp\Microsoft\VSFeedbackCollector is invalid.

and it was definitely originating from trying to initialize EditAndContinueFeedbackDiagnosticFileProvider. Looking at the constructor code, it seems the most likely culprit is from FileSystemWatcher, as the documentation for the FileSystemWatcher constructor says it can throw:

        //   T:System.ArgumentException:
        //     The path parameter is an empty string (""). -or- The path specified through the
        //     path parameter does not exist.
        //

Copy link
Member

@tmat tmat Jun 14, 2023

Choose a reason for hiding this comment

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

I see. ok.

{
return;
}

_vsFeedbackSemaphoreFileWatcher = new FileSystemWatcher(vsFeedbackTempDir, VSFeedbackSemaphoreFileName);
_vsFeedbackSemaphoreFileWatcher.Created += (_, _) => OnFeedbackSemaphoreCreatedOrChanged();
_vsFeedbackSemaphoreFileWatcher.Changed += (_, _) => OnFeedbackSemaphoreCreatedOrChanged();
Expand Down Expand Up @@ -93,7 +99,9 @@ private string GetZipFilePath()
=> Path.Combine(Path.Combine(_tempDir, $"EnC_{_vsProcessId}", ZipFileName));

public IReadOnlyCollection<string> GetFiles()
=> new[] { GetZipFilePath() };
=> _vsFeedbackSemaphoreFileWatcher is null
? Array.Empty<string>()
: (IReadOnlyCollection<string>)(new[] { GetZipFilePath() });

private void OnFeedbackSemaphoreCreatedOrChanged()
{
Expand Down