Skip to content

Await sibling deliveries when one delivery fails #1380

Description

@dahlia

Motivation

FederationImpl.sendActivity() and the immediate forwardActivity() path start one delivery per inbox and then wait for all of them with Promise.all(). If one delivery rejects, Promise.all() rejects immediately. The method returns while the other deliveries are still running.

On Node.js, Deno, and Bun this is mostly a latency issue: the remaining requests finish in the background. On Cloudflare Workers the isolate can be terminated as soon as the error response is returned, so those siblings are canceled. A single failing recipient can suppress deliveries to every recipient that had not finished yet.

Line numbers are from main unless marked. The immediate paths are at packages/fedify/src/federation/middleware.ts:3769 and :7648. On 2.4.4 they are at :3144 and :6506, and on 2.0-maintenance at :1468 and :3348.

Reproduction

Use two recipients. Make the first send helper reject immediately and the second resolve after a short delay. Call ctx.sendActivity() with immediate: true, or call forwardActivity() with a queue-less federation, and measure when the call returns. It returns before the delayed delivery settles.

The loss is easier to see with a queue-less federation on Workers: return an error response after the first rejection, and the second request is never sent.

Expected behavior

After one delivery fails, the method should wait for every delivery it already started before propagating the original error.

Fix

Keep the original rejection, but drain the started promises in a finally block:

const promises: Promise<void>[] = [];
try {
  for (const inbox in inboxes) {
    promises.push(sendOne(/* ... */));
  }
  await Promise.all(promises);
} finally {
  await Promise.allSettled(promises);
}

sendOne() stands for sendActivityToInbox() on main and 2.4.4, and for the per-inbox sendActivity() call on 2.0-maintenance. On success allSettled() settles immediately. On failure it waits for the siblings, then the original rejection propagates.

Put the loop itself inside the try. On 2.0-maintenance a synchronous throw while preparing a later recipient currently abandons the promises pushed before it, because Promise.all() is never reached.

Add a regression test with one rejecting and one delayed destination, following the fanout regression test for #661. The test should assert that the delayed delivery has settled before sendActivity() rejects.

Scope

  • Fix 2.0-maintenance first at middleware.ts:1468 and :3348, then forward-port to later maintenance branches and main.
  • No public API change.

Metadata

Metadata

Assignees

Labels

Type

Fields

Priority

None yet

Effort

None yet

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions