diff --git a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentItemFetchWorkflow.cs b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentItemFetchWorkflow.cs index 45a624f1d..4dd0aa951 100644 --- a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentItemFetchWorkflow.cs +++ b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentItemFetchWorkflow.cs @@ -87,14 +87,31 @@ public async Task> GetItemsAsync(DownloadClientConfigur foreach (var torrent in torrents) { - items.Add(QbittorrentResponseMapper.MapDownloadClientItem( - torrent, - client, - removeCompletedDownloads, - globalMaxRatioEnabled, - globalMaxRatio, - globalMaxSeedingTimeEnabled, - globalMaxSeedingTime)); + // Same per-item isolation as the queue fetch. This list is what completion and + // import decisions are made from, so a torrent lost here is not just a missing + // row in a view: everything after it stops being considered for import at all. + try + { + items.Add(QbittorrentResponseMapper.MapDownloadClientItem( + torrent, + client, + removeCompletedDownloads, + globalMaxRatioEnabled, + globalMaxRatio, + globalMaxSeedingTimeEnabled, + globalMaxSeedingTime)); + } + catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) + { + var hash = torrent.TryGetValue("hash", out var hashEl) && hashEl.ValueKind == JsonValueKind.String + ? hashEl.GetString() ?? string.Empty + : string.Empty; + logger.LogWarning( + ex, + "Skipping unreadable qBittorrent torrent {TorrentHash} for client {ClientId}; the rest of the item list is unaffected", + LogRedaction.SanitizeText(hash), + LogRedaction.SanitizeText(client.Id)); + } } } catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) diff --git a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentQueueFetchWorkflow.cs b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentQueueFetchWorkflow.cs index bd3c62a0a..9296d7107 100644 --- a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentQueueFetchWorkflow.cs +++ b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentQueueFetchWorkflow.cs @@ -98,17 +98,35 @@ public async Task> GetQueueAsync(DownloadClientConfiguration cli foreach (var torrent in torrents) { - var hash = torrent.TryGetValue("hash", out var hashEl) ? hashEl.GetString() ?? string.Empty : string.Empty; + var hash = torrent.TryGetValue("hash", out var hashEl) && hashEl.ValueKind == JsonValueKind.String + ? hashEl.GetString() ?? string.Empty + : string.Empty; - List> files = []; - using var filesResp = await httpClient.GetAsync($"{baseUrl}/api/v2/torrents/files?hash={hash}", ct); - if (filesResp.IsSuccessStatusCode) + // One torrent that cannot be read must not take the rest of the response with + // it. Without this, an exception raised while mapping torrent N escapes the + // loop and is caught only by the handler below, so torrents N..end are dropped + // while the poll still reports itself as a healthy live snapshot: the queue + // simply appears shorter, with nothing to say a row was lost. + try { - var filesJson = await filesResp.Content.ReadAsStringAsync(ct); - files = JsonSerializer.Deserialize>>(filesJson) ?? []; - } + List> files = []; + using var filesResp = await httpClient.GetAsync($"{baseUrl}/api/v2/torrents/files?hash={hash}", ct); + if (filesResp.IsSuccessStatusCode) + { + var filesJson = await filesResp.Content.ReadAsStringAsync(ct); + files = JsonSerializer.Deserialize>>(filesJson) ?? []; + } - items.Add(QbittorrentResponseMapper.MapQueueItem(torrent, client, files)); + items.Add(QbittorrentResponseMapper.MapQueueItem(torrent, client, files)); + } + catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) + { + logger.LogWarning( + ex, + "Skipping unreadable qBittorrent torrent {TorrentHash} for client {ClientId}; the rest of the queue is unaffected", + LogRedaction.SanitizeText(hash), + LogRedaction.SanitizeText(client.Id)); + } } } catch (DownloadClientAdapterPollingException) diff --git a/tests/Features/Infrastructure/DownloadClients/Qbittorrent/QbittorrentAdapterTests.cs b/tests/Features/Infrastructure/DownloadClients/Qbittorrent/QbittorrentAdapterTests.cs index 038abe508..5a986dc87 100644 --- a/tests/Features/Infrastructure/DownloadClients/Qbittorrent/QbittorrentAdapterTests.cs +++ b/tests/Features/Infrastructure/DownloadClients/Qbittorrent/QbittorrentAdapterTests.cs @@ -495,6 +495,57 @@ public async Task GetQueueAsync_WithoutIds_ReturnsEmpty_OnQueueRequestFailure() Assert.Empty(items); } + + // A queue response whose middle torrent carries `downloaded` in the given JSON token form. + // The torrents either side of it are well formed, so anything missing from the result is + // attributable to that one field. + private static string QueueWithMalformedMiddleTorrent(string malformedDownloaded) => $$""" + [ + { + "hash": "aaaa1111", "name": "First", "progress": 0.5, "size": 1000, + "downloaded": 500, "state": "downloading", "save_path": "/downloads/a" + }, + { + "hash": "bbbb2222", "name": "Second", "progress": 0.5, "size": 1000, + "downloaded": {{malformedDownloaded}}, "state": "downloading", "save_path": "/downloads/b" + }, + { + "hash": "cccc3333", "name": "Third", "progress": 0.5, "size": 1000, + "downloaded": 700, "state": "downloading", "save_path": "/downloads/c" + } + ] + """; + + // qBittorrent documents `downloaded` as an integer, so the typed accessor reading it is + // right about the normal case. It was not resilient about the abnormal one: a value in + // another token form threw out of the mapper, out of the loop walking the response, and + // took every torrent after it along with it, while the poll still reported itself as a + // healthy live snapshot. + // + // "600.5" is a JSON number that is not an integer (FormatException from GetInt64) and + // "\"600\"" is a quoted one (InvalidOperationException). The quoted form is the shape + // already reported against the NZBGet adapter in #618 and #619. + [Theory] + [InlineData("600.5")] + [InlineData("\"600\"")] + [InlineData("6e2")] + public async Task GetQueueAsync_WhenOneTorrentIsUnreadable_DropsOnlyThatTorrent(string malformedDownloaded) + { + var apiMock = _provider.GetRequiredService(); + apiMock.InfoResponseOverride = QueueWithMalformedMiddleTorrent(malformedDownloaded); + var gateway = (DownloadClientGateway)_provider.GetRequiredService(); + var adapter = (QbittorrentAdapter)gateway.ResolveAdapter(_client); + + var items = await adapter.GetQueueAsync(_client); + + // The torrent AFTER the unreadable one is the whole point. Asserting only that the + // list is non-empty would pass on the truncating behaviour, because the first torrent + // is mapped before anything throws. + Assert.Contains(items, item => item.Id == "aaaa1111"); + Assert.Contains(items, item => item.Id == "cccc3333"); + Assert.DoesNotContain(items, item => item.Id == "bbbb2222"); + Assert.Equal(2, items.Count); + } [Fact] public async Task MarkItemAsImportedAsync_SetsConfiguredPostImportCategory() {