From 729396302a8b7bf40042e302c0def8151159d5fd Mon Sep 17 00:00:00 2001
From: Your Name <you@example.com>
Date: Sun, 13 Sep 2026 22:48:04 +0930
Subject: [PATCH] fix(federation): swallow ConnectionException on synchronous
 AP delivery

queueDelivery() runs synchronously from the v1 follow/unfollow endpoints
(via Helpers::sendSignedObject), which commit local state before delivery
and have no try/catch. After the Http::send() rewrite, a ConnectionException
from a momentarily-unreachable remote was rethrown out of queueDelivery(),
turning a best-effort delivery into a 500 for the user after the follow/
unfollow was already persisted. For unfollows, a retry then hit the
isFollowing==false branch and never re-sent the Undo, diverging state.

Treat transport failures (ConnectionException) as best-effort on this
single-delivery path: log, record host health, and return without
propagating. Other exception types (invalid sender/destination, signing,
serialization) still throw, matching pre-rewrite precondition behavior.

Also widen SendUpdateActor's per-user catch from HttpException to Throwable
so a single bad host no longer aborts a fleet-wide actor update (the old
HttpException catch is dead for ConnectionException/invalid-destination).
---
 .../Commands/Admin/SendUpdateActor.php        |  6 +-
 app/Services/ActivityPubDeliveryService.php   | 11 +++
 .../ActivityPubDeliveryConnectionTest.php     | 93 +++++++++++++++++++
 3 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 tests/Feature/ActivityPubDeliveryConnectionTest.php

diff --git a/app/Console/Commands/Admin/SendUpdateActor.php b/app/Console/Commands/Admin/SendUpdateActor.php
index 9f1df231d..98981c0ea 100644
--- a/app/Console/Commands/Admin/SendUpdateActor.php
+++ b/app/Console/Commands/Admin/SendUpdateActor.php
@@ -8,7 +8,6 @@ use App\Models\User;
 use App\Util\ActivityPub\Helpers;
 use Illuminate\Console\Command;
 use Illuminate\Support\Facades\Storage;
-use Symfony\Component\HttpKernel\Exception\HttpException;
 
 class SendUpdateActor extends Command
 {
@@ -97,7 +96,10 @@ class SendUpdateActor extends Command
                     $body = $this->updateObject($profile);
                     try {
                         Helpers::sendSignedObject($profile, $url, $body);
-                    } catch (HttpException $e) {
+                    } catch (\Throwable $e) {
+                        // Best-effort per user: a single bad host (transport
+                        // failure, invalid destination, etc.) must not abort the
+                        // fleet-wide actor update.
                         continue;
                     }
                     $bar->advance();
diff --git a/app/Services/ActivityPubDeliveryService.php b/app/Services/ActivityPubDeliveryService.php
index ac8e48c85..aac3da0b8 100644
--- a/app/Services/ActivityPubDeliveryService.php
+++ b/app/Services/ActivityPubDeliveryService.php
@@ -152,6 +152,17 @@ class ActivityPubDeliveryService
                 'error' => $e->getMessage(),
             ]);
 
+            // Transport failures (remote momentarily unreachable: connection
+            // refused / timeout / DNS) are best-effort — this single-delivery
+            // path runs synchronously from follow/unfollow, which already
+            // committed local state. Log + record host health, but don't
+            // propagate to the caller (matches the old non-throwing curl path).
+            // Other exception types (invalid sender/destination, signing,
+            // serialization) still throw, as they did before the rewrite.
+            if ($e instanceof ConnectionException) {
+                return;
+            }
+
             throw $e;
         }
     }
diff --git a/tests/Feature/ActivityPubDeliveryConnectionTest.php b/tests/Feature/ActivityPubDeliveryConnectionTest.php
new file mode 100644
index 000000000..4dee04f91
--- /dev/null
+++ b/tests/Feature/ActivityPubDeliveryConnectionTest.php
@@ -0,0 +1,93 @@
+<?php
+
+use App\Models\User;
+use App\Services\ActivityPubDeliveryService;
+use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
+use Illuminate\Http\Client\ConnectionException;
+use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Http;
+
+uses(LazilyRefreshDatabase::class);
+
+/*
+|--------------------------------------------------------------------------
+| ActivityPubDeliveryService single-delivery transport failures
+|--------------------------------------------------------------------------
+|
+| queueDelivery() runs synchronously from follow/unfollow (via
+| Helpers::sendSignedObject), which already commit local state before delivery
+| and have no try/catch. A momentarily-unreachable remote raises a
+| ConnectionException from Http::send(); that must be treated as best-effort
+| (logged, host-health recorded) and NOT propagated to the caller, or the
+| follow/unfollow request 500s after the DB mutation is committed. Other
+| exception types (bad sender/destination) must still throw.
+|
+*/
+
+/**
+ * Run a callback with the app environment temporarily set to production, since
+ * delivery is skipped outside production. Restores env afterwards.
+ */
+function deliverAsProduction(callable $fn): mixed
+{
+    $app = app();
+    $previous = $app['env'];
+    $app['env'] = 'production';
+
+    try {
+        return $fn();
+    } finally {
+        $app['env'] = $previous;
+    }
+}
+
+it('does not propagate a ConnectionException from a momentarily-unreachable remote', function () {
+    Http::fake(function () {
+        throw new ConnectionException('cURL error 7: Failed to connect to remote.example (Connection refused)');
+    });
+
+    $user = User::factory()->create();
+    $user->refresh();
+    $sender = $user->profile;
+
+    // Seed AFTER creating the user: model/factory setup and the lazy DB refresh
+    // can flush the cache store, which would wipe an earlier seed. Seed the DNS
+    // cache so validateDestination treats the host as publicly resolvable, and
+    // the banned-domains cache so the production ban check hits cache
+    // (validateUrl runs its ban check only in production).
+    Cache::put('helpers:url:public-ips:'.hash('xxh128', 'remote.example'), ['203.0.113.40'], 3600);
+    Cache::put('instances:banned:domains', [], 1209600);
+
+    $payload = [
+        '@context' => 'https://www.w3.org/ns/activitystreams',
+        'id' => $sender->permalink('#follow/1'),
+        'type' => 'Follow',
+        'actor' => $sender->permalink(),
+        'object' => 'https://remote.example/users/target',
+    ];
+
+    // Must complete without throwing (best-effort delivery).
+    deliverAsProduction(function () use ($sender, $payload) {
+        (new ActivityPubDeliveryService)
+            ->from($sender)
+            ->to('https://remote.example/users/target/inbox')
+            ->payload($payload)
+            ->send();
+    });
+
+    // Reaching here without an exception is the assertion.
+    expect(true)->toBeTrue();
+});
+
+it('still throws for a missing sender (non-transport error)', function () {
+    $user = User::factory()->create();
+    $user->refresh();
+
+    // No ->from(): validateSender/precondition path must still throw, unchanged.
+    expect(fn () => deliverAsProduction(function () {
+        (new ActivityPubDeliveryService)
+            ->to('https://remote.example/users/target/inbox')
+            ->payload(['type' => 'Follow'])
+            ->send();
+    }))->toThrow(InvalidArgumentException::class);
+});