fix(backend): auto-restart supervisor + client transport-retry (#567/#570/#571) (#572)
The "Can't reach the local OmniVoice backend" cluster was a long-standing supervision gap (dates to v0.3.0/#38), not a v0.3.7 regression: the backend was spawned once and never watched again — `spawn_backend_and_wait` returned the instant it was healthy. When the uvicorn process then died mid-session (a CUDA OOM/context fault under a burst of generations — #571's log shows the startup banner replaying 6× during a 20-generate burst — an antivirus kill, any crash), nothing restarted it, so every later request threw connection-refused and the user was stuck on the toast until a full app restart. Two layers, both default-mode and platform-neutral: 1. Backend auto-restart supervisor (bootstrap.rs). After Ready, the bootstrap thread (which used to just return) keeps watching the child and respawns it on a *confirmed process exit* (try_wait — never a slow health probe, so a busy-but-alive backend is never killed). Bounded to 5 restarts/60s (then Failed) so a deterministic startup crash can't fork-bomb; the #314 broken-venv self-heal stays the venv-failure path. Strictly gated on AppFlags.quitting so it never resurrects the backend during shutdown. A single-supervisor guard (compare_exchange) prevents duplicate loops when Retry re-enters concurrently. Emits backend-restarting/backend-restored events (the splash poll stops post-Ready, so the stage alone can't show it). 2. Client transport-retry (client.ts). A *thrown* fetch (the backend briefly down while it respawns) is retried a bounded few times with backoff (~2.9s total) before surfacing the actionable ApiError, making the restart window invisible. HTTP errors and deliberate aborts are never retried. Resolves the whole cluster regardless of the crash trigger. Tests: Rust backoff-policy unit test (cap + window-pruning); 4 client-retry vitest cases (retry-then-succeed, no-retry-on-HTTP-error, no-retry-on-abort, bounded give-up). Also corrects a stale Cargo.lock omnivoice-studio version (0.3.6→0.3.8). Co-authored-by: mergetest <test@local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
mergetest
Claude Opus 4.8
parent
b0c93a598e
commit
e14644f77a
Generated
+1
-1
@@ -2941,7 +2941,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "omnivoice-studio"
|
||||
version = "0.3.6"
|
||||
version = "0.3.8"
|
||||
dependencies = [
|
||||
"arboard",
|
||||
"dirs-next",
|
||||
|
||||
@@ -4,15 +4,16 @@ use std::fs;
|
||||
use std::io::{self, BufRead, BufReader};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::{Command, Stdio};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use serde::Serialize;
|
||||
use tauri::{Emitter, Manager};
|
||||
|
||||
use crate::config::get_effective_region;
|
||||
use crate::tools::resolve_uv;
|
||||
use crate::{BackendState, backend_port};
|
||||
use crate::{AppFlags, BackendState, backend_port};
|
||||
|
||||
// ── Bootstrap stages ──────────────────────────────────────────────────────
|
||||
|
||||
@@ -179,6 +180,18 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stage_handle: &Arc<Mutex<B
|
||||
while start.elapsed() < Duration::from_secs(300) {
|
||||
if crate::backend::backend_healthy(backend_port()) {
|
||||
set_stage(stage_handle, BootstrapStage::Ready);
|
||||
// #567/#570/#571: once Ready, keep watching the backend child
|
||||
// and respawn it if it dies mid-session, so a crash self-heals
|
||||
// instead of leaving every later request to dead-end on
|
||||
// "Can't reach the local backend". Only one supervisor runs at
|
||||
// a time — Retry can re-enter this function concurrently.
|
||||
if SUPERVISOR_ACTIVE
|
||||
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
|
||||
.is_ok()
|
||||
{
|
||||
supervise_backend(app, stage_handle);
|
||||
SUPERVISOR_ACTIVE.store(false, Ordering::SeqCst);
|
||||
}
|
||||
return;
|
||||
}
|
||||
let process_dead = if let Ok(mut guard) = app.state::<BackendState>().process.lock() {
|
||||
@@ -244,6 +257,131 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stage_handle: &Arc<Mutex<B
|
||||
}
|
||||
}
|
||||
|
||||
// ── Backend supervisor (auto-restart) ─────────────────────────────────────
|
||||
//
|
||||
// #567/#570/#571: the backend used to be spawned once and never watched again
|
||||
// (`spawn_backend_and_wait` returned the instant it was healthy). When the
|
||||
// uvicorn process then died mid-session — a CUDA OOM/context fault under a
|
||||
// burst of generations, an antivirus kill, any crash — nothing restarted it,
|
||||
// so every later request threw connection-refused and the user was stuck on
|
||||
// the "Can't reach the local backend" toast until they restarted the whole
|
||||
// app. The supervisor closes that gap: after Ready, it watches the child and
|
||||
// respawns it (bounded) so a crash self-heals.
|
||||
|
||||
/// Only one supervisor loop may run at a time. The launch-time bootstrap and
|
||||
/// the Retry button both call `spawn_backend_and_wait` (and can race), so the
|
||||
/// first to reach Ready claims this and the rest fall through.
|
||||
static SUPERVISOR_ACTIVE: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
/// Give up (surface Failed) if the backend dies this many times within
|
||||
/// `RESTART_WINDOW` — a deterministic startup crash must not become a
|
||||
/// fork-bomb. The #314 broken-venv self-heal stays the venv-failure path; the
|
||||
/// supervisor only handles post-Ready deaths.
|
||||
const MAX_RESTARTS: usize = 5;
|
||||
const RESTART_WINDOW: Duration = Duration::from_secs(60);
|
||||
|
||||
fn app_is_quitting(app: &tauri::AppHandle) -> bool {
|
||||
app.try_state::<AppFlags>()
|
||||
.map(|f| f.quitting.load(Ordering::SeqCst))
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Returns `Some(exit description)` if the tracked backend child has exited,
|
||||
/// `None` if it is still running (or none is tracked — which we never treat as
|
||||
/// a death to respawn, to avoid fighting a deliberate teardown).
|
||||
fn backend_child_exit(app: &tauri::AppHandle) -> Option<String> {
|
||||
let state = app.try_state::<BackendState>()?;
|
||||
let mut guard = state.process.lock().ok()?;
|
||||
match guard.as_mut() {
|
||||
Some(child) => match child.try_wait() {
|
||||
Ok(Some(status)) => Some(status.to_string()),
|
||||
Ok(None) => None,
|
||||
Err(e) => Some(format!("try_wait error: {e}")),
|
||||
},
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop restart timestamps older than `RESTART_WINDOW` and report whether the
|
||||
/// remaining count has hit the cap. Pure so the backoff policy is unit-tested
|
||||
/// without spawning real processes.
|
||||
fn restart_budget_exhausted(times: &mut Vec<Instant>, now: Instant) -> bool {
|
||||
times.retain(|t| now.duration_since(*t) < RESTART_WINDOW);
|
||||
times.len() >= MAX_RESTARTS
|
||||
}
|
||||
|
||||
/// After the backend is Ready, watch its process and respawn it on an
|
||||
/// unexpected exit. Runs on the (otherwise-returning) bootstrap thread and
|
||||
/// stops the instant the app is quitting so it never resurrects the backend
|
||||
/// during shutdown. Death is detected only via a *confirmed process exit*
|
||||
/// (`try_wait`), never a slow health probe, so a busy-but-alive backend is
|
||||
/// never killed.
|
||||
fn supervise_backend(app: &tauri::AppHandle, stage_handle: &Arc<Mutex<BootstrapStage>>) {
|
||||
let mut restart_times: Vec<Instant> = Vec::new();
|
||||
loop {
|
||||
std::thread::sleep(Duration::from_secs(2));
|
||||
if app_is_quitting(app) {
|
||||
return;
|
||||
}
|
||||
let exit_info = match backend_child_exit(app) {
|
||||
Some(info) => info,
|
||||
None => continue, // still running
|
||||
};
|
||||
// The exit may have raced with a shutdown that killed the child.
|
||||
if app_is_quitting(app) {
|
||||
return;
|
||||
}
|
||||
if restart_budget_exhausted(&mut restart_times, Instant::now()) {
|
||||
let tail = crate::backend::read_error_log_tail(30);
|
||||
let msg = format!(
|
||||
"The backend kept crashing ({} times in {}s) and couldn't be kept running. \
|
||||
Use Clean & Retry, or check Settings → Logs → Backend.{}",
|
||||
MAX_RESTARTS,
|
||||
RESTART_WINDOW.as_secs(),
|
||||
if tail.is_empty() { String::new() } else { format!("\n\nLast output:\n{tail}") },
|
||||
);
|
||||
log::error!("Backend supervisor giving up: {msg}");
|
||||
let _ = app.emit("backend-restart-failed", msg.clone());
|
||||
set_stage(stage_handle, BootstrapStage::Failed { message: msg });
|
||||
return;
|
||||
}
|
||||
restart_times.push(Instant::now());
|
||||
log::warn!("Backend process exited unexpectedly ({exit_info}) — restarting it (#567)");
|
||||
emit_log(app, "starting_backend", "Backend stopped unexpectedly — restarting it automatically");
|
||||
// Frontend listens for this to show a "reconnecting" banner (the splash
|
||||
// poll has already stopped post-Ready, so the stage alone won't show).
|
||||
let _ = app.emit("backend-restarting", exit_info.clone());
|
||||
set_stage(stage_handle, BootstrapStage::StartingBackend);
|
||||
// Clear any orphan still holding the port before the respawn.
|
||||
if crate::backend::port_in_use(backend_port()) {
|
||||
crate::backend::kill_orphan_on_port(backend_port());
|
||||
std::thread::sleep(Duration::from_millis(300));
|
||||
}
|
||||
let child = crate::backend::spawn_backend(app, Some(stage_handle));
|
||||
if let Ok(mut guard) = app.state::<BackendState>().process.lock() {
|
||||
*guard = child;
|
||||
}
|
||||
// Wait (bounded) for the respawn to become healthy. If it dies again
|
||||
// immediately, bail early so the next loop counts it toward the cap.
|
||||
let start = Instant::now();
|
||||
while start.elapsed() < Duration::from_secs(120) {
|
||||
if app_is_quitting(app) {
|
||||
return;
|
||||
}
|
||||
if crate::backend::backend_healthy(backend_port()) {
|
||||
set_stage(stage_handle, BootstrapStage::Ready);
|
||||
let _ = app.emit("backend-restored", ());
|
||||
log::info!("Backend restarted and healthy again");
|
||||
break;
|
||||
}
|
||||
if backend_child_exit(app).is_some() {
|
||||
break;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(500));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
pub fn clean_and_retry_bootstrap(app: tauri::AppHandle, state: tauri::State<'_, BootstrapState>) {
|
||||
// env_root honors the setup-screen choice (portable / custom env dir), so
|
||||
@@ -1052,6 +1190,36 @@ mod tests {
|
||||
assert_eq!(envs.get("UV_HTTP_RETRIES").map(String::as_str), Some("5"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_budget_caps_respawns_and_prunes_old_ones() {
|
||||
// Supervisor backoff policy (#567): fewer than MAX_RESTARTS deaths
|
||||
// inside the window keeps restarting; hitting the cap gives up.
|
||||
let t0 = Instant::now();
|
||||
let mut times: Vec<Instant> = (0..MAX_RESTARTS - 1).map(|_| t0).collect();
|
||||
assert!(
|
||||
!restart_budget_exhausted(&mut times, t0),
|
||||
"{} deaths in-window is under the cap",
|
||||
MAX_RESTARTS - 1
|
||||
);
|
||||
times.push(t0);
|
||||
assert!(
|
||||
restart_budget_exhausted(&mut times, t0),
|
||||
"{} deaths in-window must trip the cap",
|
||||
MAX_RESTARTS
|
||||
);
|
||||
|
||||
// Restarts older than the window are pruned and never count toward the
|
||||
// cap, so an app left running for hours never crash-loops on stale
|
||||
// history. (Forward Instant arithmetic — always representable.)
|
||||
let later = t0 + RESTART_WINDOW + Duration::from_secs(1);
|
||||
let mut aged: Vec<Instant> = (0..MAX_RESTARTS).map(|_| t0).collect();
|
||||
assert!(
|
||||
!restart_budget_exhausted(&mut aged, later),
|
||||
"deaths older than the window must be pruned, not counted"
|
||||
);
|
||||
assert!(aged.is_empty(), "stale timestamps should have been dropped");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rocm_reinstall_args_target_the_rocm_index() {
|
||||
let args = rocm_torch_reinstall_args(ROCM_TORCH_INDEX);
|
||||
|
||||
+42
-22
@@ -99,6 +99,12 @@ async function readError(res: Response): Promise<string> {
|
||||
}
|
||||
}
|
||||
|
||||
// Backoff (ms) for retrying a *transport-level* failure — the backend briefly
|
||||
// down while the auto-restart supervisor brings it back (#567/#570/#571). One
|
||||
// short cascade (~2.9 s total) so a restart window becomes invisible, yet a
|
||||
// genuinely-down backend still surfaces the actionable error promptly.
|
||||
const TRANSPORT_RETRY_BACKOFF_MS = [400, 900, 1600];
|
||||
|
||||
export async function apiFetch(path: string, opts: RequestInit = {}): Promise<Response> {
|
||||
const pin = typeof sessionStorage !== 'undefined' ? sessionStorage.getItem('ov_pin') : null;
|
||||
const key = _apiKey();
|
||||
@@ -111,30 +117,44 @@ export async function apiFetch(path: string, opts: RequestInit = {}): Promise<Re
|
||||
const finalOpts: RequestInit = Object.keys(extra).length
|
||||
? { ...opts, headers: { ...(opts.headers as Record<string, string> || {}), ...extra } }
|
||||
: opts;
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fetch(apiUrl(path), finalOpts);
|
||||
} catch (e) {
|
||||
// A thrown fetch (TypeError "Failed to fetch" / "NetworkError") means the
|
||||
// request never reached the backend — it's still starting up, crashed, or
|
||||
// the dev server dropped. Surface that as an actionable ApiError instead of
|
||||
// the raw browser string (issues #438/#454/#466). status:0 lets callers
|
||||
// distinguish a transport failure from an HTTP error.
|
||||
throw new ApiError(
|
||||
"Can't reach the local OmniVoice backend — it may still be starting up, or it stopped. " +
|
||||
"Wait a few seconds and try again; if it persists, restart the app (or check Settings → Logs → Backend).",
|
||||
{ status: 0, detail: String((e as Error)?.message || e) },
|
||||
);
|
||||
}
|
||||
if (!res.ok) {
|
||||
// 401 from the LAN PIN middleware on a remote device → surface the gate.
|
||||
if (res.status === 401 && typeof window !== 'undefined') {
|
||||
window.dispatchEvent(new Event('ov:pin-required'));
|
||||
const signal = finalOpts.signal as AbortSignal | null | undefined;
|
||||
let lastDetail = '';
|
||||
for (let attempt = 0; ; attempt++) {
|
||||
if (signal?.aborted) throw new DOMException('Aborted', 'AbortError');
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fetch(apiUrl(path), finalOpts);
|
||||
} catch (e) {
|
||||
// A thrown fetch (TypeError "Failed to fetch" / "NetworkError") means the
|
||||
// request never reached the backend — it's still starting up, crashed, or
|
||||
// the dev server dropped. The auto-restart supervisor revives it within a
|
||||
// few seconds, so retry a bounded few times with backoff before surfacing
|
||||
// the actionable ApiError, making a brief restart window invisible
|
||||
// (issues #438/#454/#466/#567). Never retry a deliberate abort. status:0
|
||||
// lets callers distinguish a transport failure from an HTTP error.
|
||||
if (signal?.aborted || (e as Error)?.name === 'AbortError') throw e;
|
||||
lastDetail = String((e as Error)?.message || e);
|
||||
if (attempt < TRANSPORT_RETRY_BACKOFF_MS.length) {
|
||||
await new Promise((r) => setTimeout(r, TRANSPORT_RETRY_BACKOFF_MS[attempt]));
|
||||
continue;
|
||||
}
|
||||
throw new ApiError(
|
||||
"Can't reach the local OmniVoice backend — it may still be starting up, or it stopped. " +
|
||||
"Wait a few seconds and try again; if it persists, restart the app (or check Settings → Logs → Backend).",
|
||||
{ status: 0, detail: lastDetail },
|
||||
);
|
||||
}
|
||||
const detail = await readError(res);
|
||||
throw new ApiError(`${res.status} ${res.statusText}: ${detail}`, { status: res.status, detail });
|
||||
if (!res.ok) {
|
||||
// 401 from the LAN PIN middleware on a remote device → surface the gate.
|
||||
// An HTTP error means the backend *did* respond — never retry it.
|
||||
if (res.status === 401 && typeof window !== 'undefined') {
|
||||
window.dispatchEvent(new Event('ov:pin-required'));
|
||||
}
|
||||
const detail = await readError(res);
|
||||
throw new ApiError(`${res.status} ${res.statusText}: ${detail}`, { status: res.status, detail });
|
||||
}
|
||||
return res;
|
||||
}
|
||||
return res;
|
||||
}
|
||||
|
||||
export async function apiJson<T = unknown>(path: string, opts: RequestInit = {}): Promise<T> {
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { apiFetch, ApiError } from '../api/client';
|
||||
|
||||
// #567/#570/#571: when the backend dies mid-session, the auto-restart
|
||||
// supervisor revives it within a few seconds. apiFetch must ride out that
|
||||
// brief window by retrying a *transport* failure (a thrown fetch) a bounded
|
||||
// few times — but never retry an HTTP error (the backend responded) or a
|
||||
// deliberate abort, and still surface the actionable error if it stays down.
|
||||
describe('apiFetch transport-retry', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it('retries a transient transport failure, then succeeds', async () => {
|
||||
vi.useFakeTimers();
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new TypeError('Failed to fetch'))
|
||||
.mockResolvedValueOnce(new Response('ok', { status: 200 }));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const p = apiFetch('/health');
|
||||
await vi.advanceTimersByTimeAsync(500); // first backoff is 400ms
|
||||
const res = await p;
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('does NOT retry an HTTP error response', async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue(new Response('nope', { status: 500, statusText: 'Server Error' }));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
await expect(apiFetch('/x')).rejects.toBeInstanceOf(ApiError);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does NOT call fetch once the signal is already aborted', async () => {
|
||||
const fetchMock = vi.fn().mockRejectedValue(new TypeError('Failed to fetch'));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const ac = new AbortController();
|
||||
ac.abort();
|
||||
|
||||
await expect(apiFetch('/x', { signal: ac.signal })).rejects.toMatchObject({
|
||||
name: 'AbortError',
|
||||
});
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('gives up after the bounded retries with a status:0 ApiError', async () => {
|
||||
vi.useFakeTimers();
|
||||
const fetchMock = vi.fn().mockRejectedValue(new TypeError('Failed to fetch'));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const p = apiFetch('/x');
|
||||
const assertion = expect(p).rejects.toMatchObject({ status: 0 });
|
||||
await vi.advanceTimersByTimeAsync(400 + 900 + 1600 + 100);
|
||||
await assertion;
|
||||
// initial attempt + 3 bounded retries
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user