Added timeout to queue outbound requests to minimize in-flight requests

[rmgpinto]

ref https://linear.app/ghost/issue/BER-3241

• Adds a 10s timeout to all outbound requests. The timeout is more than enough for servers to process the request and triggers a fail-fast mechanism that frees up resources.

[coderabbitai]

Walkthrough

This pull request adds timeout handling for outbound requests in a Google Cloud Pub/Sub push message handler. The error-utils file now treats TimeoutError as a network connectivity error to enable retries. The mq.ts file introduces a global fetch wrapper that applies a 10-second timeout to outbound requests during message processing, using AsyncLocalStorage to manage timeout context across async boundaries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding a timeout to queue outbound requests to minimize in-flight requests, which aligns with the changeset modifications to error handling and fetch timeout logic.
Description check	✅ Passed	The description is directly related to the changeset, explaining the purpose of adding a 10s timeout to outbound requests and the intended fail-fast mechanism to free up resources.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch timeout-remote-http

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/mq/gcloud-pubsub-push/mq.ts`:
- Around line 23-29: The fetch wrapper currently skips enforcing the timeout
when callers pass init.signal; update the globalThis.fetch override (the wrapper
around originalFetch) to merge the caller's init.signal with the timeout signal
from fetchTimeoutStorage instead of choosing one: create an AbortController, if
init.signal exists attach a listener to abort the controller when the caller
signal aborts, use AbortSignal.timeout(timeout) to set a timeout that aborts the
controller when fired (or race both by wiring both signals to the controller),
pass controller.signal into originalFetch and ensure any listeners are cleaned
up after the request; reference the globalThis.fetch wrapper, originalFetch,
fetchTimeoutStorage.getStore(), and init.signal when making the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fcab610-434f-4a31-91a9-fc72ba2346eb

📥 Commits

Reviewing files that changed from the base of the PR and between feb6aa7 and 706a835.

📒 Files selected for processing (2)

• src/mq/gcloud-pubsub-push/error-utils.ts
• src/mq/gcloud-pubsub-push/mq.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In the Fetch API, when fetchis called with aRequestobject plus aninitobject, doesinit.signaloverride the request's existing signal? Also, isAbortSignal.any() available in the Node.js version this project targets?

💡 Result:

• Yes. When you call fetch(request, init), the Fetch spec effectively runs the Request constructor with input=request and your init. In that constructor, it starts with the request’s existing signal, but if init.signal is present it replaces (overrides) that signal, and the created Request ends up using a dependent signal based on the (possibly replaced) one. [1]

• AbortSignal.any() availability in Node.js: It exists in Node.js v18.17.0+ (backported into the Node 18 line) and later. So it’s available only if your project’s target Node version is ≥ 18.17.0. [2]

Sources:
[1] Fetch Standard (WHATWG), new Request(input, init) steps (init["signal"] sets/overrides the signal)
[2] Node.js v18 global docs (AbortSignal.any(signals) “Added in: v18.17.0”)

---

🏁 Script executed:

#!/bin/bash
# Check Node.js version requirement
if [ -f "package.json" ]; then
  echo "=== package.json engines field ==="
  cat package.json | jq '.engines' 2>/dev/null || grep -A2 '"engines"' package.json || echo "No engines field"
fi

if [ -f ".nvmrc" ]; then
  echo "=== .nvmrc ==="
  cat .nvmrc
fi

if [ -f ".node-version" ]; then
  echo "=== .node-version ==="
  cat .node-version
fi

Repository: TryGhost/ActivityPub

Length of output: 126

---

Preserve caller abort signals while enforcing the 10s timeout.

This wrapper skips the timeout if the caller provides init.signal, which defeats the PR's goal of enforcing timeouts on all outbound requests. Merge the signals instead of choosing one:

Fix

 globalThis.fetch = (input: RequestInfo | URL, init?: RequestInit) => {
     const timeout = fetchTimeoutStorage.getStore();
-    if (timeout && !init?.signal) {
+    if (timeout) {
+        const callerSignal =
+            init?.signal ??
+            (input instanceof Request ? input.signal : undefined);
+
         return originalFetch(input, {
             ...init,
-            signal: AbortSignal.timeout(timeout),
+            signal: callerSignal
+                ? AbortSignal.any([
+                      callerSignal,
+                      AbortSignal.timeout(timeout),
+                  ])
+                : AbortSignal.timeout(timeout),
         });
     }
     return originalFetch(input, init);
 };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	globalThis.fetch = (input: RequestInfo | URL, init?: RequestInit) => {
	const timeout = fetchTimeoutStorage.getStore();
	if (timeout && !init?.signal) {
	return originalFetch(input, {
	...init,
	signal: AbortSignal.timeout(timeout),
	});
	globalThis.fetch = (input: RequestInfo | URL, init?: RequestInit) => {
	const timeout = fetchTimeoutStorage.getStore();
	if (timeout) {
	const callerSignal =
	init?.signal ??
	(input instanceof Request ? input.signal : undefined);
	return originalFetch(input, {
	...init,
	signal: callerSignal
	? AbortSignal.any([
	callerSignal,
	AbortSignal.timeout(timeout),
	])
	: AbortSignal.timeout(timeout),
	});
	}
	return originalFetch(input, init);
	};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/mq/gcloud-pubsub-push/mq.ts` around lines 23 - 29, The fetch wrapper
currently skips enforcing the timeout when callers pass init.signal; update the
globalThis.fetch override (the wrapper around originalFetch) to merge the
caller's init.signal with the timeout signal from fetchTimeoutStorage instead of
choosing one: create an AbortController, if init.signal exists attach a listener
to abort the controller when the caller signal aborts, use
AbortSignal.timeout(timeout) to set a timeout that aborts the controller when
fired (or race both by wiring both signals to the controller), pass
controller.signal into originalFetch and ensure any listeners are cleaned up
after the request; reference the globalThis.fetch wrapper, originalFetch,
fetchTimeoutStorage.getStore(), and init.signal when making the change.

[mike182uk]

@rmgpinto Instead of monkey patching fetch, would this achieve what you are trying to do: https://nodejs.org/docs/latest-v22.x/api/globals.html#custom-dispatcher

Not sure on specific usage, but agent is suggesting something like:

import { Agent, setGlobalDispatcher } from 'undici';

setGlobalDispatcher(new Agent({
    connectTimeout: 5_000,
    headersTimeout: 10_000,
    bodyTimeout: 10_000,
}));

This might also require some tweaking in the error utils to catch whatever errors are thrown by undici (e.g Connect Timeout Error?)

Ideally we would be able to pass a customised version of fetch to Fedify but im not seeing an option for that