Skip to content

Add a lint rule for outbox delivery that is never awaited #1057

Description

@dahlia

Why

ctx.sendActivity() returns a promise. An outbox listener that calls it without awaiting the result hands the promise to nobody, and the handler returns 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 docs/manual/send.md awaits the call, and nothing checks that user code does the same.

outbox-listener-delivery-required does not cover this. It asks whether a delivery call runs, not whether anything waits for it. All of these pass today:

ctx.sendActivity(sender, inbox, activity);
void ctx.sendActivity(sender, inbox, activity);
ctx.sendActivity(sender, inbox, activity).catch(console.error);
const pending = ctx.sendActivity(sender, inbox, activity);
inboxes.forEach((inbox) => ctx.sendActivity(sender, inbox, activity));
deliver();

Trying to fold this into the existing rule would be a mistake. Its message tells the reader to call a delivery method, which is unhelpful advice for someone looking straight at the call they wrote, and its contract is about reachability. A separate rule can say what it means and can be turned off on its own by projects that deliberately fire and forget.

Scope

Report a delivery call inside an outbox listener whose result is discarded. A call counts as handled when it is awaited, returned, passed to something that awaits it such as Promise.all(), or handed to a runtime that takes ownership of it, ctx.waitUntil() being the case worth supporting first.

Decide deliberately what to do with the deliberate-discard spellings. void ctx.sendActivity(...) and a trailing .catch() both read as someone who has thought about it, and a rule that reports them will be turned off. Treating void as an opt-out and .catch() without an await as still reportable is one defensible split, but this deserves a decision rather than falling out of the implementation.

The existing rule's helper resolution and reachability walk are worth reusing. This rule only cares about delivery calls that run, so a call in a dead branch is not its problem.

Non-goals

No type information and no cross-file analysis, the same boundary #900 set. Nothing about whether the delivery succeeded at the protocol level. Do not make this rule part of the recommended set until it has been tried against real applications; an unawaited delivery is a strong smell rather than a certain bug.

Suggested checks

Cover each spelling above, plus an awaited call, await Promise.all(...) over several, a returned promise, and ctx.waitUntil(). Add the rule to docs/manual/lint.md with an explanation of why an unawaited delivery can be lost, and link it from the outbox-listener-delivery-required section, since a reader who satisfies one rule will want to know the other exists.

Background

This came out of reviewing #1050, which rewrote outbox-listener-delivery-required around a reachability contract. The third pattern in #900 asked for array.map(() => ctx.sendActivity(...)) to be reported when the callback is not awaited or returned. That case belongs here: map() does invoke its callback, so the delivery call runs, and what is wrong with the code is that nothing waits for the promise.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Fields

Priority

None yet

Effort

None yet

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions