Skip to content

Report automatic queue startup failures - #1390

Open
dahlia wants to merge 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/cfworkers/report-queue-startup-failures
Open

dahlia wants to merge 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/cfworkers/report-queue-startup-failures

Conversation

@dahlia

@dahlia dahlia commented Nov 5, 2026

Copy link
Copy Markdown
Member

Fixes #1381.

Why

manuallyStartQueue defaults to false, so inbox handling and Context.sendActivity() start queue workers implicitly and drop the returned promise. WorkersMessageQueue.listen() throws synchronously because Cloudflare Queues cannot be polled. The throw also aborted the loop that collected listener promises, so listeners started before it were never awaited and later queues never started.

How

#listenQueue() converts synchronous throws to rejected promises, so all started listener promises reach Promise.all() and a failing queue no longer blocks the rest. The three automatic call sites now use _startQueueAutomatically(), which logs one warning per federation instance instead of leaking the rejection. Federation.startQueue() keeps rejecting. A failed worker is not retried, because its started flag is set before listen() is called. The changelog and the manuallyStartQueue reference say so.

Tests

Regression tests in packages/fedify/src/federation/middleware.test.ts cover the fetch() path, a rejection that arrives after startup, the explicit startQueue() rejection, a failing queue next to healthy ones, and the outbox and fanout send paths. They pass on Deno, Node.js, and Bun, and mise run check and the docs build pass.

The Cloudflare Workers example and the deployment, message queue, and federation pages now set manuallyStartQueue: true.

Automatic queue worker startup is best-effort: inbox handling and
Context.sendActivity() call _startQueueInternal() without awaiting it.
WorkersMessageQueue.listen() throws synchronously because Cloudflare
Queues cannot be polled, so the async call produced an unhandled
rejection.  The synchronous throw also aborted the loop that collects
listener promises, leaving listeners started before the throw
unobserved and later queues never started.

Normalize a synchronous throw from listen() into a rejected promise so
every started listener reaches Promise.all, and route the three implicit
call sites through a helper that logs the failure once instead of
leaving the rejection unhandled.  Explicit Federation.startQueue()
keeps rejecting so deliberate worker startup failures stay visible.
The failed worker is not retried, which the changelog and the
manuallyStartQueue reference now state.

Cloudflare Workers cannot start workers in the request process at all,
so the example and the deployment and message queue documentation now
set manuallyStartQueue to true.

Fixes fedify-dev#1381

Assisted-by: OpenCode:deepseek-flash
Assisted-by: Codex:gpt-6-astra
Assisted-by: Claude Code:claude-fable-5-1
@dahlia dahlia self-assigned this Nov 5, 2026
@dahlia dahlia added component/federation Federation object related component/mq Message queue related runtime/cfworkers Cloudflare Workers runtime related labels Nov 5, 2026
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Automatic queue startup failures are logged instead of becoming unhandled rejections. Explicit Federation.startQueue() calls still reject on failure. Cloudflare Workers examples and documentation configure manual queue processing through the queue() handler.

Changes: Queue Startup Failure Handling

Layer / File(s) Summary
Automatic and explicit startup behavior
packages/fedify/src/federation/middleware.ts, packages/fedify/src/federation/federation.ts, packages/fedify/src/federation/middleware.test.ts, CHANGES.md, changes.d/fedify/queue-startup-failures.md, docs/manual/federation.md
Queue listeners convert synchronous throws into rejected promises. Automatic startup catches failures and logs a warning; explicit startup continues to reject. Tests cover synchronous and asynchronous failures, request handling, queued messages, and startup of later workers.
Cloudflare Workers manual queue processing
examples/cloudflare-workers/src/index.ts, packages/cfworkers/README.md, docs/manual/deploy.md, docs/manual/mq.md
Cloudflare Workers examples set manuallyStartQueue: true and process messages through the queue() handler. The examples and guidance describe the listener limitation and update processQueuedTask() argument order where shown.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant InboxRequest
  participant FederationMiddleware
  participant QueueListener
  participant QueueLogger
  InboxRequest->>FederationMiddleware: trigger automatic startup
  FederationMiddleware->>QueueListener: call listen()
  QueueListener-->>FederationMiddleware: reject on startup failure
  FederationMiddleware->>QueueLogger: log startup warning
Loading

Possibly related issues

  • fedify-dev/fedify#1389 — The PR handles automatic startup rejection logging, but the described fanout-only early-return case remains unchanged.
  • fedify-dev/fedify#1383 — The PR handles automatic startup rejections; the issue’s waitUntil hook remains outside this change.

Possibly related PRs

  • fedify-dev/fedify#806 — Adds custom task queues that use the queue startup behavior changed here.
  • fedify-dev/fedify#1321 — Updates the same Cloudflare queue listener failure and manual message-processing path.

Suggested reviewers: 2chanhaeng

