fix(docker): runtime API-base override (Settings 'Failed to load engines: Failed to fetch') - #174
Conversation
…e backend Docker users reported Settings -> Engines failing with 'Failed to load engines: Failed to fetch'. Two problems: (1) the two API-base resolvers diverged (client.ts honored VITE_API_URL; apiBase.ts honored VITE_OMNIVOICE_API), and the docs documented VITE_OMNIVOICE_API -- which the Engines request path ignored; (2) VITE_* is inlined at BUILD time, so a prebuilt ghcr.io image has no working runtime override at all for reverse-proxy / split-origin deploys. - backend: when OMNIVOICE_PUBLIC_API_BASE is set, inject it into index.html as window.__OMNIVOICE_API_BASE__ (core/spa_inject.py; validated to a plain http(s) URL so it can't break out of the <script>). Unset (default) -> StaticFiles serves index.html untouched (same-origin, zero overhead). - frontend: both resolvers (client.ts _resolveApiBase + utils/apiBase.ts) now read the runtime global FIRST, then VITE_OMNIVOICE_API/VITE_API_URL, then fall through to same-origin. client.ts also strips trailing slashes and recognises __TAURI_INTERNALS__ (parity with apiBase.ts/external.ts). - docs: docker.md + troubleshooting.md document OMNIVOICE_PUBLIC_API_BASE as the runtime override that works on the prebuilt image (the old VITE_OMNIVOICE_API docker run -e example never worked -- build-time inlining). - tests: spa_inject helpers (inject + URL validation/breakout); resolver precedence for the runtime global + VITE_OMNIVOICE_API in both test files. Default same-origin behavior is unchanged on every platform; override is opt-in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a runtime API base override mechanism for SPA deployments. A new backend module validates and injects a ChangesRuntime API Base Override
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
|
| Filename | Overview |
|---|---|
| backend/core/spa_inject.py | New pure-function module: URL validation via regex and HTML <head> injection of the runtime global; JSON-encodes the value to prevent script breakout. Well-tested. |
| backend/main.py | Registers GET / and GET /index.html to serve the injected index.html, but SPA deep-link navigations to any other path (e.g. /settings) still fall through to StaticFiles and receive the un-injected file. Also reads from disk on every request instead of caching at startup. |
| frontend/src/api/client.ts | Unified API-base resolver: runtime global → VITE_OMNIVOICE_API → VITE_API_URL → same-origin, with trailing-slash stripping and __TAURI_INTERNALS__ parity. Clean. |
| frontend/src/utils/apiBase.ts | Added runtime global check (priority 0) ahead of the existing env/Tauri/origin chain; consistent with client.ts precedence order. |
| tests/test_spa_inject.py | Good unit coverage: valid/invalid URL cases, <head> injection, no-head fallback, and JSON encoding. All pure-function tests with no app bootstrapping required. |
| frontend/src/api/client.apibase.test.ts | New test cases cover runtime global precedence (beats Tauri + VITE_API_URL), VITE_OMNIVOICE_API alias, and TAURI_INTERNALS loopback. |
| frontend/src/utils/apiBase.test.ts | Adds runtime global test; afterEach properly cleans up __OMNIVOICE_API_BASE__ to prevent test pollution. |
| docs/install/docker.md | Docs updated to replace VITE_OMNIVOICE_API with OMNIVOICE_PUBLIC_API_BASE; clearly explains runtime vs build-time distinction. |
| docs/install/troubleshooting.md | Updated troubleshooting guide to reference the new runtime override variable. |
Sequence Diagram
sequenceDiagram
participant Op as Operator
participant D as Docker / Env
participant B as FastAPI Backend
participant S as StaticFiles Mount
participant Br as Browser
Op->>D: "OMNIVOICE_PUBLIC_API_BASE=https://api.host"
D->>B: startup env var read
B->>B: is_valid_public_api_base() → true
B->>B: register GET / and GET /index.html routes
Br->>B: GET /
B->>B: read index.html + inject_api_base()
B-->>Br: index.html with window.__OMNIVOICE_API_BASE__
Note over Br: module-level API constant resolved to https://api.host ✓
Br->>B: GET /settings (page refresh / deep link)
B->>S: no matching route → StaticFiles handles
S-->>Br: raw index.html (no injection ⚠)
Note over Br: window.__OMNIVOICE_API_BASE__ undefined, API falls back to same-origin ✗
Reviews (1): Last reviewed commit: "fix(docker): runtime API-base override s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @app.get("/", include_in_schema=False) | ||
| def _index_root(): | ||
| return _index_with_api_base() | ||
|
|
||
| @app.get("/index.html", include_in_schema=False) | ||
| def _index_html(): | ||
| return _index_with_api_base() | ||
|
|
||
| app.mount("/", StaticFiles(directory=frontend_path, html=True), name="frontend") |
There was a problem hiding this comment.
Only GET / and GET /index.html are intercepted by the injected routes; every other path (e.g. a user refreshing at /settings or /engines, or opening a bookmarked URL) falls through to the StaticFiles mount, which serves the raw index.html without window.__OMNIVOICE_API_BASE__. Because client.ts evaluates API as a module-level constant at import time, those page loads will resolve the API base as same-origin (or fall back to loopback), silently ignoring OMNIVOICE_PUBLIC_API_BASE. The fix is to add a catch-all route (or a middleware) before the StaticFiles mount that serves the injected HTML for all non-asset paths — the same pattern used by most single-page-app servers.
| if _public_api_base and os.path.isfile(_index_path): | ||
| from fastapi.responses import HTMLResponse | ||
|
|
||
| def _index_with_api_base() -> "HTMLResponse": | ||
| with open(_index_path, "r", encoding="utf-8") as _fh: | ||
| return HTMLResponse(inject_api_base(_fh.read(), _public_api_base)) |
There was a problem hiding this comment.
_public_api_base is resolved once at startup (it never changes at runtime), so the injected HTML will always be identical. Reading and injecting on every / and /index.html request wastes disk I/O unnecessarily. Cache the result at startup instead.
| if _public_api_base and os.path.isfile(_index_path): | |
| from fastapi.responses import HTMLResponse | |
| def _index_with_api_base() -> "HTMLResponse": | |
| with open(_index_path, "r", encoding="utf-8") as _fh: | |
| return HTMLResponse(inject_api_base(_fh.read(), _public_api_base)) | |
| if _public_api_base and os.path.isfile(_index_path): | |
| from fastapi.responses import HTMLResponse | |
| with open(_index_path, "r", encoding="utf-8") as _fh: | |
| _injected_html = inject_api_base(_fh.read(), _public_api_base) | |
| def _index_with_api_base() -> "HTMLResponse": | |
| return HTMLResponse(_injected_html) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/install/docker.md (2)
82-86: 💤 Low valueRemove blank line between blockquote paragraphs.
Markdown linting flags the blank line at line 86 inside the blockquote block. Removing it improves formatting consistency.
📝 Suggested change
> `OMNIVOICE_PUBLIC_API_BASE` must be a plain `http(s)://…` URL; anything else > is ignored and the app falls back to same-origin. If you build from source you > may instead bake `VITE_OMNIVOICE_API` at build time, but the runtime var above > is simpler and image-agnostic. - > **Security:** OmniVoice ships no authentication. Anything on your LAN with🤖 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 `@docs/install/docker.md` around lines 82 - 86, Remove the extra blank line inside the blockquote in docs/install/docker.md between the two sentences mentioning `OMNIVOICE_PUBLIC_API_BASE` and `VITE_OMNIVOICE_API` so the two lines form a single continuous blockquote paragraph (i.e., delete the empty line after the sentence referencing `OMNIVOICE_PUBLIC_API_BASE`), keeping the same wording and markdown blockquote markers.
68-68: 💤 Low valueConsider American English variant for consistency.
The word "afterwards" is more common in British English. For American English documentation, "afterward" is preferred.
📝 Suggested change
-from, so opening the UI from `http://<lan-ip>:3900` Just Works for both the -page load *and* the API/media requests it makes afterwards. +from, so opening the UI from `http://<lan-ip>:3900` Just Works for both the +page load *and* the API/media requests it makes afterward.🤖 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 `@docs/install/docker.md` at line 68, Replace the British English adverb "afterwards" in the sentence containing "page load *and* the API/media requests it makes afterwards." with the American English variant "afterward" so the sentence reads "...the API/media requests it makes afterward." to maintain consistency in the docs; update the single occurrence of the string "afterwards" accordingly.backend/main.py (1)
625-656: Runtime override looks correct; note the CORS prerequisite for split-origin.The explicit
/and/index.htmlhandlers are registered before theStaticFilesmount at/, so Starlette matches them first — injection wins for the shell while assets still fall through to the mount. Logic is sound.One deployment caveat worth surfacing in the docs/log: when
OMNIVOICE_PUBLIC_API_BASEpoints at a different origin than where the SPA is served (the reverse-proxy / split-origin case this PR targets), the browser will issue cross-origin requests to that base. Unless that SPA origin is added toOMNIVOICE_ALLOWED_ORIGINS(default is localhost only, Lines 554-557), the calls will be blocked by CORS and the user still sees "Failed to fetch" — just from a different cause. Worth calling out alongside theOMNIVOICE_PUBLIC_API_BASEinstructions so operators set both together.🤖 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 `@backend/main.py` around lines 625 - 656, When OMNIVOICE_PUBLIC_API_BASE is set to a different origin than the SPA host, cross-origin requests can be blocked unless that SPA origin is present in OMNIVOICE_ALLOWED_ORIGINS; detect this at startup and log a clear warning. After computing _public_api_base (and validating via is_valid_public_api_base), parse its origin and check the OMNIVOICE_ALLOWED_ORIGINS environment value (comma-separated) for that SPA origin; if not present, emit a warning via logging.getLogger("omnivoice.api").warning naming OMNIVOICE_PUBLIC_API_BASE and advising operators to add the SPA origin to OMNIVOICE_ALLOWED_ORIGINS. Ensure you reference the existing symbols _public_api_base, is_valid_public_api_base, and OMNIVOICE_ALLOWED_ORIGINS so the change sits next to the current injection logic.
🤖 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.
Nitpick comments:
In `@backend/main.py`:
- Around line 625-656: When OMNIVOICE_PUBLIC_API_BASE is set to a different
origin than the SPA host, cross-origin requests can be blocked unless that SPA
origin is present in OMNIVOICE_ALLOWED_ORIGINS; detect this at startup and log a
clear warning. After computing _public_api_base (and validating via
is_valid_public_api_base), parse its origin and check the
OMNIVOICE_ALLOWED_ORIGINS environment value (comma-separated) for that SPA
origin; if not present, emit a warning via
logging.getLogger("omnivoice.api").warning naming OMNIVOICE_PUBLIC_API_BASE and
advising operators to add the SPA origin to OMNIVOICE_ALLOWED_ORIGINS. Ensure
you reference the existing symbols _public_api_base, is_valid_public_api_base,
and OMNIVOICE_ALLOWED_ORIGINS so the change sits next to the current injection
logic.
In `@docs/install/docker.md`:
- Around line 82-86: Remove the extra blank line inside the blockquote in
docs/install/docker.md between the two sentences mentioning
`OMNIVOICE_PUBLIC_API_BASE` and `VITE_OMNIVOICE_API` so the two lines form a
single continuous blockquote paragraph (i.e., delete the empty line after the
sentence referencing `OMNIVOICE_PUBLIC_API_BASE`), keeping the same wording and
markdown blockquote markers.
- Line 68: Replace the British English adverb "afterwards" in the sentence
containing "page load *and* the API/media requests it makes afterwards." with
the American English variant "afterward" so the sentence reads "...the API/media
requests it makes afterward." to maintain consistency in the docs; update the
single occurrence of the string "afterwards" accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cfe258c6-065d-4b6f-9210-f36da2ec1ed2
📒 Files selected for processing (9)
backend/core/spa_inject.pybackend/main.pydocs/install/docker.mddocs/install/troubleshooting.mdfrontend/src/api/client.apibase.test.tsfrontend/src/api/client.tsfrontend/src/utils/apiBase.test.tsfrontend/src/utils/apiBase.tstests/test_spa_inject.py
…ode-fallback log Two coupled Windows fixes for the preview/blob audio path (the "playBlobAudio decode error: EncodingError: Unable to decode audio data" users see in Logs → Frontend on Windows). 1. apiBase 127.0.0.1, not localhost (Tauri context). The backend binds IPv4 127.0.0.1 only; on Windows "localhost" often resolves to ::1 (IPv6) first, so requests miss the backend. The main client (api/client.ts) already did this since #174, but utils/apiBase.ts lagged on "localhost" — and its one consumer is utils/media.js's preview upload, the #653 fallback. So #653's streamed fallback fetched http://localhost:3900/preview/upload and FAILED on Windows, leaving preview playback broken even after #653. Align the two resolvers. 2. Quieter, accurate logging in playBlobAudio. The Web Audio decodeAudioData path is EXPECTED to fail for long-form / AAC renders on WebView2 and is recovered by the streamed fallback — yet it logged at error level, so users saw a red "decode error" even when playback succeeded. Downgrade that branch to console.warn ("falling back to streamed playback"); reserve error level for the real failure (both decode AND fallback failed). With fix #1 the fallback now actually reaches the backend on Windows, so the recovery completes. Tests: apiBase.test.ts asserts Tauri → http://127.0.0.1:3900; the existing playBlobAudioFallback.test.js (#653) still passes (fetch hits /preview/upload, plays the HTTP URL, never a blob:). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ode-fallback log (#659) Two coupled Windows fixes for the preview/blob audio path (the "playBlobAudio decode error: EncodingError: Unable to decode audio data" users see in Logs → Frontend on Windows). 1. apiBase 127.0.0.1, not localhost (Tauri context). The backend binds IPv4 127.0.0.1 only; on Windows "localhost" often resolves to ::1 (IPv6) first, so requests miss the backend. The main client (api/client.ts) already did this since #174, but utils/apiBase.ts lagged on "localhost" — and its one consumer is utils/media.js's preview upload, the #653 fallback. So #653's streamed fallback fetched http://localhost:3900/preview/upload and FAILED on Windows, leaving preview playback broken even after #653. Align the two resolvers. 2. Quieter, accurate logging in playBlobAudio. The Web Audio decodeAudioData path is EXPECTED to fail for long-form / AAC renders on WebView2 and is recovered by the streamed fallback — yet it logged at error level, so users saw a red "decode error" even when playback succeeded. Downgrade that branch to console.warn ("falling back to streamed playback"); reserve error level for the real failure (both decode AND fallback failed). With fix #1 the fallback now actually reaches the backend on Windows, so the recovery completes. Tests: apiBase.test.ts asserts Tauri → http://127.0.0.1:3900; the existing playBlobAudioFallback.test.js (#653) still passes (fetch hits /preview/upload, plays the HTTP URL, never a blob:). Co-authored-by: mergetest <test@local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Bug
Docker users open the web UI, go to Settings → Engines, and get "Failed to load engines: Failed to fetch" (a browser
TypeError— the request never reached a server). Works in the desktop build.Root cause
client.ts(what the Engines tab uses) honoredVITE_API_URL;utils/apiBase.ts(media) honoredVITE_OMNIVOICE_API. The docs told users to setVITE_OMNIVOICE_API— which the Engines path ignored.VITE_*is inlined at build time, so the prebuiltghcr.ioimage can't take an override fromdocker run -e— leaving reverse-proxy / split-origin deployments with no knob at all. (The standard single-containerlocalhost:3900case already works via same-origin; the failure is non-same-origin topologies.)Fix
core/spa_inject.py+main.py): whenOMNIVOICE_PUBLIC_API_BASEis set, inject it intoindex.htmlaswindow.__OMNIVOICE_API_BASE__. The value is validated to a plainhttp(s)://URL (no quotes/angle brackets) so it can't break out of the<script>. Unset (default) → StaticFiles servesindex.htmluntouched — same-origin, zero overhead, no behavior change.client.ts+utils/apiBase.ts): both resolvers now read the runtime global first, thenVITE_OMNIVOICE_API/VITE_API_URL, then fall through to same-origin.client.tsalso strips trailing slashes and recognizes__TAURI_INTERNALS__(parity with the other resolvers).docker.md+troubleshooting.mdnow documentOMNIVOICE_PUBLIC_API_BASE— the runtime override that actually works on the prebuilt image.Verify
pytest tests/test_spa_inject.py(5) ✓, router smoke (23) ✓.VITE_OMNIVOICE_APIhonored; trailing-slash strip;__TAURI_INTERNALS__), full vitest 107/107, build ✓.docker run -e OMNIVOICE_PUBLIC_API_BASE=https://api.host …→ Engines tab loads (request targets the configured origin). Standardlocalhost:3900unchanged.Default same-origin behavior identical on macOS/Windows/Linux; override is opt-in.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
OMNIVOICE_PUBLIC_API_BASEenvironment variable, allowing dynamic API endpoint setup for Docker deployments without rebuilding the application.Documentation