fix: Handle run-test-payment failures safely

BTCPayServer.Plugins.Payjoin.Tests/UIPayJoinControllerTests.cs

@@ -81,11 +81,17 @@ public void RunTestPaymentUsesCheatModeRoute()
     }
 
     [Fact]
-    public async Task RunTestPaymentThrowsWhenRequestIsNull()
+    public async Task RunTestPaymentReturnsBadRequestWhenRequestIsNull()
     {
         using var controller = CreateController();
 
-        await Assert.ThrowsAsync<ArgumentNullException>(() => controller.RunTestPayment(null!, TestContext.Current.CancellationToken));
+        var result = await controller.RunTestPayment(null!, TestContext.Current.CancellationToken);
+
+        var badRequest = Assert.IsType<BadRequestObjectResult>(result.Result);
+        var response = Assert.IsType<RunTestPaymentResponse>(badRequest.Value);
+        Assert.False(response.Succeeded);
+        Assert.Contains("invoiceId", response.Message, StringComparison.Ordinal);
+        Assert.Null(response.TransactionId);
     }
 
     [Fact]

BTCPayServer.Plugins.Payjoin/Controllers/UIPayJoinController.cs

@@ -12,6 +12,7 @@
 using NBitcoin;
 using Payjoin;
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
 using PayjoinUri = Payjoin.Uri;
@@ -26,6 +27,10 @@ public class UIPayJoinController : Controller
         LoggerMessage.Define<string, string>(LogLevel.Information, new EventId(1, nameof(LogPayjoinSenderBroadcasted)),
             "Payjoin sender broadcasted payjoin transaction {TransactionId} for {InvoiceId}");
 
+    private static readonly Action<ILogger, string, Exception?> LogRunTestPaymentFailed =
+        LoggerMessage.Define<string>(LogLevel.Error, new EventId(2, nameof(LogRunTestPaymentFailed)),
+            "Payjoin test payment for {InvoiceId} failed with an unexpected exception");
+
     private readonly BTCPayServerEnvironment _env;
     private readonly InvoiceRepository _invoiceRepository;
     private readonly StoreRepository _storeRepository;
@@ -93,19 +98,41 @@ private static GetCheckoutBip21Response ToCheckoutResponse(GetBip21Response paym
     [AllowAnonymous]
     [IgnoreAntiforgeryToken]
     [HttpPost("run-test-payment")]
+    [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Any exception escaping a plugin controller makes BTCPay disable the plugin and stop the host process.")]
     public async Task<ActionResult<RunTestPaymentResponse>> RunTestPayment([FromBody] RunTestPaymentRequest request, CancellationToken cancellationToken)
     {
         if (request is null)
         {
-            throw new ArgumentNullException(nameof(request));
+            return BadRequest(RunTestPaymentResponse.Failure("A JSON body containing an invoiceId is required."));
         }
 
         if (string.IsNullOrWhiteSpace(request.InvoiceId))
         {
             return RunTestPaymentFailure("invoiceId is required");
         }
 
-        var invoicePaymentUrl = await _paymentUrlService.GetInvoicePaymentUrlAsync(request.InvoiceId, cancellationToken).ConfigureAwait(false);
+        try
+        {
+            return await RunTestPaymentCoreAsync(request.InvoiceId, cancellationToken).ConfigureAwait(false);
+        }
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+        {
+            throw;
+        }
+        catch (Exception ex)
+        {
+            if (_logger is not null)
+            {
+                LogRunTestPaymentFailed(_logger, request.InvoiceId, ex);
+            }
+
+            return RunTestPaymentFailure($"The test payment for invoice {request.InvoiceId} failed unexpectedly: {ex.Message}");
+        }
+    }
+
+    private async Task<ActionResult<RunTestPaymentResponse>> RunTestPaymentCoreAsync(string invoiceId, CancellationToken cancellationToken)
+    {
+        var invoicePaymentUrl = await _paymentUrlService.GetInvoicePaymentUrlAsync(invoiceId, cancellationToken).ConfigureAwait(false);
         if (invoicePaymentUrl is null)
         {
             return RunTestPaymentFailure("paymentUrl not available for invoice");
@@ -121,7 +148,7 @@ public async Task<ActionResult<RunTestPaymentResponse>> RunTestPayment([FromBody
             return RunTestPaymentFailure("invoice paymentUrl invalid");
         }
 
-        var invoice = await _invoiceRepository.GetInvoice(request.InvoiceId).ConfigureAwait(false);
+        var invoice = await _invoiceRepository.GetInvoice(invoiceId).ConfigureAwait(false);
         if (invoice is null)
         {
             return RunTestPaymentFailure("invoice not found");
@@ -185,7 +212,7 @@ public async Task<ActionResult<RunTestPaymentResponse>> RunTestPayment([FromBody
         }
 
         var runTestPaymentContext = new RunTestPaymentContext(
-            request.InvoiceId,
+            invoiceId,
             canonicalPaymentUrl,
             ohttpRelayUrls,
             paymentAddressValue,
@@ -197,7 +224,7 @@ public async Task<ActionResult<RunTestPaymentResponse>> RunTestPayment([FromBody
             var txid = await _runTestPaymentService.ExecuteAsync(runTestPaymentContext, cancellationToken).ConfigureAwait(false);
             if (_logger is not null)
             {
-                LogPayjoinSenderBroadcasted(_logger, txid, request.InvoiceId, null);
+                LogPayjoinSenderBroadcasted(_logger, txid, invoiceId, null);
             }
 
             return RunTestPaymentSuccess($"Payjoin transaction broadcasted: {txid}", txid);