Add a lint rule for outbox delivery that is never awaited

CHANGES.md

@@ -283,6 +283,20 @@ To be released.
     when an actor dispatcher's return value does not include a
     `preferredUsername` property. [[#895], [#1022] by Jae-Hyuk-Jang\]
 
+ -  Added the `outbox-listener-delivery-not-awaited` rule to `@fedify/lint`.
+    It reports an outbox listener that calls `ctx.sendActivity()` or
+    `ctx.forwardActivity()` and drops the returned promise, so that the
+    activity may never leave on a runtime such as Cloudflare Workers, which
+    discards pending work once the response is returned. A call counts as
+    handled when its promise is awaited, returned, passed to `Promise.all()`
+    or `Promise.allSettled()`, or handed to `waitUntil()`, and `void`,
+    `Promise.race()` and `Promise.any()` are accepted as deliberate choices to
+    stop waiting.
+    The ESLint `recommended` configuration enables the rule as a warning and
+    `strict` as an error, and Oxlint users enable it by name. It is not
+    available in Deno Lint, which turns on every rule of a plugin at once.
+    [[#1057], [#1067] by Jae-Hyuk-Jang\]
+
  -  Changed `outbox-listener-delivery-required` (`@fedify/lint`) to decide
     whether a `ctx.sendActivity()`/`ctx.forwardActivity()` call actually
     runs, instead of scanning the listener's source as a flat block of text.
@@ -298,6 +312,8 @@ To be released.
 [#900]: https://github.com/fedify-dev/fedify/issues/900
 [#1022]: https://github.com/fedify-dev/fedify/pull/1022
 [#1050]: https://github.com/fedify-dev/fedify/pull/1050
+[#1057]: https://github.com/fedify-dev/fedify/issues/1057
+[#1067]: https://github.com/fedify-dev/fedify/pull/1067
 
 ### @fedify/mysql
 

changes.d/lint/outbox-delivery-not-awaited.md

@@ -0,0 +1,18 @@
+---
+links:
+  '#1057': https://github.com/fedify-dev/fedify/issues/1057
+  '#1067': https://github.com/fedify-dev/fedify/pull/1067
+---
+ -  Added the `outbox-listener-delivery-not-awaited` rule to `@fedify/lint`.
+    It reports an outbox listener that calls `ctx.sendActivity()` or
+    `ctx.forwardActivity()` and drops the returned promise, so that the
+    activity may never leave on a runtime such as Cloudflare Workers, which
+    discards pending work once the response is returned. A call counts as
+    handled when its promise is awaited, returned, passed to `Promise.all()`
+    or `Promise.allSettled()`, or handed to `waitUntil()`, and `void`,
+    `Promise.race()` and `Promise.any()` are accepted as deliberate choices to
+    stop waiting.
+    The ESLint `recommended` configuration enables the rule as a warning and
+    `strict` as an error, and Oxlint users enable it by name. It is not
+    available in Deno Lint, which turns on every rule of a plugin at once.
+    [[#1057], [#1067] by Jae-Hyuk-Jang]

docs/manual/lint.md

@@ -781,9 +781,9 @@ explicit delivery path, and that path must actually run.
 
 The rule checks that a delivery call exists and can run, not that the delivery
 completes, so a listener it accepts is not guaranteed to federate.  A delivery
-call that is never awaited is not reported;
-[#1057] tracks a rule for
-that.
+call that is never awaited is not reported here; the
+[`outbox-listener-delivery-not-awaited`](#outbox-listener-delivery-not-awaited)
+rule checks for that.
 
 ~~~~ typescript twoslash
 // @noErrors: 2345
@@ -859,7 +859,136 @@ federation
   });
 ~~~~
 
-[#1057]: https://github.com/fedify-dev/fedify/issues/1057
+### `outbox-listener-delivery-not-awaited`
+
+Warns when an outbox listener calls `ctx.sendActivity()` or
+`ctx.forwardActivity()` and lets the returned promise go without waiting for it.
+
+::: info
+This rule is available in ESLint and Oxlint, but not in Deno Lint: Deno turns on
+every rule of a plugin as soon as the plugin is listed, and gives a project no
+way to keep one off until it asks for it.  In ESLint, the *recommended*
+configuration enables it as a warning and *strict* as an error.  In Oxlint,
+enable it by name.
+:::
+
+**When this rule applies:**
+You've registered an outbox listener with `setOutboxListeners()`, and it calls
+`ctx.sendActivity()` or `ctx.forwardActivity()` in a way that drops the
+returned promise.  The rule follows the promise from the call to where it ends
+up.  A call counts as handled when its promise is awaited, returned, passed to
+`Promise.all()` or `Promise.allSettled()`, or handed to a method named
+`waitUntil()`.  A call is reported when its promise is discarded, including when
+it is only kept in a variable that nothing else uses.
+
+When it cannot tell where a promise goes, the rule stays quiet.  In practice:
+
+ -  `void ctx.sendActivity(...)`, `Promise.race(...)` and `Promise.any(...)` are
+    read as deliberate choices to stop waiting, and are not reported.  A
+    `race()` or `any()` whose own result is dropped is still reported, and so is
+    any other operator applied to a promise, such as `!` or `typeof`.  So is a
+    promise used as the test of an `if` statement, a loop or a conditional
+    expression, since a promise is always truthy and testing it waits for
+    nothing.  A discarded `.catch()`, `.then()` or `.finally()` chain is
+    reported, since a `.catch()` handles the error but does not wait.
+ -  An array of promises, such as the result of `map()`, waits for nothing on
+    its own, and neither does an object that holds a promise, such as
+    `{ pending: ctx.sendActivity(...) }`.  Awaiting one, or returning it from
+    the listener, is reported.  An array counts as handled once it reaches
+    `Promise.all()` or one of its siblings, and either one counts as handled
+    when it is kept in a variable that is mentioned again.
+ -  An `async` callback that awaits a delivery is judged by where the callback
+    goes, since its own promise is what carries the delivery.  `forEach()` drops
+    that promise, so `inboxes.forEach(async (inbox) => { await ... })` is
+    reported.  A callback that is invoked immediately, or given to `map()` or
+    `then()`, is only as safe as the result of that call.  The same holds for a
+    function passed by name, as in `inboxes.forEach(deliver)`.
+ -  A local helper that delivers is judged by how it is called: `deliver();` is
+    reported when `deliver()` awaits or returns a delivery, and
+    `await deliver();` is not.  An array of promises, or an object holding one,
+    that a helper returns is not followed to where the helper is called, since
+    the caller may pass it to `Promise.all()`.  So `await deliverAll();` and a
+    bare `deliverAll();` are not reported when `deliverAll()` returns the
+    result of `map()`.
+ -  A promise passed to a function the rule does not know, such as
+    `queue.push(...)` or `setTimeout(...)`, is left alone, since the rule cannot
+    tell what that function does with it.
+ -  The rule leans towards quiet where a name is only mentioned.  A stored
+    promise counts as used when its variable is mentioned anywhere in the
+    listener, even only in a dead branch or in a helper that is never called.  A
+    helper that delivers counts as running once its name is mentioned, even if
+    it is only stored or logged, so a bare delivery call inside it is still
+    reported.
+ -  As in `outbox-listener-delivery-required`, the rule reads only the listener
+    body.  A delivery call in a helper that is declared outside the listener,
+    or in another module, is not seen.
+
+**Why it matters:**
+`ctx.sendActivity()` returns a promise.  A listener that calls it without
+waiting hands that promise to nobody, and the handler can return while delivery
+is still in flight.  On a long-lived Node.js or Deno process this usually works
+out.  On Cloudflare Workers, which Fedify supports through `@fedify/cfworkers`,
+pending work is dropped once the response is returned, so the activity may never
+leave.  Every `sendActivity()` example in [*Sending activities*](./send.md)
+awaits the call.
+
+This rule is separate from
+[`outbox-listener-delivery-required`](#outbox-listener-delivery-required),
+which asks whether a delivery call exists and can run, not whether anything
+waits for it.
+
+~~~~ typescript twoslash
+// @noErrors: 2345
+import { createFederation } from "@fedify/fedify";
+import { Activity } from "@fedify/vocab";
+import type { Recipient } from "@fedify/vocab";
+const federation = createFederation<void>({ kv: null as any });
+declare const recipients: Recipient[];
+declare const executionCtx: { waitUntil(promise: Promise<unknown>): void };
+// ---cut-before---
+// ❌ Bad: The promise is dropped, so the activity can be lost
+federation
+  .setOutboxListeners("/users/{identifier}/outbox")
+  .on(Activity, async (ctx, activity) => {
+    ctx.sendActivity({ identifier: ctx.identifier }, "followers", activity);
+  });
+
+// ❌ Bad: forEach() discards what its callback returns
+federation
+  .setOutboxListeners("/users/{identifier}/outbox")
+  .on(Activity, async (ctx, activity) => {
+    recipients.forEach((recipient) =>
+      ctx.sendActivity({ identifier: ctx.identifier }, recipient, activity)
+    );
+  });
+
+// ✅ Good: The delivery is awaited
+federation
+  .setOutboxListeners("/users/{identifier}/outbox")
+  .on(Activity, async (ctx, activity) => {
+    await ctx.sendActivity({ identifier: ctx.identifier }, "followers", activity);
+  });
+
+// ✅ Good: Every delivery is awaited together
+federation
+  .setOutboxListeners("/users/{identifier}/outbox")
+  .on(Activity, async (ctx, activity) => {
+    await Promise.all(
+      recipients.map((recipient) =>
+        ctx.sendActivity({ identifier: ctx.identifier }, recipient, activity)
+      ),
+    );
+  });
+
+// ✅ Good: The runtime keeps the work alive after the response is returned
+federation
+  .setOutboxListeners("/users/{identifier}/outbox")
+  .on(Activity, async (ctx, activity) => {
+    executionCtx.waitUntil(
+      ctx.sendActivity({ identifier: ctx.identifier }, "followers", activity),
+    );
+  });
+~~~~
 
 ### `media-uploader-object-uri-required`
 

packages/lint/README.md

@@ -140,6 +140,9 @@ federation code:
     callback does not derive its return value from `getObjectUri`
  -  **`media-uploader-authorization-required`**: Warns when `setMediaUploader`
     is registered without an `authorize` hook
+ -  **`outbox-listener-delivery-not-awaited`**: Warns when an outbox listener
+    calls `sendActivity` or `forwardActivity` and does not wait for the result
+    (ESLint and Oxlint only)
 
 
 Installation

packages/lint/src/index.ts

@@ -81,6 +81,9 @@ import {
 import {
   eslint as mediaUploaderObjectUriRequired,
 } from "./rules/media-uploader-object-uri-required.ts";
+import {
+  eslint as outboxListenerDeliveryNotAwaited,
+} from "./rules/outbox-listener-delivery-not-awaited.ts";
 import {
   eslint as outboxListenerDeliveryRequired,
 } from "./rules/outbox-listener-delivery-required.ts";
@@ -116,6 +119,7 @@ const rules: Record<
   [RULE_IDS.actorPreferredUsernameRequired]: actorPreferredUsernameRequired,
   [RULE_IDS.collectionFilteringNotImplemented]: collectionFiltering,
   [RULE_IDS.outboxListenerDeliveryRequired]: outboxListenerDeliveryRequired,
+  [RULE_IDS.outboxListenerDeliveryNotAwaited]: outboxListenerDeliveryNotAwaited,
   [RULE_IDS.mediaUploaderObjectUriRequired]: mediaUploaderObjectUriRequired,
   [RULE_IDS.mediaUploaderAuthorizationRequired]:
     mediaUploaderAuthorizationRequired,

packages/lint/src/lib/const.ts

@@ -159,6 +159,7 @@ export const RULE_IDS = {
 
   // Listener rules
   outboxListenerDeliveryRequired: "outbox-listener-delivery-required",
+  outboxListenerDeliveryNotAwaited: "outbox-listener-delivery-not-awaited",
   mediaUploaderObjectUriRequired: "media-uploader-object-uri-required",
   mediaUploaderAuthorizationRequired: "media-uploader-authorization-required",
 } as const;