Improve error logging

src/Podsync/Controllers/DownloadController.cs

@@ -9,7 +9,7 @@ using Podsync.Services.Storage;
 namespace Podsync.Controllers
 {
     [Route("download")]
-    [HandleException]
+    [ServiceFilter(typeof(HandleExceptionAttribute), IsReusable = true)]
     public class DownloadController : Controller
     {
         private readonly IResolverService _resolverService;

src/Podsync/Controllers/FeedController.cs

@@ -19,7 +19,7 @@ using Shared;
 namespace Podsync.Controllers
 {
     [Route("feed")]
-    [HandleException]
+    [ServiceFilter(typeof(HandleExceptionAttribute), IsReusable = true)]
     public class FeedController : Controller
     {
         private static readonly IDictionary<string, string> Extensions = new Dictionary<string, string>
@@ -105,7 +105,7 @@ namespace Podsync.Controllers
             }
             catch (KeyNotFoundException)
             {
-                return NotFound(feedId);
+                return NotFound($"ERROR: No feed with id {feedId}");
             }
 
             var selfHost = Request.GetBaseUrl();

src/Podsync/Helpers/HandleExceptionAttribute.cs

@@ -2,11 +2,20 @@
 using System.Net;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.Extensions.Logging;
+using Podsync.Services;
 
 namespace Podsync.Helpers
 {
     public class HandleExceptionAttribute : ExceptionFilterAttribute
     {
+        private readonly ILogger _logger;
+
+        public HandleExceptionAttribute(ILogger<HandleExceptionAttribute> logger)
+        {
+            _logger = logger;
+        }
+
         public override void OnException(ExceptionContext context)
         {
             var exception = context.Exception;
@@ -17,6 +26,8 @@ namespace Podsync.Helpers
             else
             {
                 context.Result = new StatusCodeResult((int)HttpStatusCode.InternalServerError);
+
+                _logger.LogCritical(Constants.Events.UnhandledError, context.Exception, "Unhandled exception");
             }
         }
     }

src/Podsync/Services/Builder/CompositeRssBuilder.cs

@@ -4,6 +4,7 @@ using System.Collections.ObjectModel;
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
 using Podsync.Services.Feed;
 using Podsync.Services.Links;
 using Podsync.Services.Storage;
@@ -15,13 +16,17 @@ namespace Podsync.Services.Builder
     public class CompositeRssBuilder : RssBuilderBase
     {
         private readonly IDictionary<Provider, IRssBuilder> _builders;
+        private readonly ILogger _logger;
 
-        public CompositeRssBuilder(IServiceProvider serviceProvider, IStorageService storageService) : base(storageService)
+        public CompositeRssBuilder(IServiceProvider serviceProvider, IStorageService storageService, ILogger<CompositeRssBuilder> logger) : base(storageService)
         {
+            _logger = logger;
+
             // Find all RSS builders (all implementations of IRssBuilder), create instances and make dictionary for fast search by Provider type
             var buildTypes = serviceProvider.FindAllImplementationsOf<IRssBuilder>(Assembly.GetEntryAssembly()).Where(x => x != typeof(CompositeRssBuilder));
             var builders = buildTypes.Select(builderType => (IRssBuilder)serviceProvider.CreateInstance(builderType)).ToDictionary(builder => builder.Provider);
-
+            
+            _logger.LogInformation($"Found {builders.Count} RSS builders");
             _builders = new ReadOnlyDictionary<Provider, IRssBuilder>(builders);
         }
 
@@ -32,13 +37,22 @@ namespace Podsync.Services.Builder
 
         public override Task<Rss> Query(FeedMetadata feed)
         {
-            IRssBuilder builder;
-            if (_builders.TryGetValue(feed.Provider, out builder))
+            try
             {
-                return builder.Query(feed);
-            }
+                IRssBuilder builder;
+                if (_builders.TryGetValue(feed.Provider, out builder))
+                {
+                    return builder.Query(feed);
+                }
 
-            throw new NotSupportedException("Not supported provider");
+                throw new NotSupportedException("Not supported provider");
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(Constants.Events.RssError, ex, "Failed to query RSS feed (id: {ID})", feed.Id);
+
+                throw;
+            }
         }
     }
 }

src/Podsync/Services/Constants.cs

@@ -18,5 +18,12 @@ namespace Podsync.Services
 
             public const string AmountDonated = "Patreon/" + nameof(AmountDonated);
         }
+
+        public static class Events
+        {
+            public const int RssError = 1;
+            public const int YtdlError = 2;
+            public const int UnhandledError = 3;
+        }
     }
 }

