Handle double-knock transport failures

[dahlia]

Summary

Fixes #762.

This fixes doubleKnock() so transport-level fetch() failures, such as transient DNS or connection errors, no longer leak raw TypeError values to callers. Idempotent signed GET and HEAD requests now get one bounded retry after a short delay, while non-idempotent requests such as inbox POST deliveries are not replayed.

The fix keeps the public API unchanged. Remaining transport failures are wrapped in FetchError with the original error preserved as cause, so callers still get a document-loading error type with access to the runtime failure. Cancellation remains separate from transport failure handling: aborts from options.signal, the original Request, or the signed Request are propagated without retrying or wrapping.

Changed files

• packages/fedify/src/sig/http.ts
• packages/fedify/src/sig/http.test.ts
• CHANGES.md

Tests

Tests cover:

• retrying a transient transport failure for an idempotent request
• wrapping repeated transport failures in FetchError
• avoiding retries for non-idempotent requests
• preserving custom abort reasons from request cancellation
• aborting cleanly while waiting to retry

Verification:

• deno test --check --allow-all --unstable-kv packages/fedify/src/sig/http.test.ts
• mise run check
• Codex self-review loop completed with no remaining findings

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ae6f9ef3-8549-4d26-b362-9044f17bffad

📥 Commits

Reviewing files that changed from the base of the PR and between dae7217 and 5aa5966.

📒 Files selected for processing (2)

• packages/fedify/src/sig/http.test.ts
• packages/fedify/src/sig/http.ts

---

📝 Walkthrough

Walkthrough

The changes add retry logic and error wrapping to doubleKnockInternal() to prevent transient transport failures from propagating as raw TypeError. Idempotent GET/HEAD requests are retried once with backoff, and all transport errors are wrapped in FetchError while preserving the original error as the cause.

Changes

Transport retry and error wrapping

Layer / File(s)	Summary
Transport retry and error wrapping helpers packages/fedify/src/sig/http.ts	Adds DOUBLE_KNOCK_TRANSPORT_RETRY_DELAY_MS constant and fetchDoubleKnockRequest helper with supporting functions (createFetchError, isAbortError, sleep, getAbortedSignal, getAbortReason). The helper performs up to two fetch attempts for idempotent methods, detects and rethrows abort errors immediately, retries non-abort transport failures after a backoff delay (which is abort-aware), and wraps remaining failures as FetchError with the original cause preserved.
Integration into signed requests packages/fedify/src/sig/http.ts	Replaces two unguarded fetch() calls in doubleKnockInternal—the initial signed-spec request and the fallback after signature spec rotation—with calls to fetchDoubleKnockRequest, applying retry and error wrapping logic to both paths.
Test coverage for retry and error handling packages/fedify/src/sig/http.test.ts	Imports FetchError and adds comprehensive test suite verifying retry behavior (idempotent GET retries on transient TypeError), error wrapping (repeated failures wrapped as FetchError with original cause and url preserved), non-retry for non-idempotent methods (POST), and abort handling (abort reasons preserved at call time and during retry delay).
Changelog documentation CHANGES.md	Documents under version 2.0.17 that doubleKnock() now prevents transient transport failures from leaking raw TypeError, with idempotent authenticated document fetches retried once and remaining failures wrapped as FetchError.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested labels

type/enhancement, component/testing

Suggested reviewers

• sij411
• 2chanhaeng

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Handle double-knock transport failures' accurately summarizes the main change: adding transport failure handling and retry logic to the doubleKnock() function.
Description check	✅ Passed	The description is well-related to the changeset, explaining the purpose of transport failure handling, retry strategy, and the unchanged public API.
Linked Issues check	✅ Passed	The PR implements all key requirements from issue #762: wrapping transport errors in FetchError with original error as cause, adding bounded retry for idempotent GET/HEAD requests, avoiding retries for non-idempotent requests, and preserving cancellation without retrying.
Out of Scope Changes check	✅ Passed	All changes are in-scope: http.ts refactors doubleKnockInternal with fetchDoubleKnockRequest helper for retry logic, http.test.ts adds comprehensive transport failure tests, and CHANGES.md documents the fix.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[dahlia]

@codex review

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/fedify/src/sig/http.ts`:
- Around line 1339-1370: The final throw new FetchError(request.url) at the end
of fetchDoubleKnockRequest is unreachable because the for loop always returns or
throws (successful fetch, abort via isAbortError, or createFetchError on final
attempt); remove that trailing throw to clean up dead code and keep the existing
retry/throw behavior (retain maxAttempts, the attempt loop, isAbortError check,
createFetchError usage, and the sleep retry).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e28f5a92-d1a9-422b-8dc3-85e81adb73d0

📥 Commits

Reviewing files that changed from the base of the PR and between ff5b755 and dae7217.

📒 Files selected for processing (3)

• CHANGES.md
• packages/fedify/src/sig/http.test.ts
• packages/fedify/src/sig/http.ts

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[gemini-code-assist]

Code Review

This pull request enhances the doubleKnock() function to handle transient transport failures by retrying idempotent requests (GET/HEAD) once and wrapping persistent failures in a FetchError. It introduces several helper functions for retry logic, abort-aware sleep, and error handling, along with comprehensive unit tests. Feedback includes a recommendation to prioritize abort reasons over transport errors in the retry loop to ensure proper cancellation propagation and the removal of an unreachable return statement at the end of the new fetch helper.

[codecov]

Codecov Report

❌ Patch coverage is 87.65432% with 10 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
packages/fedify/src/sig/http.ts	87.65%	4 Missing and 6 partials ⚠️

Files with missing lines	Coverage Δ
packages/fedify/src/sig/http.ts	75.18% <87.65%> (+0.79%)	⬆️

... and 1 file 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]

/gemini review

[dahlia]

@coderabbitai review

[dahlia]

@codex review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[gemini-code-assist]

Code Review

This pull request enhances the doubleKnock() function to handle transient transport failures by implementing a retry mechanism for idempotent requests (GET and HEAD). It introduces the fetchDoubleKnockRequest helper to manage these attempts and a sleep utility that integrates with multiple AbortSignals to ensure responsive cancellation. Additionally, the changes include comprehensive unit tests covering retry behavior, error wrapping, and abort signal priority. I have no feedback to provide.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🚀

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".