Merge Risk: Minimal · up to 772fb

No merge-blocking risk is established. Cloudflare deployments should use manual queue processing as documented.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Linked Issues check Inconclusive Linked issue #1381 has one directly linked objective set. Source evidence in middleware.ts at the reviewed head implements the production behavior: (1) #listenQueue() wraps each queue.listen() call in… Provide readable evidence of the regression tests in packages/fedify/src/federation/middleware.test.ts at the reviewed head. Confirm that a test uses a queue whose listen() throws synchronously, drives the fetch() path, and checks for no un…
Passed Passed checks: 4 passed

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: reporting failures during automatic queue startup.
Description check Passed The description directly explains the queue startup failure fix, automatic warning behavior, explicit start behavior, tests, and documentation changes.
Out of Scope Changes check Passed All changes in packages/fedify/src/federation/middleware.ts relate to issue #1381. #listenQueue() normalizes synchronous listen() throws. _startQueueAutomatically() logs automatic startup failures. Th…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Out of Scope Changes check

Explanation

All changes in packages/fedify/src/federation/middleware.ts relate to issue #1381. #listenQueue() normalizes synchronous listen() throws. _startQueueAutomatically() logs automatic startup failures. The three automatic call sites use the new helper. Explicit startup behavior is unchanged. No unrelated logic changed in the verified diff. The PR summaries also describe test, changelog (changes.d/fedify/queue-startup-failures.md and CHANGES.md), documentation (federation.md, deploy.md, mq.md, cfworkers README), and federation.ts JSDoc changes. These support the same behavior change, and the issue names the documentation and example files. The Cloudflare Workers example changes described in the summary (typed Env and Message values, env passed as context data and to processQueuedTask) sit in the example file that the issue asks to edit, and they concern the same Workers configuration. These example and documentation files were not read directly, so this verdict rests on the summaries for those files.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 4 files. (6 skipped: 6 unsupported.)

Full details: Linked Issues check

Explanation

Linked issue #1381 has one directly linked objective set. Source evidence in middleware.ts at the reviewed head implements the production behavior: (1) #listenQueue() wraps each queue.listen() call in try/catch and returns Promise.reject(error), so one synchronous throw no longer stops _startQueueInternal() from starting the remaining queues. (2) All started listener promises reach await Promise.all(promises). (3) The three automatic call sites (outbox enqueue path, inbox request path, and ContextImpl.sendActivity() fanout path) call _startQueueAutomatically(), which attaches a catch handler and logs warn 'Failed to start queue workers automatically: {error}' under ["fedify","federation","queue"]. (4) _startQueueInternal() still rejects, and the diff does not change Federation.startQueue(), so explicit startup keeps rejecting. The started flags are set before listen(), so a failed queue is not retried. The issue requires a regression test with a fake queue whose listen() throws, asserting that fetch() emits no unhandled rejection and that explicit startQueue() still rejects. The test file could not be read in full. The diff read failed because changed blobs are missing from the checkout, and the file read at head was truncated at 16000 bytes before the new tests. The test requirement is therefore unverified. The documentation and example updates requested by the issue (manuallyStartQueue: true in the Cloudflare Workers example, docs/manual/deploy.md, and docs/manual/mq.md) are reported in the PR description and summaries, but those files were not inspected directly.

Resolution

Provide readable evidence of the regression tests in packages/fedify/src/federation/middleware.test.ts at the reviewed head. Confirm that a test uses a queue whose listen() throws synchronously, drives the fetch() path, and checks for no unhandled rejection. Confirm that another test asserts explicit Federation.startQueue() rejects with the listen() error. Confirm that the Cloudflare Workers example and docs/manual/deploy.md and docs/manual/mq.md set manuallyStartQueue: true.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 4fb3a04f-68a1-4a79-a362-a248aa9263d0
Commits

Reviewing files that changed from the base of the PR and between 90504ae and 772fbee.

Files selected for processing: 10

  • CHANGES.md
  • changes.d/fedify/queue-startup-failures.md
  • docs/manual/deploy.md
  • docs/manual/federation.md
  • docs/manual/mq.md
  • examples/cloudflare-workers/src/index.ts
  • packages/cfworkers/README.md
  • packages/fedify/src/federation/federation.ts
  • packages/fedify/src/federation/middleware.test.ts
  • packages/fedify/src/federation/middleware.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T13:37:05.594552Z 772fbee PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codecov

codecov Bot commented Nov 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines Coverage Δ
packages/fedify/src/federation/middleware.ts 79.32% <100.00%> (-12.79%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dahlia
dahlia requested review from 2chanhaeng and sij411 November 5, 2026 15:48

This branch has not been deployed

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

Labels

component/federation Federation object related component/mq Message queue related runtime/cfworkers Cloudflare Workers runtime related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report automatic queue startup failures instead of leaving them unhandled

1 participant