src/Podsync/Services/Resolver/YtdlWrapper.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.IO;
 using System.Threading.Tasks;
 using Medallion.Shell;
+using Microsoft.Extensions.Logging;
 
 namespace Podsync.Services.Resolver
 {
@@ -13,12 +14,18 @@ namespace Podsync.Services.Resolver
 
         private const string Ytdl = "youtube-dl";
 
-        public YtdlWrapper()
+        private readonly ILogger _logger;
+
+        public YtdlWrapper(ILogger<YtdlWrapper> logger)
         {
+            _logger = logger;
+
             try
             {
                 var cmd = Command.Run(Ytdl, "--version");
                 Version = cmd.Result.StandardOutput;
+
+                _logger.LogInformation("Uring youtube-dl {VERSION}", Version);
             }
             catch (Exception ex)
             {
@@ -82,7 +89,7 @@ namespace Podsync.Services.Resolver
             yield return "--no-call-home";
         }
 
-        private static async Task<Uri> ResolveInternal(Uri videoUrl, string format)
+        private async Task<Uri> ResolveInternal(Uri videoUrl, string format)
         {
             var cmd = Command.Run(Ytdl, GetArguments(videoUrl, format), opts => opts.ThrowOnError().Timeout(ProcessWaitTimeout));
 
@@ -95,6 +102,8 @@ namespace Podsync.Services.Resolver
                 var errout = await cmd.StandardError.ReadToEndAsync();
                 var msg = !string.IsNullOrWhiteSpace(errout) ? errout : ex.Message;
 
+                _logger.LogError(Constants.Events.YtdlError, ex, "Failed to resolve {URL} in format {FORMAT}", videoUrl, format);
+
                 if (string.Equals(errout, "ERROR: requested format not available"))
                 {
                     throw new NotSupportedException("Requested format not available", ex);

src/Podsync/Startup.cs

@@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Http.Authentication;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Podsync.Helpers;
 using Podsync.Services;
 using Podsync.Services.Builder;
 using Podsync.Services.Links;
@@ -59,6 +60,7 @@ namespace Podsync
             services.AddAuthentication(config => config.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme);
 
             // Add framework services
+            services.AddScoped<HandleExceptionAttribute>();
             services.AddMvc();
         }
 

test/Podsync.Tests/Services/Resolver/YtdlWrapperTests.cs

@@ -1,13 +1,21 @@
 using System;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
+using Moq;
 using Podsync.Services.Resolver;
 using Xunit;
 
 namespace Podsync.Tests.Services.Resolver
 {
-    public class YtdlWrapperTests
+    public class YtdlWrapperTests : TestBase
     {
-        private readonly IResolverService _resolver = new YtdlWrapper();
+        private readonly IMock<ILogger<YtdlWrapper>> _logger = new Mock<ILogger<YtdlWrapper>>();
+        private readonly IResolverService _resolver;
+
+        public YtdlWrapperTests()
+        {
+            _resolver = new YtdlWrapper(_logger.Object);
+        }
 
         [Theory]
         [InlineData("https://www.youtube.com/watch?v=BaW_jenozKc")]

test/Podsync.Tests/TestBase.cs

@@ -24,6 +24,6 @@ namespace Podsync.Tests
 
         protected IOptions<PodsyncConfiguration> Options { get; }
 
-        protected PodsyncConfiguration Configuration => Options.Value;        
+        protected PodsyncConfiguration Configuration => Options.Value;
     }
 }