Skip to content

Commit 2269484

Browse files
lzybkrlzybkr
committed
Better error for history file access issues
When there is an IO error trying to create the directory for the history file, or some other sort of error trying to read or write the history file, we shouldn't tell the user to report a bug. Instead, the error will just report the exception message, and if it happens twice, suggest some solutions. The error will not be reported more than twice, because that would be too annoying. Fixes #322
1 parent 50b0bda commit 2269484

File tree

3 files changed

+127
-72
lines changed

3 files changed

+127
-72
lines changed

‎PSReadLine/History.cs

Lines changed: 96 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -105,110 +105,135 @@ private void SaveHistoryAtExit()
105105
WriteHistoryRange(0, _history.Count - 1, File.CreateText);
106106
}
107107

108-
private void WriteHistoryRange(int start, int end, Func<string, StreamWriter> fileOpener)
108+
109+
private int historyErrorReportedCount;
110+
private void ReportHistoryFileError(Exception e)
111+
{
112+
if (historyErrorReportedCount == 2)
113+
return;
114+
115+
historyErrorReportedCount += 1;
116+
var fgColor = Console.ForegroundColor;
117+
var bgColor = Console.BackgroundColor;
118+
Console.ForegroundColor = Options.ErrorForegroundColor;
119+
Console.WriteLine(PSReadLineResources.HistoryFileErrorMessage, Options.HistorySavePath, e.Message);
120+
if (historyErrorReportedCount == 2)
121+
{
122+
Console.WriteLine(PSReadLineResources.HistoryFileErrorFinalMessage);
123+
}
124+
Console.ForegroundColor = fgColor;
125+
Console.BackgroundColor = bgColor;
126+
}
127+
128+
private bool WithHistoryFileMutexDo(int timeout, Action action)
109129
{
110-
if (_historyFileMutex.WaitOne(100))
130+
if (_historyFileMutex.WaitOne(timeout))
111131
{
112132
try
113133
{
114-
MaybeReadHistoryFile();
134+
action();
135+
}
136+
catch (UnauthorizedAccessException uae)
137+
{
138+
ReportHistoryFileError(uae);
139+
return false;
140+
}
141+
catch (IOException ioe)
142+
{
143+
ReportHistoryFileError(ioe);
144+
return false;
145+
}
146+
finally
147+
{
148+
_historyFileMutex.ReleaseMutex();
149+
}
150+
}
151+
152+
// No errors to report, so consider it a success even if we timed out on the mutex.
153+
return true;
154+
}
115155

116-
bool retry = true;
156+
private void WriteHistoryRange(int start, int end, Func<string, StreamWriter> fileOpener)
157+
{
158+
WithHistoryFileMutexDo(100, () =>
159+
{
160+
if (!MaybeReadHistoryFile())
161+
return;
162+
163+
bool retry = true;
117164
retry_after_creating_directory:
118-
try
119-
{
120-
using (var file = fileOpener(Options.HistorySavePath))
121-
{
122-
for (var i = start; i <= end; i++)
123-
{
124-
_history[i]._saved = true;
125-
var line = _history[i]._line.Replace("\n", "`\n");
126-
file.WriteLine(line);
127-
}
128-
}
129-
var fileInfo = new FileInfo(Options.HistorySavePath);
130-
_historyFileLastSavedSize = fileInfo.Length;
131-
}
132-
catch (DirectoryNotFoundException)
165+
try
166+
{
167+
using (var file = fileOpener(Options.HistorySavePath))
133168
{
134-
// Try making the directory, but just once
135-
if (retry)
169+
for (var i = start; i <= end; i++)
136170
{
137-
retry = false;
138-
Directory.CreateDirectory(Path.GetDirectoryName(Options.HistorySavePath));
139-
goto retry_after_creating_directory;
171+
_history[i]._saved = true;
172+
var line = _history[i]._line.Replace("\n", "`\n");
173+
file.WriteLine(line);
140174
}
141175
}
176+
var fileInfo = new FileInfo(Options.HistorySavePath);
177+
_historyFileLastSavedSize = fileInfo.Length;
142178
}
143-
finally
179+
catch (DirectoryNotFoundException)
144180
{
145-
_historyFileMutex.ReleaseMutex();
181+
// Try making the directory, but just once
182+
if (retry)
183+
{
184+
retry = false;
185+
Directory.CreateDirectory(Path.GetDirectoryName(Options.HistorySavePath));
186+
goto retry_after_creating_directory;
187+
}
146188
}
147-
}
189+
});
148190
}
149191

