Fix health check errors (#717)

* Fixed: RemotePathMappingCheck deals with case where path invalid

The `Ensure` built into `FileExists` and `FolderExists` was throwing
an exception previously.

* Fixed: RemotePathMappingCheck doesn't fail if client unavailable

src/NzbDrone.Core.Test/HealthCheck/Checks/RemotePathMappingCheckFixture.cs

@@ -4,7 +4,9 @@ using System.IO;
 using Moq;
 using NUnit.Framework;
 using NzbDrone.Common.Disk;
+using NzbDrone.Common.EnsureThat;
 using NzbDrone.Core.Download;
+using NzbDrone.Core.Download.Clients;
 using NzbDrone.Core.HealthCheck.Checks;
 using NzbDrone.Core.MediaFiles;
 using NzbDrone.Core.MediaFiles.Events;
@@ -22,6 +24,7 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
 
         private DownloadClientInfo clientStatus;
         private DownloadClientItem downloadItem;
+        private Mock<IDownloadClient> downloadClient;
 
         [SetUp]
         public void Setup()
@@ -37,7 +40,7 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
                         OutputRootFolders = new List<OsPath> { new OsPath(downloadRootPath) }
             };
 
-            var downloadClient = Mocker.GetMock<IDownloadClient>();
+            downloadClient = Mocker.GetMock<IDownloadClient>();
             downloadClient.Setup(s => s.Definition)
                 .Returns(new DownloadClientDefinition { Name = "Test" });
 
@@ -53,11 +56,17 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
 
             Mocker.GetMock<IDiskProvider>()
                 .Setup(x => x.FolderExists(It.IsAny<string>()))
-                .Returns(false);
+                .Returns((string path) => {
+                        Ensure.That(path, () => path).IsValidPath();
+                        return false;
+                    });
 
             Mocker.GetMock<IDiskProvider>()
                 .Setup(x => x.FileExists(It.IsAny<string>()))
-                .Returns(false);
+                .Returns((string path) => {
+                        Ensure.That(path, () => path).IsValidPath();
+                        return false;
+                    });
         }
 
         private void GivenFolderExists(string folder)
@@ -94,6 +103,24 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
             Subject.Check().ShouldBeError(wikiFragment: "permissions-error");
         }
 
+        [Test]
+        public void should_return_mapping_error_if_remote_client_root_path_invalid()
+        {
+            clientStatus.IsLocalhost = false;
+            clientStatus.OutputRootFolders = new List<OsPath> { new OsPath("An invalid path") };
+
+            Subject.Check().ShouldBeError(wikiFragment: "bad-remote-path-mapping");
+        }
+
+        [Test]
+        public void should_return_download_client_error_if_local_client_root_path_invalid()
+        {
+            clientStatus.IsLocalhost = true;
+            clientStatus.OutputRootFolders = new List<OsPath> { new OsPath("An invalid path") };
+
+            Subject.Check().ShouldBeError(wikiFragment: "bad-download-client-settings");
+        }
+
         [Test]
         public void should_return_path_mapping_error_if_remote_client_download_root_missing()
         {
@@ -102,6 +129,15 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
             Subject.Check().ShouldBeError(wikiFragment: "bad-remote-path-mapping");
         }
 
+        [Test]
+        public void should_return_ok_if_client_unavailable()
+        {
+            downloadClient.Setup(s => s.GetStatus())
+                .Throws(new DownloadClientUnavailableException("error"));
+            
+            Subject.Check().ShouldBeOk();
+        }
+
         [Test]
         [Explicit("Only works if running inside a docker container")]
         public void should_return_docker_path_mapping_error_if_on_docker_and_root_missing()
@@ -158,6 +194,26 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
             Subject.Check(importEvent).ShouldBeError(wikiFragment: "bad-remote-path-mapping");
         }
 
+        [Test]
+        public void should_return_mapping_error_on_track_import_failed_event_for_remote_client_if_path_invalid()
+        {
+            clientStatus.IsLocalhost = false;
+            downloadItem.OutputPath = new OsPath("an invalid path");
+            var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem);
+
+            Subject.Check(importEvent).ShouldBeError(wikiFragment: "bad-remote-path-mapping");
+        }
+
+        [Test]
+        public void should_return_download_client_error_on_track_import_failed_event_for_remote_client_if_path_invalid()
+        {
+            clientStatus.IsLocalhost = true;
+            downloadItem.OutputPath = new OsPath("an invalid path");
+            var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem);
+
+            Subject.Check(importEvent).ShouldBeError(wikiFragment: "bad-download-client-settings");
+        }
+
         [Test]
         [Explicit("Only works if running inside a docker container")]
         public void should_return_docker_mapping_error_on_track_import_failed_event_inside_docker_if_folder_does_not_exist()
