Skip to content

feat: retry and time out external link fetches - #185

Draft
claude[bot] wants to merge 2 commits into
mainfrom
robust-external-link-fetching
Draft

feat: retry and time out external link fetches#185
claude[bot] wants to merge 2 commits into
mainfrom
robust-external-link-fetching

Conversation

@claude

@claude claude Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Requested by David Sanders · Slack thread

Before: a single stalled external link could hang lint-roller-markdown-links --fetch-external-links for minutes and flake CI. In this scheduled run, fetching https://www.electronjs.org/docs/api/browser-window hung past the 60s test timeout.

After: each link fetch attempt is hard-aborted after 10s of wall-clock time and retried up to 3 times with backoff, so a stalled link fails fast (worst case ~43.5s per link) instead of hanging the check.

How

  • fetchExternalLink now uses a small manual retry loop: up to 4 attempts total (1 + 3 retries) with 500ms/1s/2s backoff between attempts. It retries on any thrown error and on 429 or >= 500 responses.
  • Every attempt carries a hard AbortSignal.timeout(10_000) — this is the load-bearing guard. undici's connect/headers/body timeouts are idle-based, so a redirect chain that stalls mid-hop or a server that slowly drips bytes never goes idle long enough to trip them (which is how the flake survived an earlier per-phase-timeouts-plus-RetryAgent approach). A wall-clock abort per attempt covers connect hangs, mid-redirect stalls, and slow-drip bodies alike, and turns them all into retryable errors.
  • The dispatcher is a plain undici Agent (5s connect, 10s headers, 10s body) so common failures still fail fast; no RetryAgent or error-code lists needed since the loop retries on anything thrown.
  • Worst case per link ≈ 4×10s + 3.5s backoff = 43.5s, comfortably under vitest's 60s test timeout, so the live --check-redirects test can no longer be outlived by a single hung link.
  • On persistent failure, downstream behavior is unchanged: the last error is rethrown and reported as Broken link <url>, and a persistent non-OK response is returned and reported with its HTTP status.
  • Requires Node >=20.18.1 — this uses undici v7, which needs Node >=20.18.1, so engines is bumped to >=20.18.1 || >=22.4.0 (per maintainer request) and the CI test matrix entry moves from Node 20.16 to 20.18.1.
  • Adds two deterministic local node:http server tests: one where the server destroys the socket on the first two requests and returns 200 on the third (retry on connection errors), and one where the server accepts the first connection but never responds at all, then returns 200 (hard per-attempt abort recovers the hang; runs in ~11s: 10s abort + backoff + fast success). These required a small async cp.spawn-based test helper, since the existing spawnSync helper blocks the event loop and an in-process server couldn't respond.
  • The --check-redirects test now also runs against a local server (307 -> 200) instead of live-fetching electronjs.org: the CI stalls turned out to be region-scoped egress hangs on some Azure runners that persist past 60s, so no retry policy can save a test that requires a live fetch to succeed. tests/fixtures/redirected-external-link.md is removed since nothing else referenced it.

Canary workflow (debugging)

This PR also adds .github/workflows/canary-external-links.yml, an hourly region-sampling canary for debugging the westcentralus hang. Each run spins up 3 parallel ubuntu-latest jobs (independent region draws); each job records the runner's Azure region via IMDS and then probes https://www.electronjs.org/docs/api/browser-window ~20 times with three client shapes — curl over HTTP/2 with curl's default UA, curl over HTTP/1.1 with the exact header set lint-markdown-links sends, and Node's global fetch with the same headers — capturing per-phase timings (DNS / TCP / TLS / TTFB / redirect / total) so the next time a probe lands on an affected runner we see exactly which phase stalls and for which client fingerprints. Full --trace-time -v output is kept only for slow samples (>5s) or errors; each job writes a region + timing table to its step summary. Probe failures never fail the job.

Since schedule/workflow_dispatch only fire for workflows on the default branch, the canary also has a pull_request trigger so it samples runners on every push to this PR (and can be re-run from the checks UI for more draws). It's debugging scaffolding — easy to delete once the question is answered.

Note: this will conflict with #163, which touches the same function — happy to rebase this on top of it or stack it, whichever is preferred.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NYcfm99SSXPiHtUNhBD71R


Generated by Claude Code

@socket-security

socket-security Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedundici@​7.28.09710010098100

View full report

@claude
claude Bot force-pushed the robust-external-link-fetching branch from f459c7c to f774618 Compare July 10, 2026 00:37
@claude claude Bot changed the title feat!: retry and time out external link fetches feat: retry and time out external link fetches Jul 10, 2026
@claude
claude Bot force-pushed the robust-external-link-fetching branch from f774618 to fcf8197 Compare July 10, 2026 00:52
Retry external link fetches up to 3 times with backoff and a hard 10s
wall-clock abort per attempt, since undici's per-phase timeouts are
idle-based and don't catch slow-drip responses or redirect stalls.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NYcfm99SSXPiHtUNhBD71R
@claude
claude Bot force-pushed the robust-external-link-fetching branch from fcf8197 to cb16bed Compare July 10, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant