diff --git a/src/PowerShellEditorServices/Utility/Logging.cs b/src/PowerShellEditorServices/Utility/Logging.cs index f4320eb59..6813feacb 100644 --- a/src/PowerShellEditorServices/Utility/Logging.cs +++ b/src/PowerShellEditorServices/Utility/Logging.cs @@ -78,6 +78,21 @@ void WriteException( [CallerMemberName] string callerName = null, [CallerFilePath] string callerSourceFile = null, [CallerLineNumber] int callerLineNumber = 0); + + /// + /// Log an exception that has been handled cleanly or is otherwise not expected to cause problems in the logs. + /// + /// The error message of the exception to be logged. + /// The exception itself that has been thrown. + /// The name of the method in which the ILogger is being called. + /// The name of the source file in which the ILogger is being called. + /// The line number in the file where the ILogger is being called. + void WriteHandledException( + string errorMessage, + Exception exception, + [CallerMemberName] string callerName = null, + [CallerFilePath] string callerSourceFile = null, + [CallerLineNumber] int callerLineNumber = 0); } diff --git a/src/PowerShellEditorServices/Utility/PsesLogger.cs b/src/PowerShellEditorServices/Utility/PsesLogger.cs index b7ad8f6b2..b60ea603b 100644 --- a/src/PowerShellEditorServices/Utility/PsesLogger.cs +++ b/src/PowerShellEditorServices/Utility/PsesLogger.cs @@ -11,11 +11,22 @@ namespace Microsoft.PowerShell.EditorServices.Utility /// public class PsesLogger : ILogger { + /// + /// The standard log template for all log entries. + /// + private static readonly string s_logMessageTemplate = + "[{LogLevelName:l}] tid:{ThreadId} in '{CallerName:l}' {CallerSourceFile:l} (line {CallerLineNumber}):{IndentedLogMsg:l}"; + /// /// The name of the ERROR log level. /// private static readonly string ErrorLevelName = LogLevel.Error.ToString().ToUpper(); + /// + /// The name of the WARNING log level. + /// + private static readonly string WarningLevelName = LogLevel.Warning.ToString().ToUpper(); + /// /// The internal Serilog logger to log to. /// @@ -51,31 +62,46 @@ public void Write( [CallerMemberName] string callerName = null, [CallerFilePath] string callerSourceFile = null, [CallerLineNumber] int callerLineNumber = 0) + { + Write(logLevel, new StringBuilder(logMessage), callerName, callerSourceFile, callerLineNumber); + } + + /// + /// Write a message with the given severity to the logs. Takes a StringBuilder to allow for minimal allocation. + /// + /// The severity level of the log message. + /// The log message itself in StringBuilder form for manipulation. + /// The name of the calling method. + /// The name of the source file of the caller. + /// The line number where the log is being called. + private void Write( + LogLevel logLevel, + StringBuilder logMessage, + [CallerMemberName] string callerName = null, + [CallerFilePath] string callerSourceFile = null, + [CallerLineNumber] int callerLineNumber = 0) { string indentedLogMsg = IndentMsg(logMessage); string logLevelName = logLevel.ToString().ToUpper(); int threadId = Thread.CurrentThread.ManagedThreadId; - string messageTemplate = - "[{LogLevelName:l}] tid:{threadId} in '{CallerName:l}' {CallerSourceFile:l}:{CallerLineNumber}:{IndentedLogMsg:l}"; - switch (logLevel) { case LogLevel.Diagnostic: - _logger.Verbose(messageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); + _logger.Verbose(s_logMessageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); return; case LogLevel.Verbose: - _logger.Debug(messageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); + _logger.Debug(s_logMessageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); return; case LogLevel.Normal: - _logger.Information(messageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); + _logger.Information(s_logMessageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); return; case LogLevel.Warning: - _logger.Warning(messageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); + _logger.Warning(s_logMessageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); return; case LogLevel.Error: - _logger.Error(messageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); + _logger.Error(s_logMessageTemplate, logLevelName, threadId, callerName, callerSourceFile, callerLineNumber, indentedLogMsg); return; } } @@ -95,26 +121,63 @@ public void WriteException( [CallerFilePath] string callerSourceFile = null, [CallerLineNumber] int callerLineNumber = 0) { - string indentedException = IndentMsg(exception.ToString()); + StringBuilder body = FormatExceptionMessage("Exception", errorMessage, exception); + Write(LogLevel.Error, body, callerName, callerSourceFile, callerLineNumber); + } - _logger.Error("[{ErrorLevelName:l}] {CallerSourceFile:l}: In method '{CallerName:l}', line {CallerLineNumber}: {ErrorMessage:l}{IndentedException:l}", - ErrorLevelName, callerSourceFile, callerName, callerLineNumber, errorMessage, indentedException); + /// + /// Log an exception that has been handled cleanly or is otherwise not expected to cause problems in the logs. + /// + /// The error message of the exception to be logged. + /// The exception itself that has been thrown. + /// The name of the method in which the ILogger is being called. + /// The name of the source file in which the ILogger is being called. + /// The line number in the file where the ILogger is being called. + public void WriteHandledException( + string errorMessage, + Exception exception, + [CallerMemberName] string callerName = null, + [CallerFilePath] string callerSourceFile = null, + [CallerLineNumber] int callerLineNumber = 0) + { + StringBuilder body = FormatExceptionMessage("Handled exception", errorMessage, exception); + Write(LogLevel.Warning, body, callerName, callerSourceFile, callerLineNumber); } /// /// Utility function to indent a log message by one level. /// - /// The log message to indent. + /// Log message string builder to transform. /// The indented log message string. - private static string IndentMsg(string logMessage) + private static string IndentMsg(StringBuilder logMessageBuilder) { - return new StringBuilder(logMessage) + return logMessageBuilder .Replace(Environment.NewLine, s_indentedPrefix) .Insert(0, s_indentedPrefix) .AppendLine() .ToString(); } + /// + /// Creates a prettified log message from an exception. + /// + /// The user-readable tag for this exception entry. + /// The user-readable short description of the error. + /// The exception object itself. Must not be null. + /// An indented, formatted string of the body. + private static StringBuilder FormatExceptionMessage( + string messagePrelude, + string errorMessage, + Exception exception) + { + var sb = new StringBuilder() + .Append(messagePrelude).Append(": ").Append(errorMessage).Append(Environment.NewLine) + .Append(Environment.NewLine) + .Append(exception.ToString()); + + return sb; + } + /// /// A newline followed by a single indentation prefix. /// diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index ce0df719e..7dc328b41 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -22,6 +22,13 @@ public class Workspace { #region Private Fields + private static readonly string[] s_psFilePatterns = new [] + { + "*.ps1", + "*.psm1", + "*.psd1" + }; + private ILogger logger; private Version powerShellVersion; private Dictionary workspaceFiles = new Dictionary(); @@ -292,67 +299,107 @@ public IEnumerable EnumeratePSFiles() #region Private Methods + /// + /// Find PowerShell files recursively down from a given directory path. + /// Currently returns files in depth-first order. + /// Directory.GetFiles(folderPath, pattern, SearchOption.AllDirectories) would provide this, + /// but a cycle in the filesystem will cause that to enter an infinite loop. + /// + /// The absolute path of the base folder to search. + /// + /// All PowerShell files in the recursive directory hierarchy under the given base directory, up to 64 directories deep. + /// private IEnumerable RecursivelyEnumerateFiles(string folderPath) { - var foundFiles = Enumerable.Empty(); - var patterns = new string[] { @"*.ps1", @"*.psm1", @"*.psd1" }; + var foundFiles = new List(); + var dirStack = new Stack(); - try + // Kick the search off with the base directory + dirStack.Push(folderPath); + + const int recursionDepthLimit = 64; + while (dirStack.Any()) { - IEnumerable subDirs = Directory.GetDirectories(folderPath); - foreach (string dir in subDirs) + string currDir = dirStack.Pop(); + + // Look for any PowerShell files in the current directory + foreach (string pattern in s_psFilePatterns) { - foundFiles = - foundFiles.Concat( - RecursivelyEnumerateFiles(dir)); + string[] psFiles; + try + { + psFiles = Directory.GetFiles(currDir, pattern, SearchOption.TopDirectoryOnly); + } + catch (DirectoryNotFoundException e) + { + this.logger.WriteHandledException( + $"Could not enumerate files in the path '{currDir}' due to it being an invalid path", + e); + + continue; + } + catch (PathTooLongException e) + { + this.logger.WriteHandledException( + $"Could not enumerate files in the path '{currDir}' due to the path being too long", + e); + + continue; + } + catch (Exception e) when (e is SecurityException || e is UnauthorizedAccessException) + { + this.logger.WriteHandledException( + $"Could not enumerate files in the path '{currDir}' due to the path not being accessible", + e); + + continue; + } + + foundFiles.AddRange(psFiles); } - } - catch (DirectoryNotFoundException e) - { - this.logger.WriteException( - $"Could not enumerate files in the path '{folderPath}' due to it being an invalid path", - e); - } - catch (PathTooLongException e) - { - this.logger.WriteException( - $"Could not enumerate files in the path '{folderPath}' due to the path being too long", - e); - } - catch (Exception e) when (e is SecurityException || e is UnauthorizedAccessException) - { - this.logger.WriteException( - $"Could not enumerate files in the path '{folderPath}' due to the path not being accessible", - e); - } - foreach (var pattern in patterns) - { + // Prevent unbounded recursion here + // If we get too deep, keep processing but go no deeper + if (dirStack.Count >= recursionDepthLimit) + { + this.logger.Write(LogLevel.Warning, $"Recursion depth limit hit for path {folderPath}"); + continue; + } + + // Add the recursive directories to search next + string[] subDirs; try { - foundFiles = - foundFiles.Concat( - Directory.GetFiles( - folderPath, - pattern)); + subDirs = Directory.GetDirectories(currDir); } catch (DirectoryNotFoundException e) { - this.logger.WriteException( - $"Could not enumerate files in the path '{folderPath}' due to a path being an invalid path", + this.logger.WriteHandledException( + $"Could not enumerate directories in the path '{currDir}' due to it being an invalid path", e); + + continue; } catch (PathTooLongException e) { - this.logger.WriteException( - $"Could not enumerate files in the path '{folderPath}' due to a path being too long", + this.logger.WriteHandledException( + $"Could not enumerate directories in the path '{currDir}' due to the path being too long", e); + + continue; } catch (Exception e) when (e is SecurityException || e is UnauthorizedAccessException) { - this.logger.WriteException( - $"Could not enumerate files in the path '{folderPath}' due to a path not being accessible", + this.logger.WriteHandledException( + $"Could not enumerate directories in the path '{currDir}' due to the path not being accessible", e); + + continue; + } + + foreach (string subDir in subDirs) + { + dirStack.Push(subDir); } }