From 0118e29f47b7a67ff245644343b556317cd9dc7a Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Mon, 19 Oct 2026 21:14:45 +0000 Subject: [PATCH] Move SftpPathNotFoundException.Path down to SftpException And populate it in more places (where the error could not relate to any other path). --- src/Renci.SshNet/Common/SftpException.cs | 37 +++++++++++++++++-- .../Common/SftpPathNotFoundException.cs | 21 +---------- .../Common/SftpPermissionDeniedException.cs | 13 ++++++- src/Renci.SshNet/Sftp/SftpSession.cs | 24 ++++++------ .../SftpClientTest.CreateDirectory.cs | 7 +++- .../SftpClientTests.cs | 5 ++- .../Classes/Common/SftpExceptionTest.cs | 16 +++++++- 7 files changed, 81 insertions(+), 42 deletions(-) diff --git a/src/Renci.SshNet/Common/SftpException.cs b/src/Renci.SshNet/Common/SftpException.cs index c930aeb28..e1b5204fd 100644 --- a/src/Renci.SshNet/Common/SftpException.cs +++ b/src/Renci.SshNet/Common/SftpException.cs @@ -20,6 +20,20 @@ public class SftpException : SshException /// public StatusCode StatusCode { get; } + /// + /// Gets the path that is associated with this exception. + /// + /// + /// The path that the failed operation was performed on, or + /// if no path was passed to the constructor for this instance. + /// + /// + /// This is populated only for operations which act on a single path, such as + /// opening or deleting a file. It is for operations acting + /// on an open handle or on more than one path, such as renaming a file. + /// + public string? Path { get; } + /// /// Initializes a new instance of the class. /// @@ -46,15 +60,28 @@ public SftpException(StatusCode statusCode, string? message) /// The error message that explains the reason for the exception. /// The exception that is the cause of the current exception. public SftpException(StatusCode statusCode, string? message, Exception? innerException) - : base(string.IsNullOrEmpty(message) ? GetDefaultMessage(statusCode) : message, innerException) + : this(statusCode, message, path: null, innerException) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The status code that indicates the error that occurred. + /// The error message that explains the reason for the exception. + /// The path that the failed operation was performed on. + /// The exception that is the cause of the current exception. + public SftpException(StatusCode statusCode, string? message, string? path, Exception? innerException) + : base(string.IsNullOrEmpty(message) ? GetDefaultMessage(statusCode, path) : message, innerException) { StatusCode = statusCode; + Path = path; } - private protected static string GetDefaultMessage(StatusCode statusCode) + private static string GetDefaultMessage(StatusCode statusCode, string? path) { #pragma warning disable IDE0072 // Add missing cases - return statusCode switch + var message = statusCode switch { StatusCode.Ok => "The operation completed successfully.", StatusCode.NoSuchFile => "A reference was made to a file that does not exist.", @@ -65,6 +92,10 @@ private protected static string GetDefaultMessage(StatusCode statusCode) _ => statusCode.ToString() }; #pragma warning restore IDE0072 // Add missing cases + + return path is not null + ? $"{message} Path: '{path}'." + : message; } #if NETFRAMEWORK diff --git a/src/Renci.SshNet/Common/SftpPathNotFoundException.cs b/src/Renci.SshNet/Common/SftpPathNotFoundException.cs index 83fd904f0..f9101980a 100644 --- a/src/Renci.SshNet/Common/SftpPathNotFoundException.cs +++ b/src/Renci.SshNet/Common/SftpPathNotFoundException.cs @@ -15,15 +15,6 @@ public class SftpPathNotFoundException : SftpException { private const StatusCode Code = StatusCode.NoSuchFile; - /// - /// Gets the path that cannot be found. - /// - /// - /// The path that cannot be found, or if no path was - /// passed to the constructor for this instance. - /// - public string? Path { get; } - /// /// Initializes a new instance of the class. /// @@ -66,18 +57,8 @@ public SftpPathNotFoundException(string? message, Exception? innerException) /// The path that cannot be found. /// The exception that is the cause of the current exception. public SftpPathNotFoundException(string? message, string? path, Exception? innerException) - : base(Code, string.IsNullOrEmpty(message) ? GetDefaultMessage(path) : message, innerException) + : base(Code, message, path, innerException) { - Path = path; - } - - private static string GetDefaultMessage(string? path) - { - var message = GetDefaultMessage(Code); - - return path is not null - ? $"{message} Path: '{path}'." - : message; } #if NETFRAMEWORK diff --git a/src/Renci.SshNet/Common/SftpPermissionDeniedException.cs b/src/Renci.SshNet/Common/SftpPermissionDeniedException.cs index 1f8d9fe36..8e0a4af88 100644 --- a/src/Renci.SshNet/Common/SftpPermissionDeniedException.cs +++ b/src/Renci.SshNet/Common/SftpPermissionDeniedException.cs @@ -37,7 +37,18 @@ public SftpPermissionDeniedException(string? message) /// /// public SftpPermissionDeniedException(string? message, Exception? innerException) - : base(Code, message, innerException) + : this(message, path: null, innerException) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The error message that explains the reason for the exception. + /// The path that the failed operation was performed on. + /// The exception that is the cause of the current exception. + public SftpPermissionDeniedException(string? message, string? path, Exception? innerException) + : base(Code, message, path, innerException) { } diff --git a/src/Renci.SshNet/Sftp/SftpSession.cs b/src/Renci.SshNet/Sftp/SftpSession.cs index 041554505..9bdb895ba 100644 --- a/src/Renci.SshNet/Sftp/SftpSession.cs +++ b/src/Renci.SshNet/Sftp/SftpSession.cs @@ -1034,7 +1034,7 @@ public void RequestMkDir(string path) _encoding, response => { - exception = GetSftpException(response); + exception = GetSftpException(response, path); wait.SetIgnoringObjectDisposed(); }); @@ -1071,7 +1071,7 @@ public Task RequestMkDirAsync(string path, CancellationToken cancellationToken) } else { - _ = tcs.TrySetException(GetSftpException(response)); + _ = tcs.TrySetException(GetSftpException(response, path)); } })); @@ -1519,25 +1519,23 @@ public uint CalculateOptimalWriteLength(uint bufferSize, byte[] handle) internal static SftpException GetSftpException(SftpStatusResponse response, string path = null) { #pragma warning disable IDE0010 // Add missing cases + var message = response.ErrorMessage; + + if (!string.IsNullOrEmpty(message) && path is not null) + { + message = $"{message}{(message[^1] == '.' ? " " : ". ")}Path: '{path}'."; + } + switch (response.StatusCode) { case StatusCode.Ok: return null; case StatusCode.PermissionDenied: - return new SftpPermissionDeniedException(response.ErrorMessage); + return new SftpPermissionDeniedException(message, path, innerException: null); case StatusCode.NoSuchFile: - - var message = response.ErrorMessage; - - if (!string.IsNullOrEmpty(message) && path is not null) - { - message = $"{message}{(message[^1] == '.' ? " " : ". ")}Path: '{path}'."; - } - return new SftpPathNotFoundException(message, path); - default: - return new SftpException(response.StatusCode, response.ErrorMessage); + return new SftpException(response.StatusCode, message, path, innerException: null); } #pragma warning restore IDE0010 // Add missing cases } diff --git a/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.CreateDirectory.cs b/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.CreateDirectory.cs index 098ee824e..37f07e138 100644 --- a/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.CreateDirectory.cs +++ b/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.CreateDirectory.cs @@ -31,7 +31,8 @@ public void Test_Sftp_CreateDirectory_In_Forbidden_Directory() { sftp.Connect(); - Assert.ThrowsExactly(() => sftp.CreateDirectory("/sbin/test")); + var ex = Assert.ThrowsExactly(() => sftp.CreateDirectory("/sbin/test")); + Assert.AreEqual("/sbin/test", ex.Path); } } @@ -43,7 +44,8 @@ public void Test_Sftp_CreateDirectory_Invalid_Path() { sftp.Connect(); - Assert.ThrowsExactly(() => sftp.CreateDirectory("/abcdefg/abcefg")); + var ex = Assert.ThrowsExactly(() => sftp.CreateDirectory("/abcdefg/abcefg")); + Assert.AreEqual("/abcdefg/abcefg", ex.Path); } } @@ -59,6 +61,7 @@ public void Test_Sftp_CreateDirectory_Already_Exists() var ex = Assert.ThrowsExactly(() => sftp.CreateDirectory("test")); Assert.AreEqual(StatusCode.Failure, ex.StatusCode); + Assert.AreEqual($"{sftp.WorkingDirectory}/test", ex.Path); } } } diff --git a/test/Renci.SshNet.IntegrationTests/SftpClientTests.cs b/test/Renci.SshNet.IntegrationTests/SftpClientTests.cs index 6fffc22f6..5f63e3c5c 100644 --- a/test/Renci.SshNet.IntegrationTests/SftpClientTests.cs +++ b/test/Renci.SshNet.IntegrationTests/SftpClientTests.cs @@ -1,4 +1,4 @@ -using Renci.SshNet.Common; +using Renci.SshNet.Common; namespace Renci.SshNet.IntegrationTests { @@ -104,7 +104,8 @@ public async Task Create_directory_with_contents_and_list_it_async() [TestMethod] public void Test_Sftp_ListDirectory_Permission_Denied() { - Assert.ThrowsExactly(() => _sftpClient.ListDirectory("/root")); + var ex = Assert.ThrowsExactly(() => _sftpClient.ListDirectory("/root")); + Assert.AreEqual("/root", ex.Path); } [TestMethod] diff --git a/test/Renci.SshNet.Tests/Classes/Common/SftpExceptionTest.cs b/test/Renci.SshNet.Tests/Classes/Common/SftpExceptionTest.cs index 5ce69fbab..23719fc6a 100644 --- a/test/Renci.SshNet.Tests/Classes/Common/SftpExceptionTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Common/SftpExceptionTest.cs @@ -16,10 +16,12 @@ public void StatusCodes() Assert.AreEqual(StatusCode.BadMessage, new SftpException(StatusCode.BadMessage).StatusCode); Assert.AreEqual(StatusCode.OperationUnsupported, new SftpException(StatusCode.OperationUnsupported, null).StatusCode); Assert.AreEqual(StatusCode.Failure, new SftpException(StatusCode.Failure, null, null).StatusCode); + Assert.AreEqual(StatusCode.Failure, new SftpException(StatusCode.Failure, null, null, null).StatusCode); Assert.AreEqual(StatusCode.PermissionDenied, new SftpPermissionDeniedException().StatusCode); Assert.AreEqual(StatusCode.PermissionDenied, new SftpPermissionDeniedException(null).StatusCode); Assert.AreEqual(StatusCode.PermissionDenied, new SftpPermissionDeniedException(null, null).StatusCode); + Assert.AreEqual(StatusCode.PermissionDenied, new SftpPermissionDeniedException(null, null, null).StatusCode); Assert.AreEqual(StatusCode.NoSuchFile, new SftpPathNotFoundException().StatusCode); Assert.AreEqual(StatusCode.NoSuchFile, new SftpPathNotFoundException(null).StatusCode); @@ -34,12 +36,14 @@ public void Message() Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpException(StatusCode.Failure).Message)); Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpException(StatusCode.Failure, "").Message)); Assert.AreEqual("Custom message", new SftpException(StatusCode.Failure, "Custom message").Message); + Assert.AreEqual("Custom message", new SftpException(StatusCode.Failure, "Custom message", "path1", null).Message); Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpPermissionDeniedException().Message)); Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpPermissionDeniedException("").Message)); Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpPermissionDeniedException("", null).Message)); Assert.AreEqual("Custom message1", new SftpPermissionDeniedException("Custom message1").Message); Assert.AreEqual("Custom message2", new SftpPermissionDeniedException("Custom message2", null).Message); + Assert.AreEqual("Custom message3", new SftpPermissionDeniedException("Custom message3", "path1", null).Message); Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpPathNotFoundException().Message)); Assert.IsFalse(string.IsNullOrWhiteSpace(new SftpPathNotFoundException("").Message)); @@ -52,8 +56,18 @@ public void Message() } [TestMethod] - public void PathNotFoundException_Path() + public void Path() { + Assert.IsNull(new SftpException(StatusCode.Failure).Path); + Assert.IsNull(new SftpException(StatusCode.Failure, "message", innerException: null).Path); + Assert.AreEqual("path1", new SftpException(StatusCode.Failure, null, "path1", null).Path); + Assert.Contains("Path: 'path2'.", new SftpException(StatusCode.Failure, null, "path2", null).Message, StringComparison.Ordinal); + + Assert.IsNull(new SftpPermissionDeniedException().Path); + Assert.IsNull(new SftpPermissionDeniedException("message").Path); + Assert.AreEqual("path1", new SftpPermissionDeniedException("message", "path1", null).Path); + Assert.Contains("Path: 'path2'.", new SftpPermissionDeniedException("", "path2", null).Message, StringComparison.Ordinal); + Assert.IsNull(new SftpPathNotFoundException().Path); Assert.IsNull(new SftpPathNotFoundException("message").Path); Assert.AreEqual("path1", new SftpPathNotFoundException("message", "path1").Path);