@@ -167,5 +223,16 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
 
             Subject.Check(importEvent).ShouldBeError(wikiFragment: "docker-bad-remote-path-mapping");
         }
+        
+        [Test]
+        public void should_return_ok_on_import_failed_event_if_client_unavailable()
+        {
+            downloadClient.Setup(s => s.GetStatus())
+                .Throws(new DownloadClientUnavailableException("error"));
+            
+            var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem);
+            
+            Subject.Check(importEvent).ShouldBeOk();
+        }
     }
 }

src/NzbDrone.Core/HealthCheck/Checks/RemotePathMappingCheck.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Linq;
 using NLog;
 using NzbDrone.Common.Disk;
@@ -6,6 +7,7 @@ using NzbDrone.Common.Extensions;
 using NzbDrone.Common.Messaging;
 using NzbDrone.Core.Datastore.Events;
 using NzbDrone.Core.Download;
+using NzbDrone.Core.Download.Clients;
 using NzbDrone.Core.MediaFiles.Events;
 using NzbDrone.Core.RemotePathMappings;
 using NzbDrone.Core.ThingiProvider.Events;
@@ -23,14 +25,17 @@ namespace NzbDrone.Core.HealthCheck.Checks
         private readonly IDiskProvider _diskProvider;
         private readonly IProvideDownloadClient _downloadClientProvider;
         private readonly Logger _logger;
+        private readonly IOsInfo _osInfo;
 
         public RemotePathMappingCheck(IDiskProvider diskProvider,
                                       IProvideDownloadClient downloadClientProvider,
+                                      IOsInfo osInfo,
                                       Logger logger)
         {
             _diskProvider = diskProvider;
             _downloadClientProvider = downloadClientProvider;
             _logger = logger;
+            _osInfo = osInfo;
         }
 
         public override HealthCheck Check()
@@ -39,18 +44,37 @@ namespace NzbDrone.Core.HealthCheck.Checks
 
             foreach (var client in clients)
             {
-                var folders = client.GetStatus().OutputRootFolders;
+                try
+                {
+                    var status = client.GetStatus();
+                    var folders = status.OutputRootFolders;
                     if (folders != null)
                     {
                         foreach (var folder in folders)
                         {
+                            if (!folder.IsValid)
+                            {
+                                if (!status.IsLocalhost)
+                                {
+                                    return new HealthCheck(GetType(), HealthCheckResult.Error, $"Remote download client {client.Definition.Name} places downloads in {folder.FullPath} but this is not a valid {_osInfo.Name} path.  Review your remote path mappings and download client settings.", "#bad-remote-path-mapping");
+                                }
+                                else if (OsInfo.IsDocker)
+                                {
+                                    return new HealthCheck(GetType(), HealthCheckResult.Error, $"You are using docker; download client {client.Definition.Name} places downloads in {folder.FullPath} but this is not a valid {_osInfo.Name} path.  Review your remote path mappings and download client settings.", "#docker-bad-remote-path-mapping");
+                                }
+                                else
+                                {
+                                    return new HealthCheck(GetType(), HealthCheckResult.Error, $"Local download client {client.Definition.Name} places downloads in {folder.FullPath} but this is not a valid {_osInfo.Name} path.  Review your download client settings.", "#bad-download-client-settings");
+                                }
+                            }
+
                             if (!_diskProvider.FolderExists(folder.FullPath))
                             {
                                 if (OsInfo.IsDocker)
                                 {
                                     return new HealthCheck(GetType(), HealthCheckResult.Error, $"You are using docker; download client {client.Definition.Name} places downloads in {folder.FullPath} but this directory does not appear to exist inside the container.  Review your remote path mappings and container volume settings.", "#docker-bad-remote-path-mapping");
                                 }
-                            else if (!client.GetStatus().IsLocalhost)
+                                else if (!status.IsLocalhost)
                                 {
                                     return new HealthCheck(GetType(), HealthCheckResult.Error, $"Remote download client {client.Definition.Name} places downloads in {folder.FullPath} but this directory does not appear to exist.  Likely missing or incorrect remote path mapping.", "#bad-remote-path-mapping");
                                 }
@@ -62,6 +86,15 @@ namespace NzbDrone.Core.HealthCheck.Checks
                         }
                     }
                 }
+                catch (DownloadClientUnavailableException ex)
+                {
+                    _logger.Debug(ex, "Unable to communicate with {0}", client.Definition.Name);
+                }
+                catch (Exception ex)
+                {
+                    _logger.Error(ex, "Unknown error occured in RemotePathMapping HealthCheck");
+                }
+            }
             return new HealthCheck(GetType());
         }
 
