Skip to content

Commit

Permalink
Trace2FileWriter: ignore some exceptions on write (#1340)
Browse files Browse the repository at this point in the history
These exceptions were discovered while exploring trace2 settings with a
full Git client.

Git can take a directory location as a trace2 target and will create new
files for every process. GCM currently throws a
DirectoryNotFoundException given such a parameter.

Git processes somehow can append to the same file across multiple
subprocesses. When GCM attempts to append to the file, it gets an
UnauthorizedAccessException on Windows due to multiple writers being
problematic. This exception could also happen on other platforms if the
setting is pointing to a file with restricted permissions.

In both of these cases, we chose to do nothing. The traces are lost, but
that's better than crashing the process.

Future directions could include:

1. Sending a warning over stderr if these exceptions occur, to make it
clear why trace2 are not showing up.

2. Directories could be noticed as a different kind of trace target and
we create a new file for the process before passing it to the
Trace2FileWriter.

3. Perhaps there is a way for Git to pass the handle to the trace file
so we can append to the file that Git was using. (Alternatively, Git
could close the file and then reopen it after running the GCM
subprocess.)

Each of these issues are a bit complicated, so this quick fix is chosen
as a stop gap to avoid this problem for current users.
  • Loading branch information
derrickstolee authored Jul 17, 2023
2 parents d6035ef + 92cc905 commit cc77173
Showing 1 changed file with 18 additions and 1 deletion.
19 changes: 18 additions & 1 deletion src/shared/Core/Trace2FileWriter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO;
using System;

namespace GitCredentialManager;

Expand All @@ -13,6 +14,22 @@ public Trace2FileWriter(Trace2FormatTarget formatTarget, string path) : base(for

public override void Write(Trace2Message message)
{
File.AppendAllText(_path, Format(message));
try
{
File.AppendAllText(_path, Format(message));
}
catch (DirectoryNotFoundException)
{
// Do nothing, as this either means we don't have the
// parent directories above the file, or this trace2
// target points to a directory.
}
catch (UnauthorizedAccessException)
{
// Do nothing, as this either means the file is not
// accessible with current permissions, or we are on
// Windows and the file is currently open for writing
// by another process (likely Git itself.)
}
}
}

0 comments on commit cc77173

Please sign in to comment.