fix(web-radio): try the round-robin host, and retry, before failing - #455
Merged
Conversation
Reported in #452: on Windows, the first category click after reopening WaveFlow often failed with a server error, and simply leaving the Web Radio page and coming back made it work. The plugin pinned a single host, `de1.api.radio-browser.info`, with no fallback and no retry — so any hiccup on the first HTTP call of a session surfaced straight to the user, and the "leave and come back" workaround was just a second attempt. The pin rested on the comment's assumption that "any regional prefix serves the same catalogue, so the chosen one doesn't matter". Checked against reality: of the prefixes that comment names, `at1`, `nl1` and `fi1` no longer resolve at all. A hard-coded node list rots on its own. So the first host is now `all.api.radio-browser.info`, the project's official round-robin DNS entry, which resolves only to nodes that are alive — no list to maintain. `de1` follows as a second attempt, both because it does answer and because having a second entry means a failed first request is retried at all, which is what the report comes down to. A 4xx short-circuits: that's the API rejecting the request itself, and every node would say the same. Both hosts are already covered by the manifest allowlist (`https://*.api.radio-browser.info/**`), so permissions are unchanged. Plugin bumped to 0.1.1 (the version rides in the User-Agent radio-browser asks for) and plugin.wasm rebuilt in both places it lives: the bundled resource and the core test fixture. Refs #452
InstaZDLL
added
scope: backend
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLe plugin Web Radio remplace son miroir unique par plusieurs hosts de secours et construit des chemins d’API relatifs. Le runtime conserve les causes des erreurs réseau avec une profondeur limitée. Les manifests et la version Cargo passent de 0.1.0 à 0.1.1. ChangesRésilience des requêtes réseau
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Resolve
participant FetchJson
participant Hosts
participant RadioAPI
Resolve->>FetchJson: transmettre le chemin relatif
FetchJson->>Hosts: parcourir les hosts configurés
FetchJson->>RadioAPI: envoyer GET host + path
RadioAPI-->>FetchJson: retourner le statut et le corps
FetchJson-->>Resolve: retourner le corps sur réponse 2xx
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/plugins/web-radio/src/lib.rs (1)
148-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSuffixe de requête dupliqué trois fois.
limit={PAGE_LIMIT}&order=votes&reverse=true&hidebroken=trueest répété à l'identique pourtag,countryetsearch. Extraire une constante partagée limiterait le risque de divergence si un paramètre change un jour.♻️ Exemple de factorisation
+const SORT_SUFFIX: &str = "order=votes&reverse=true&hidebroken=true"; + fn build_path(query: &str) -> Result<String, String> { ... return Ok(format!( - "/json/stations/bytag/{tag}?limit={PAGE_LIMIT}&order=votes&reverse=true&hidebroken=true" + "/json/stations/bytag/{tag}?limit={PAGE_LIMIT}&{SORT_SUFFIX}" ));🤖 Prompt for 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. In `@src-tauri/plugins/web-radio/src/lib.rs` around lines 148 - 185, Extract the repeated query suffix used by the tag, country, and search branches into one shared constant near the request-building logic, then interpolate that constant in each endpoint format. Preserve the existing parameter values and endpoint-specific path/query keys while ensuring all three branches reuse the same suffix.
🤖 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 `@src-tauri/plugins/web-radio/src/lib.rs`:
- Around line 188-236: Update the status handling in fetch_json so fail-fast
applies only to 4xx responses other than 429; let 429 continue through the retry
path like 5xx responses. Preserve successful 2xx handling and retry behavior for
transport errors and other statuses.
---
Outside diff comments:
In `@src-tauri/plugins/web-radio/src/lib.rs`:
- Around line 148-185: Extract the repeated query suffix used by the tag,
country, and search branches into one shared constant near the request-building
logic, then interpolate that constant in each endpoint format. Preserve the
existing parameter values and endpoint-specific path/query keys while ensuring
all three branches reuse the same suffix.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 201f764a-8045-4944-b40b-4a8fec359357
⛔ Files ignored due to path filters (3)
src-tauri/crates/core/tests/fixtures/web-radio/plugin.wasmis excluded by!**/*.wasmsrc-tauri/plugins/web-radio/Cargo.lockis excluded by!**/*.locksrc-tauri/resources/plugins/web-radio/plugin.wasmis excluded by!**/*.wasm
📒 Files selected for processing (5)
src-tauri/crates/core/tests/fixtures/web-radio/manifest.tomlsrc-tauri/plugins/web-radio/Cargo.tomlsrc-tauri/plugins/web-radio/manifest.tomlsrc-tauri/plugins/web-radio/src/lib.rssrc-tauri/resources/plugins/web-radio/manifest.toml
The report on #452 came with this, and nothing more: plugin web-radio: plugin: http: error sending request for url (https://de1.api.radio-browser.info/json/stations/bycountrycodeexact/IL?…) That is `reqwest::Error`'s Display, which stops at the outermost layer. It says the transport failed but never says why — DNS lookup, connection refused, timeout and TLS handshake all render identically, and the frontend shows this string verbatim. So the one question worth answering ("which of those was it?") is exactly the one the message drops, and the next report would have been just as blind. Walk `source()` and append each distinct cause. Layers that restate their parent are skipped (reqwest and hyper both do this, and "timed out: timed out" reads worse than the line it repeats), and the walk is depth-capped because `source()` is author-controlled. Refs #452
There was a problem hiding this comment.
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 `@src-tauri/crates/core/src/plugin/host_impl.rs`:
- Around line 347-370: Update describe_error so deduplication compares each
cause message only with the immediately preceding error-layer message, not the
accumulated out string. Track the last individual message in a separate
variable, update it for every traversed cause, and append the current cause
unless it exactly repeats that parent message; preserve the existing depth limit
and output formatting.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c275ea89-3835-46f2-9c07-653d9ed3d6d3
📒 Files selected for processing (1)
src-tauri/crates/core/src/plugin/host_impl.rs
Review catch on #455. `resp.status < 500` fails fast on 429 Too Many Requests, but a rate limit is per-node state rather than a verdict on the request — routing around a node that won't serve us right now is exactly what the second host is for. The same condition also swallowed 3xx (the host client disables redirects, so those arrive verbatim), which are likewise not a rejection of the request. Narrow the fail-fast to genuine 4xx minus 429; everything else moves on to the next host. Refs #452
`order=votes&reverse=true&hidebroken=true` was spelled out identically in the tag, country and search branches. The three category kinds have to rank the same way, so a future tweak would have had to be remembered in three places. `topvote` / `lastchange` deliberately stay out of it: they carry their ordering in the endpoint name and take the limit as a path segment. Outside-diff review note on #455. Refs #452
Owner
Author
|
@coderabbitai La remarque hors-diff sur le suffixe dupliqué m'avait échappé — traitée en 9c2a55f. Extrait en
La vraie raison de factoriser ici n'est pas la répétition en soi : les trois catégories doivent classer de façon identique, donc une modification future (un autre
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline --decorate --no-renames 9c2a55f
git show --format=fuller --no-ext-diff --unified=40 9c2a55f -- src-tauri/plugins/web-radio/src/lib.rs
ast-grep outline src-tauri/plugins/web-radio/src/lib.rs --items all --view expanded
rg -n -C 3 'LIST_PARAMS|hidebroken=true|order=|topvote|lastchange' src-tauri/plugins/web-radio/src/lib.rs
for wasm in \
src-tauri/resources/plugins/web-radio/plugin.wasm \
src-tauri/crates/core/tests/fixtures/web-radio/plugin.wasm
do
if [ -f "$wasm" ]; then
printf '\n== %s ==\n' "$wasm"
strings "$wasm" | rg -n 'hidebroken=true|order=|topvote|lastchange' || true
else
printf '\nMISSING: %s\n' "$wasm"
fi
doneLength of output: 15214 🏁 Script executed: #!/bin/bash
set -euo pipefail
needle='order=votes&reverse=true&hidebroken=true'
for wasm in \
src-tauri/resources/plugins/web-radio/plugin.wasm \
src-tauri/crates/core/tests/fixtures/web-radio/plugin.wasm
do
printf '\n== %s ==\n' "$wasm"
count=$(LC_ALL=C grep -aFo "$needle" "$wasm" | wc -l | tr -d ' ')
printf 'shared-suffix occurrences: %s\n' "$count"
LC_ALL=C grep -aobF "$needle" "$wasm" || true
doneLength of output: 832
Les deux |
Review catch on #455. `out.ends_with(&text)` asked "does this cause end the string built so far", not "does it repeat the layer above it". A distinct cause that happens to be a suffix of the accumulated text was therefore dropped: "failed to connect" followed by its own "connect" layer collapsed to just the first, losing information the whole function exists to preserve. Track the last appended message separately and require an exact match. The new test fails against the previous logic, which is the point. Refs #452
|
Note Docstrings generation - SKIPPED |
coderabbitai Bot
added a commit
that referenced
this pull request
Jul 29, 2026
Docstrings generation was requested by @InstaZDLL. * #455 (comment) The following files were modified: * `src-tauri/crates/core/src/plugin/host_impl.rs` * `src-tauri/plugins/web-radio/src/lib.rs`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #452 — @jo-el414 reported that on Windows the first category click after reopening WaveFlow often failed with a server error, and that leaving the Web Radio page and coming back made it work.
Cause
The plugin pinned a single host with no fallback and no retry:
Any hiccup on the first HTTP call of a session went straight to the user, and the "leave and come back" workaround was simply a second attempt. (
list_entriesis a static list, so the failure can only come from the first real request — which matches the report exactly.)What I checked before writing the fix
The pin rested on the comment's own claim that "radio-browser uses round-robin DNS across regional mirrors (
de1,at1,us1, …); the chosen prefix doesn't matter". That is no longer true:de1.api.radio-browser.infonl1/at1/fi1all.api.radio-browser.infoSo the obvious fix — a hard-coded list of named nodes — would have rotted exactly like the current comment did.
Fix
all.api.radio-browser.infofirst: it's the project's official round-robin DNS entry, resolving only to nodes that are alive, so a node going down needs no edit here.de1follows as a second attempt — both because it does answer, and because having a second entry is what makes a failed first request get retried at all, which is the whole user-visible bug.A 4xx short-circuits instead of retrying: that's the API rejecting the request itself (bad tag, malformed query) and every node would answer the same.
Both hosts are already covered by the manifest allowlist (
https://*.api.radio-browser.info/**) — no permission change.Verification
cargo check --target wasm32-wasip2andcargo clippy -- -D warningson the plugin — cleanplugin_web_radiointegration test passes against the rebuilt component (it really instantiates it through the wasmtime host)strings plugin.wasmshows both hosts andWaveFlow/Web-Radio/0.1.1Plugin bumped to 0.1.1 (the version rides in the User-Agent radio-browser asks for) and
plugin.wasmrebuilt in both places it lives — the bundled resource and the core test fixture.Follow-up in this PR: the error message itself
@jo-el414 sent the screenshot after this PR was opened, and it confirms the diagnosis — the failing call is
de1, and the wording is a transport failure, so no HTTP response ever came back:But it also exposes a gap: that's
reqwest::Error's Display, which stops at the outermost layer. DNS lookup, connection refused, timeout and TLS handshake all render identically, and the frontend shows the string verbatim — so the one question worth answering is the one the message drops, and the next report would have been just as blind.Second commit walks
source()and appends each distinct cause, skipping layers that merely restate their parent (reqwest and hyper both do that) and capping the depth, sincesource()is author-controlled. Four unit tests cover it: root cause preserved, lone error, restating layer, pathological chain.Two things to know
.wasmdiff is larger than this change. It was rebuilt with a current toolchain (142 210 → 130 644 bytes), so the binary delta mixes the code change with a compiler-version change. The source diff is the reviewable part.Summary by CodeRabbit