Race multiple config URLs; first valid response wins

[myleshorton]

Why

The config updater fetches the fronting config from a single URL (raw.githubusercontent.com/getlantern/domainfront/...), which is blocked in China. To distribute the config through a China-reachable mirror without committing to one source, the client should be able to try several and use whichever works — see getlantern/engineering#3721.

Change

WithConfigURL is now variadic: given multiple URLs, fetchAndApplyConfig fetches them concurrently and applies the first valid (fetched + parsed + applied) response. A blocked or slow source can't hold up a reachable one. Backward compatible — a single-URL call behaves exactly as before.

• Races all URLs under a shared cancellable context; once one wins, the losers' in-flight fetches are cancelled. The results channel is buffered to the URL count, so no losing goroutine can leak on its send.
• A source that fetches + parses but fails applyConfig (e.g. a corrupt mirror serving zero providers) is skipped, not treated as terminal — the point of a backup mirror is to cover for exactly that. Only when all sources fail is a single Warn logged; per-source failures are Debug (a blocked mirror is expected when racing).

Testing

go build, go vet, gofmt -l, go test -race ./... all pass (racing tests also run clean under -count=5). New tests:

• race to first valid — a failing (500) source listed first doesn't block a reachable one;
• skip an unapplyable source — an immediate providers: {} source is skipped in favor of a deliberately-delayed valid one (deterministically exercises the skip-and-continue path; would fail with a naive return);
• all sources fail — construction still succeeds and the seed config is retained.

Follow-up

This is the client-side primitive for #3721. Next: publish fronted.yaml.gz to a neutral-account jsDelivr mirror (server side) and add its URL to the raced set in radiance's NewFronted (currently a single GitHub URL).

Summary by CodeRabbit

• New Features

  • Configuration can now be loaded from multiple URLs.
  • Sources are checked concurrently, with the first valid and applicable configuration applied.
  • Invalid or unusable sources are skipped in favor of other configured sources.

• Bug Fixes

  • Startup now preserves the existing configuration when all configured sources fail.
  • Configuration loading continues reliably when an early source is unavailable or incomplete.

[coderabbitai]

Warning

Review limit reached

@myleshorton, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2edb2ef3-75c7-4457-9da7-98d29869c322

📥 Commits

Reviewing files that changed from the base of the PR and between 5912ce0 and 4dd58e8.

📒 Files selected for processing (3)

• README.md
• config_persist_test.go
• domainfront.go

📝 Walkthrough

Walkthrough

The client now accepts multiple configuration URLs, fetches them concurrently, applies and persists the first usable configuration, skips failed or unapplyable sources, and preserves seeded configuration when all sources fail. Tests cover these startup behaviors.

Changes

Config bootstrap and fallback

Layer / File(s)	Summary
Multi-source configuration contract domainfront.go	Client stores multiple configuration URLs, and WithConfigURL accepts a variadic list.
Concurrent fetch and application domainfront.go, config_persist_test.go	Configuration sources are fetched concurrently; failed and unapplyable results are skipped, the first usable result is applied and persisted, and seeded configuration remains when all sources fail.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ConfigSourceA
  participant ConfigSourceB
  participant ConfigState
  Client->>ConfigSourceA: Fetch configuration
  Client->>ConfigSourceB: Fetch configuration
  ConfigSourceA-->>Client: Failure or unapplyable config
  ConfigSourceB-->>Client: Raw bytes and parsed config
  Client->>ConfigState: Apply and persist winning config
  Client-->>ConfigSourceA: Cancel remaining request

Loading

Possibly related PRs

• getlantern/domainfront#16: Modifies the same persisted configuration bootstrap and fetch/apply pipeline.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: racing multiple config URLs and applying the first valid response.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fisk/race-config-urls

---

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

[Copilot]

Pull request overview

This PR makes the config updater resilient to blocked/slow sources by allowing multiple config URLs to be raced concurrently and applying the first config that successfully fetches, parses, and applies—supporting censored-network scenarios (e.g., GitHub raw blocked in China).

Changes:

• Make WithConfigURL variadic and store multiple config URLs on the client.
• Update fetchAndApplyConfig to concurrently fetch from all URLs and apply the first valid result (with per-source fetch/parse failures treated as non-terminal).
• Add tests covering “race to first valid”, “skip unapplyable source”, and “all sources fail keeps seed”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
domainfront.go	Switches from a single config URL to racing multiple URLs; adds fetchConfigFrom helper and updates updater logic.
config_persist_test.go	Adds new tests to validate multi-URL racing behavior and failure-handling paths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@domainfront.go`:
- Around line 481-482: Bound each individual config fetch initiated by
fetchAndApplyConfig and fetchConfigFrom with a finite request timeout, using
context.WithTimeout or a per-client http.Client timeout. Ensure the timeout
applies before the HTTP request begins, is canceled after completion, and allows
the results loop over c.configURLs to continue when a mirror hangs.
- Around line 486-497: Update the config-loading loop around applyConfig and the
final aggregate warning to track whether any fetched config failed during
application, distinguishing that outcome from sources that failed to fetch or
parse. Keep the per-source warnings unchanged, and make the final c.log.Warn
message accurately report when all available configs were unapplyable rather
than labeling them as fetch failures.

🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a1e4aab-399b-497e-8970-56fc36dc01c5

📥 Commits

Reviewing files that changed from the base of the PR and between 2862f7a and 5912ce0.

📒 Files selected for processing (2)

• config_persist_test.go
• domainfront.go

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

domainfront.go:85

• WithConfigURL(""), which previously disabled the updater (since New checked c.configURL != ""), now sets configURLs to a 1-element slice and starts the updater, causing repeated failed fetch attempts and warnings. Also, storing the caller-provided slice allows accidental later mutation by the caller via WithConfigURL(urls...). Consider filtering out empty strings and copying into a new slice before assigning.

func WithConfigURL(urls ...string) Option { return func(c *Client) { c.configURLs = urls } }

domainfront.go:492

• The PR description says per-source failures are logged at Debug and only total failure is Warn. applyConfig failures are per-source in the raced set, but this is currently logged at Warn, which can be noisy for an expected-bad mirror while racing. Consider downgrading this to Debug to match the stated behavior (total failure is already logged once at Warn).

			// Parseable but unusable (e.g. no providers). Don't give up — a
			// backup mirror exists precisely to cover for a corrupt source, so
			// keep reading the remaining results.
			c.log.Warn("Failed to apply config update", "url", r.url, "error", err)
			continue

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.