150-
private void MaybeReadHistoryFile()
192+
private bool MaybeReadHistoryFile()
151193
{
152194
if (Options.HistorySaveStyle == HistorySaveStyle.SaveIncrementally)
153195
{
154-
if (_historyFileMutex.WaitOne(1000))
196+
return WithHistoryFileMutexDo(1000, () =>
155197
{
156-
try
198+
var fileInfo = new FileInfo(Options.HistorySavePath);
199+
if (fileInfo.Length != _historyFileLastSavedSize)
157200
{
158-
try
201+
var historyLines = new List<string>();
202+
using (var fs = new FileStream(Options.HistorySavePath, FileMode.Open))
203+
using (var sr = new StreamReader(fs))
159204
{
160-
var fileInfo = new FileInfo(Options.HistorySavePath);
161-
if (fileInfo.Length != _historyFileLastSavedSize)
162-
{
163-
var historyLines = new List<string>();
164-
using (var fs = new FileStream(Options.HistorySavePath, FileMode.Open))
165-
using (var sr = new StreamReader(fs))
166-
{
167-
fs.Seek(_historyFileLastSavedSize, SeekOrigin.Begin);
168-
169-
while (!sr.EndOfStream)
170-
{
171-
historyLines.Add(sr.ReadLine());
172-
}
173-
}
174-
UpdateHistoryFromFile(historyLines, fromDifferentSession: true);
205+
fs.Seek(_historyFileLastSavedSize, SeekOrigin.Begin);
175206

176-
_historyFileLastSavedSize = fileInfo.Length;
207+
while (!sr.EndOfStream)
208+
{
209+
historyLines.Add(sr.ReadLine());
177210
}
178211
}
179-
catch (FileNotFoundException)
180-
{
181-
}
182-
}
183-
finally
184-
{
185-
_historyFileMutex.ReleaseMutex();
212+
UpdateHistoryFromFile(historyLines, fromDifferentSession: true);
213+
214+
_historyFileLastSavedSize = fileInfo.Length;
186215
}
187-
}
216+
});
188217
}
218+
219+
// true means no errors, not that we actually read the file
220+
return true;
189221
}
190222

191223
private void ReadHistoryFile()
192224
{
193-
if (_historyFileMutex.WaitOne(1000))
225+
WithHistoryFileMutexDo(1000, () =>
194226
{
195-
try
227+
if (!File.Exists(Options.HistorySavePath))
196228
{
197-
if (!File.Exists(Options.HistorySavePath))
198-
{
199-
return;
200-
}
201-
202-
var historyLines = File.ReadAllLines(Options.HistorySavePath);
203-
UpdateHistoryFromFile(historyLines, fromDifferentSession: false);
204-
var fileInfo = new FileInfo(Options.HistorySavePath);
205-
_historyFileLastSavedSize = fileInfo.Length;
206-
}
207-
finally
208-
{
209-
_historyFileMutex.ReleaseMutex();
229+
return;
210230
}
211-
}
231+
232+
var historyLines = File.ReadAllLines(Options.HistorySavePath);
233+
UpdateHistoryFromFile(historyLines, fromDifferentSession: false);
234+
var fileInfo = new FileInfo(Options.HistorySavePath);
235+
_historyFileLastSavedSize = fileInfo.Length;
236+
});
212237
}
213238

214239
void UpdateHistoryFromFile(IEnumerable<string> historyLines, bool fromDifferentSession)

‎PSReadLine/PSReadLineResources.Designer.cs

Lines changed: 21 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎PSReadLine/PSReadLineResources.resx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,4 +705,13 @@ If there are other parse errors, unresolved commands, or incorrect parameters, s
705705
<data name="InsertLineBelowDescription" xml:space="preserve">
706706
<value>Inserts a new empty line below the current line without attempting to execute the input</value>
707707
</data>
708-
</root>
708+
<data name="HistoryFileErrorMessage" xml:space="preserve">
709+
<value>Error reading or writing history file '{0}': {1}</value>
710+
</data>
711+
<data name="HistoryFileErrorFinalMessage" xml:space="preserve">
712+
<value>This error will not be reported again in this session. Consider using a different path with:
713+
Set-PSReadlineOption -HistorySavePath &lt;Path&gt;
714+
Or not saving history with:
715+
Set-PSReadlineOption -HistorySaveStyle SaveNothing</value>
716+
</data>
717+
</root>

0 commit comments

Comments
 (0)