From cfcd5f7f7b1a4ab2e3a5fe7a834fea27fe6b21cf Mon Sep 17 00:00:00 2001 From: m4bard <304653687+m4bard@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:14:28 -0500 Subject: [PATCH] fix(images): read the metadata envelope by reflection, not dynamic GetMetadataAsync is declared Task and returns an anonymous type. The compiler emits anonymous types as internal, so that envelope is internal to Listenarr.Application while the call site is in Listenarr.Api. The runtime binder resolves members against what is visible from the call site's assembly, cannot see `metadata`, and throws RuntimeBinderException reported against `object`. RuntimeBinderException is not in IsRecoverableImageLookupException, so it escapes the filtered catch in this method and reaches the controller catch-all, which returns 500. The throw is deterministic on that path, not intermittent; what makes it rare is the guard above it, since most requests resolve an image URL earlier and never enter the fallback. Reflection is not subject to the binder's accessibility rules. It is also already what this same block does ten lines further down to read ImageUrl and Isbn off the inner object, and what LibraryMetadataRescanWorkflow does to unwrap this very envelope. So this makes one outlier agree with its neighbours rather than introducing an approach. The existing fallback test returns an AudibleBookResponse directly and says in its own comment that this is to avoid anonymous envelope issues, which means the one shape production actually returns was the one shape never covered. The new test uses an anonymous envelope; because it is emitted internal to the test assembly and the workflow lives in Listenarr.Api, it reproduces the same accessibility relationship as production. Comment kept short deliberately: BackendArchitectureTests caps a production source file at 500 lines and this file is at 487 on canary. The full explanation is in the issue rather than inline. --- .../Images/ImageCandidateLookupWorkflow.cs | 13 ++- ...ontroller_MetadataDownloadFallbackTests.cs | 102 ++++++++++++++++++ 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/listenarr.api/Features/Images/ImageCandidateLookupWorkflow.cs b/listenarr.api/Features/Images/ImageCandidateLookupWorkflow.cs index 0bc3e1875..ad4b78b94 100644 --- a/listenarr.api/Features/Images/ImageCandidateLookupWorkflow.cs +++ b/listenarr.api/Features/Images/ImageCandidateLookupWorkflow.cs @@ -8,6 +8,7 @@ * (at your option) any later version. */ +using System.Reflection; namespace Listenarr.Api.Features.Images { @@ -337,9 +338,15 @@ void AddCandidateUrl(string? url, string source) } else { - // Try dynamic access - dynamic env = metadataEnvelope; - object? mdObj = env.metadata; + // Not `dynamic`: the envelope is an anonymous type, emitted + // internal to Listenarr.Application, so the binder cannot + // see `metadata` from this assembly and throws. Reflection + // is how the inner object is read below, and how + // LibraryMetadataRescanWorkflow reads this same envelope. + object? mdObj = metadataEnvelope.GetType().GetProperty( + "metadata", + BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase) + ?.GetValue(metadataEnvelope); // If it's already the Audible type, use it if (mdObj is AudibleBookResponse mdMeta) diff --git a/tests/Features/Api/Features/Images/ImagesController_MetadataDownloadFallbackTests.cs b/tests/Features/Api/Features/Images/ImagesController_MetadataDownloadFallbackTests.cs index eb708dcae..5d1a861f7 100644 --- a/tests/Features/Api/Features/Images/ImagesController_MetadataDownloadFallbackTests.cs +++ b/tests/Features/Api/Features/Images/ImagesController_MetadataDownloadFallbackTests.cs @@ -96,5 +96,107 @@ public async Task GetImage_FallsBackToGetMetadataAsync_WhenAudibleNull_AndDownlo // Best-effort test cleanup; ignore cleanup failures. } } + + // The test above returns the AudibleBookResponse directly, and its own comment says that is + // to avoid "anonymous envelope issues". But an anonymous envelope is what the production + // service actually returns, so dodging it leaves the fallback covered only in the one shape + // that never exercises the unwrap. This covers the other one. + // + // The anonymous type here is emitted internal to this test assembly, and the workflow that + // unwraps it lives in Listenarr.Api, so the accessibility relationship is the same one that + // exists in production between Listenarr.Application and Listenarr.Api. + [Fact] + public async Task GetImage_FallbackEnvelopeIsAnonymousType_StillFindsTheImageUrl() + { + var identifier = "BTESTASIN"; + var relativePath = $"config/cache/images/temp/{identifier}.jpg"; + var imageUrl = "https://audnexus.covers/anonymous-envelope.jpg"; + + var mockImageCache = new Mock(); + mockImageCache + .Setup(m => m.DownloadAndCacheImageAsync(imageUrl, identifier)) + .ReturnsAsync(relativePath); + mockImageCache + .SetupSequence(m => m.GetCachedImagePathAsync(identifier)) + .ReturnsAsync((string?)null) + .ReturnsAsync(relativePath); + + using var httpClientForAudible = new System.Net.Http.HttpClient(); + var audibleMock = new Mock( + httpClientForAudible, + Mock.Of>()); + audibleMock + .Setup(a => a.GetBookMetadataAsync( + identifier, + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync((AudibleBookResponse?)null); + + var mockMetadata = new Mock(); + mockMetadata + .Setup(m => m.GetAudibleMetadataAsync( + identifier, + It.IsAny(), + It.IsAny())) + .ReturnsAsync((AudibleBookResponse?)null); + // The same envelope shape AudiobookMetadataService returns on its success path. + mockMetadata + .Setup(m => m.GetMetadataAsync( + identifier, + It.IsAny(), + It.IsAny())) + .ReturnsAsync((object)new + { + metadata = new AudibleBookResponse { ImageUrl = imageUrl }, + source = "Audnexus", + sourceUrl = "https://api.audnex.us" + }); + + var tempRoot = Path.Join( + Path.GetTempPath(), + "listenarr_test_contentroot_anonymous_envelope"); + Directory.CreateDirectory(Path.Join(tempRoot, "config", "cache", "images", "temp")); + var fullPath = Path.Join(tempRoot, relativePath); + File.WriteAllText(fullPath, "fake image data"); + + var mockPathService = new Mock(); + mockPathService.SetupGet(p => p.ContentRootPath).Returns(tempRoot); + + var controller = new ImagesController( + mockImageCache.Object, + mockMetadata.Object, + audibleMock.Object, + Mock.Of(), + Mock.Of(), + Mock.Of>(), + mockPathService.Object, + new LocalFileSystem()); + controller.ControllerContext = new ControllerContext + { + HttpContext = new Microsoft.AspNetCore.Http.DefaultHttpContext() + }; + + var result = await controller.GetImage(identifier); + + // The URL has to be recovered from inside the envelope for the downloader to be called + // at all. Reading it with `dynamic` throws RuntimeBinderException, which is not in the + // recoverable list, so it escapes the filtered catch and the controller returns 500 + // having never reached the download. + mockImageCache.Verify( + m => m.DownloadAndCacheImageAsync(imageUrl, identifier), + Times.Once); + Assert.IsNotType(result); + + try + { + File.Delete(fullPath); + Directory.Delete(Path.Join(tempRoot, "config", "cache", "images", "temp"), true); + } + catch (Exception ex) when (ex is not OutOfMemoryException && ex is not StackOverflowException) + { + // Best-effort test cleanup; ignore cleanup failures. + } + } } }