@@ -90,6 +123,9 @@ namespace NzbDrone.Core.HealthCheck.Checks
                 // If the previous case did not match then the failure occured in DownloadedTracksImportService,
                 // while trying to locate the files reported by the download client
                 var client = _downloadClientProvider.GetDownloadClients().FirstOrDefault(x => x.Definition.Name == failureMessage.DownloadClient);
+                try
+                {
+                    var status = client.GetStatus();
                     var dlpath = client?.GetItems().FirstOrDefault(x => x.DownloadId == failureMessage.DownloadId)?.OutputPath.FullPath;
 
                     // If dlpath is null then there's not much useful we can report.  Give a generic message so
@@ -99,6 +135,22 @@ namespace NzbDrone.Core.HealthCheck.Checks
                         return new HealthCheck(GetType(), HealthCheckResult.Error, $"Lidarr failed to import a track.  Check your logs for details.");
                     }
 
+                    if (!dlpath.IsPathValid())
+                    {
+                        if (!status.IsLocalhost)
+                        {
+                            return new HealthCheck(GetType(), HealthCheckResult.Error, $"Remote download client {client.Definition.Name} reported files in {dlpath} but this is not a valid {_osInfo.Name} path.  Review your remote path mappings and download client settings.", "#bad-remote-path-mapping");
+                        }
+                        else if (OsInfo.IsDocker)
+                        {
+                            return new HealthCheck(GetType(), HealthCheckResult.Error, $"You are using docker; download client {client.Definition.Name} reported files in {dlpath} but this is not a valid {_osInfo.Name} path.  Review your remote path mappings and download client settings.", "#docker-bad-remote-path-mapping");
+                        }
+                        else
+                        {
+                            return new HealthCheck(GetType(), HealthCheckResult.Error, $"Local download client {client.Definition.Name} reported files in {dlpath} but this is not a valid {_osInfo.Name} path.  Review your download client settings.", "#bad-download-client-settings");
+                        }
+                    }
+
                     if (_diskProvider.FolderExists(dlpath))
                     {
                         return new HealthCheck(GetType(), HealthCheckResult.Error, $"Lidarr can see but not access download directory {dlpath}.  Likely permissions error.", "#permissions-error");
@@ -109,7 +161,7 @@ namespace NzbDrone.Core.HealthCheck.Checks
                     {
                         return new HealthCheck(GetType(), HealthCheckResult.Error, $"You are using docker; download client {client.Definition.Name} reported files in {dlpath} but this directory does not appear to exist inside the container.  Review your remote path mappings and container volume settings.", "#docker-bad-remote-path-mapping");
                     }
-                else if (!client.GetStatus().IsLocalhost)
+                    else if (!status.IsLocalhost)
                     {
                         return new HealthCheck(GetType(), HealthCheckResult.Error, $"Remote download client {client.Definition.Name} reported files in {dlpath} but this directory does not appear to exist.  Likely missing remote path mapping.", "#bad-remote-path-mapping");
                     }
@@ -119,6 +171,17 @@ namespace NzbDrone.Core.HealthCheck.Checks
                         return new HealthCheck(GetType(), HealthCheckResult.Error, $"Download client {client.Definition.Name} reported files in {dlpath} but Lidarr cannot see this directory.  You may need to adjust the folder's permissions.", "#permissions-error");
                     }
                 }
+                catch (DownloadClientUnavailableException ex)
+                {
+                    _logger.Debug(ex, "Unable to communicate with {0}", client.Definition.Name);
+                }
+                catch (Exception ex)
+                {
+                    _logger.Error(ex, "Unknown error occured in RemotePathMapping HealthCheck");
+                }
+
+                return new HealthCheck(GetType());
+            }
             else
             {
                 return Check();