Fix Transmission connections failing when the 409 session challenge is retried (#714)

code/backend/Cleanuparr.Infrastructure.Tests/Http/DynamicHttpClientConfigurationTests.cs

@@ -0,0 +1,43 @@
+using System.Net;
+using Cleanuparr.Infrastructure.Http.DynamicHttpClientSystem;
+using Shouldly;
+using Xunit;
+
+namespace Cleanuparr.Infrastructure.Tests.Http;
+
+public sealed class DynamicHttpClientConfigurationTests
+{
+    [Fact]
+    public void IsRetryable_ShouldReturnFalse_WhenResponseIsConflict()
+    {
+        using HttpResponseMessage response = new(HttpStatusCode.Conflict);
+
+        DynamicHttpClientConfiguration.IsRetryable(response, excludeUnauthorized: true).ShouldBeFalse();
+        DynamicHttpClientConfiguration.IsRetryable(response, excludeUnauthorized: false).ShouldBeFalse();
+    }
+
+    [Fact]
+    public void IsRetryable_ShouldReturnFalse_WhenResponseIsSuccessful()
+    {
+        using HttpResponseMessage response = new(HttpStatusCode.OK);
+
+        DynamicHttpClientConfiguration.IsRetryable(response, excludeUnauthorized: true).ShouldBeFalse();
+    }
+
+    [Fact]
+    public void IsRetryable_ShouldRespectExcludeUnauthorized()
+    {
+        using HttpResponseMessage response = new(HttpStatusCode.Unauthorized);
+
+        DynamicHttpClientConfiguration.IsRetryable(response, excludeUnauthorized: true).ShouldBeFalse();
+        DynamicHttpClientConfiguration.IsRetryable(response, excludeUnauthorized: false).ShouldBeTrue();
+    }
+
+    [Fact]
+    public void IsRetryable_ShouldReturnTrue_WhenResponseIsServerError()
+    {
+        using HttpResponseMessage response = new(HttpStatusCode.InternalServerError);
+
+        DynamicHttpClientConfiguration.IsRetryable(response, excludeUnauthorized: true).ShouldBeTrue();
+    }
+}

code/backend/Cleanuparr.Infrastructure/Http/DynamicHttpClientSystem/DynamicHttpClientConfiguration.cs

@@ -103,15 +103,7 @@ public class DynamicHttpClientConfiguration : IConfigureNamedOptions<HttpClientF
         var retryPolicy = HttpPolicyExtensions
             .HandleTransientHttpError();
 
-        if (retryConfig.ExcludeUnauthorized)
-        {
-            retryPolicy = retryPolicy.OrResult(response => 
-                !response.IsSuccessStatusCode && response.StatusCode != HttpStatusCode.Unauthorized);
-        }
-        else
-        {
-            retryPolicy = retryPolicy.OrResult(response => !response.IsSuccessStatusCode);
-        }
+        retryPolicy = retryPolicy.OrResult(response => IsRetryable(response, retryConfig.ExcludeUnauthorized));
 
         var policy = retryPolicy.WaitAndRetryAsync(
             retryConfig.MaxRetries, 
@@ -121,6 +113,21 @@ public class DynamicHttpClientConfiguration : IConfigureNamedOptions<HttpClientF
         builder.AdditionalHandlers.Add(new PolicyHttpMessageHandler(policy));
     }
 
+    internal static bool IsRetryable(HttpResponseMessage response, bool excludeUnauthorized)
+    {
+        if (response.IsSuccessStatusCode)
+        {
+            return false;
+        }
+
+        if (response.StatusCode is HttpStatusCode.Conflict)
+        {
+            return false;
+        }
+
+        return !excludeUnauthorized || response.StatusCode != HttpStatusCode.Unauthorized;
+    }
+
     public void Configure(HttpClientFactoryOptions options)
     {
         // This is called for unnamed clients - we don't need to do anything here

e2e/tests/download-cleaner/transmission-session-handshake.spec.ts

@@ -0,0 +1,58 @@
+import { test, expect } from '@playwright/test';
+import {
+  loginAndGetToken,
+  testDownloadClient,
+  getGeneralConfig,
+  updateGeneralConfig,
+} from '../helpers/app-api';
+import { TransmissionDriver } from '../helpers/torrent-clients/transmission';
+
+const transmission = new TransmissionDriver();
+
+const HTTP_TIMEOUT_SECONDS = 10;
+const HTTP_MAX_RETRIES = 3;
+
+function payload(): Record<string, unknown> {
+  return {
+    enabled: true,
+    name: 'Transmission handshake e2e',
+    typeName: transmission.typeName,
+    type: 'Torrent',
+    host: transmission.cleanuparrHost,
+    username: transmission.username,
+    password: transmission.password,
+  };
+}
+
+test.describe.serial('Transmission session handshake', () => {
+  let token: string;
+  let originalGeneralConfig: Record<string, unknown>;
+
+  test.beforeAll(async () => {
+    token = await loginAndGetToken();
+    await transmission.ready();
+
+    originalGeneralConfig = await getGeneralConfig(token);
+    await updateGeneralConfig(token, {
+      ...originalGeneralConfig,
+      httpTimeout: HTTP_TIMEOUT_SECONDS,
+      httpMaxRetries: HTTP_MAX_RETRIES,
+    });
+  });
+
+  test.afterAll(async () => {
+    await updateGeneralConfig(token, originalGeneralConfig).catch(() => {});
+  });
+
+  test('connects without spending the request timeout on 409 retries', async () => {
+    const startedAt = Date.now();
+    const res = await testDownloadClient(token, payload());
+    const elapsedMs = Date.now() - startedAt;
+
+    expect(res.ok, `test connection failed: ${res.status} ${await res.text()}`).toBe(true);
+    expect(
+      elapsedMs,
+      `handshake took ${elapsedMs}ms: the 409 session challenge is being retried with backoff`,
+    ).toBeLessThan(5_000);
+  });
+});