feat: speed test and health check for desktop app#1659
feat: speed test and health check for desktop app#1659CelebrityPunks wants to merge 1 commit intoCapSoftware:mainfrom
Conversation
Implements upload speed testing with automatic quality adjustment and a startup health check that verifies server reachability, auth, and upload functionality. Addresses issue CapSoftware#73. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| { | ||
| let state = network_state.read().await; | ||
| if state.is_recording { | ||
| return Err("Cannot run speed test during an active recording".to_string()); | ||
| } | ||
| if matches!(state.speed_test_status, SpeedTestStatus::Running) { | ||
| return Err("Speed test is already running".to_string()); | ||
| } | ||
| } | ||
|
|
||
| { | ||
| let mut state = network_state.write().await; | ||
| state.speed_test_status = SpeedTestStatus::Running; | ||
| } |
There was a problem hiding this comment.
The read() pre-check + later write() update has a race where two callers can both pass the pre-check and run concurrently. Doing check+set under a single write() lock closes that window.
| { | |
| let state = network_state.read().await; | |
| if state.is_recording { | |
| return Err("Cannot run speed test during an active recording".to_string()); | |
| } | |
| if matches!(state.speed_test_status, SpeedTestStatus::Running) { | |
| return Err("Speed test is already running".to_string()); | |
| } | |
| } | |
| { | |
| let mut state = network_state.write().await; | |
| state.speed_test_status = SpeedTestStatus::Running; | |
| } | |
| { | |
| let mut state = network_state.write().await; | |
| if state.is_recording { | |
| return Err("Cannot run speed test during an active recording".to_string()); | |
| } | |
| if matches!(state.speed_test_status, SpeedTestStatus::Running) { | |
| return Err("Speed test is already running".to_string()); | |
| } | |
| state.speed_test_status = SpeedTestStatus::Running; | |
| } |
| import { | ||
| createEffect, | ||
| createSignal, | ||
| onCleanup, | ||
| onMount, | ||
| Show, | ||
| } from "solid-js"; |
There was a problem hiding this comment.
Looks like createEffect is unused here (should trip lint/ts). Easy cleanup.
| import { | |
| createEffect, | |
| createSignal, | |
| onCleanup, | |
| onMount, | |
| Show, | |
| } from "solid-js"; | |
| import { createSignal, onCleanup, onMount, Show } from "solid-js"; |
| if (isSpeedTest || isUploadTest) { | ||
| try { | ||
| const body = await c.req.arrayBuffer(); | ||
| return c.json({ | ||
| ok: true, | ||
| bytesReceived: body.byteLength, | ||
| timestamp: Date.now(), | ||
| }); | ||
| } catch { | ||
| return c.json({ ok: false, message: "Failed to process upload" }, 500); | ||
| } |
There was a problem hiding this comment.
Since this endpoint reads the entire request body into memory, it’d be good to cap payload size (even if callers are expected to send ~1MB) to avoid someone posting an arbitrarily large body.
| if (isSpeedTest || isUploadTest) { | |
| try { | |
| const body = await c.req.arrayBuffer(); | |
| return c.json({ | |
| ok: true, | |
| bytesReceived: body.byteLength, | |
| timestamp: Date.now(), | |
| }); | |
| } catch { | |
| return c.json({ ok: false, message: "Failed to process upload" }, 500); | |
| } | |
| if (isSpeedTest || isUploadTest) { | |
| const maxBytes = 2 * 1024 * 1024; | |
| const contentLength = Number(c.req.header("content-length") ?? "0"); | |
| if (contentLength && contentLength > maxBytes) { | |
| return c.json({ ok: false, message: "Payload too large" }, 413); | |
| } | |
| try { | |
| const body = await c.req.arrayBuffer(); | |
| if (body.byteLength > maxBytes) { | |
| return c.json({ ok: false, message: "Payload too large" }, 413); | |
| } | |
| return c.json({ | |
| ok: true, | |
| bytesReceived: body.byteLength, | |
| timestamp: Date.now(), | |
| }); | |
| } catch { | |
| return c.json({ ok: false, message: "Failed to process upload" }, 500); | |
| } | |
| } |
| #[specta::specta] | ||
| #[instrument(skip(app, state))] | ||
| pub async fn stop_recording(app: AppHandle, state: MutableState<'_, App>) -> Result<(), String> { | ||
| crate::speed_test::set_recording_active(&app, false).await; |
There was a problem hiding this comment.
Minor logic bug: setting is_recording = false at the top of stop_recording opens a window where run_speed_test can run while current_recording.stop().await is still in progress. I’d move the flag flip to after stop() completes (or right before returning) so the speed-test guard matches reality.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let _ = RecordingEvent::Started.emit(&app); | ||
| let _ = RecordingStarted.emit(&app); | ||
|
|
||
| crate::speed_test::set_recording_active(&app, true).await; |
There was a problem hiding this comment.
Recording failure leaves speed test permanently blocked
When a recording fails or crashes (the error path in spawn_actor at line ~1078), set_recording_active(&app, false) is never called. The flag is set to true at line 1064, but only stop_recording (line 1257) resets it to false. If the recording actor errors out via the Err(e) branch, is_recording stays true forever, permanently blocking the speed test with "Cannot run speed test during an active recording".
You should reset the flag in the error handler as well:
Err(e) => {
let mut state = state_mtx.write().await;
crate::speed_test::set_recording_active(&app, false).await;
let _ = RecordingEvent::Failed {
error: e.to_string(),
}
.emit(&app);Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/recording.rs
Line: 1064
Comment:
**Recording failure leaves speed test permanently blocked**
When a recording fails or crashes (the error path in `spawn_actor` at line ~1078), `set_recording_active(&app, false)` is never called. The flag is set to `true` at line 1064, but only `stop_recording` (line 1257) resets it to `false`. If the recording actor errors out via the `Err(e)` branch, `is_recording` stays `true` forever, permanently blocking the speed test with "Cannot run speed test during an active recording".
You should reset the flag in the error handler as well:
```rust
Err(e) => {
let mut state = state_mtx.write().await;
crate::speed_test::set_recording_active(&app, false).await;
let _ = RecordingEvent::Failed {
error: e.to_string(),
}
.emit(&app);
```
How can I resolve this? If you propose a fix, please make it concise.| </Show> | ||
| </div> | ||
| <RecoveryToast /> | ||
| <NetworkStatus /> |
There was a problem hiding this comment.
Incorrect indentation level
<NetworkStatus /> is indented with 2 tabs, but its sibling <RecoveryToast /> above uses 3 tabs. Both are children of the same parent <div>.
| <NetworkStatus /> | |
| <NetworkStatus /> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
Line: 1987
Comment:
**Incorrect indentation level**
`<NetworkStatus />` is indented with 2 tabs, but its sibling `<RecoveryToast />` above uses 3 tabs. Both are children of the same parent `<div>`.
```suggestion
<NetworkStatus />
```
How can I resolve this? If you propose a fix, please make it concise.| app.post("/", withOptionalAuth, async (c) => { | ||
| const isSpeedTest = c.req.header("X-Speed-Test") === "true"; | ||
| const isUploadTest = c.req.header("X-Upload-Test") === "true"; | ||
|
|
||
| if (isSpeedTest || isUploadTest) { | ||
| try { | ||
| const body = await c.req.arrayBuffer(); | ||
| return c.json({ | ||
| ok: true, | ||
| bytesReceived: body.byteLength, | ||
| timestamp: Date.now(), | ||
| }); | ||
| } catch { | ||
| return c.json({ ok: false, message: "Failed to process upload" }, 500); | ||
| } | ||
| } | ||
|
|
||
| return c.json({ ok: true, timestamp: Date.now() }); | ||
| }); |
There was a problem hiding this comment.
Unauthenticated POST endpoint accepts arbitrary payloads
The POST handler uses withOptionalAuth and reads the entire request body into memory with c.req.arrayBuffer() without any size limit or authentication requirement. Since the speed test sends 1MB payloads and there's no rate limiting on this route, this endpoint could be abused to consume server memory/bandwidth. Consider either:
- Requiring authentication (
withAuth) for POST requests, or - Adding a body size limit (e.g., rejecting bodies > 2MB), or
- Adding rate limiting to this endpoint
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/api/desktop/[...route]/health-check.ts
Line: 21-39
Comment:
**Unauthenticated POST endpoint accepts arbitrary payloads**
The POST handler uses `withOptionalAuth` and reads the entire request body into memory with `c.req.arrayBuffer()` without any size limit or authentication requirement. Since the speed test sends 1MB payloads and there's no rate limiting on this route, this endpoint could be abused to consume server memory/bandwidth. Consider either:
- Requiring authentication (`withAuth`) for POST requests, or
- Adding a body size limit (e.g., rejecting bodies > 2MB), or
- Adding rate limiting to this endpoint
How can I resolve this? If you propose a fix, please make it concise.2 participants
Summary
Implements the speed test and health check system requested in #73.
speed_test.rs): Measures upload bandwidth by sending a 1MB test payload to a new/api/desktop/health-checkendpoint. Maps speed results to quality presets (Full/High/Medium/Low) that cap the recording resolution accordingly.NetworkStatus.tsx): Shows health status (OK/Issue) and upload speed in the bottom of the main window. Both are clickable to manually re-run checks. Tooltips provide details.health-check.ts): Lightweight Hono route under/api/desktop/health-checkthat handles GET (health/auth check) and POST (speed test/upload test) requests.Files changed
apps/desktop/src-tauri/src/speed_test.rs— new Rust module with speed test, health check, network state, and Tauri commands/eventsapps/desktop/src-tauri/src/lib.rs— register module, commands, events, managed state, and startup health checkapps/desktop/src-tauri/src/recording.rs— integrate recording-active flag and speed-based resolution cappingapps/desktop/src/components/NetworkStatus.tsx— new SolidJS component for health/speed indicatorsapps/desktop/src/routes/(window-chrome)/new-main/index.tsx— mount NetworkStatus in main windowapps/web/app/api/desktop/[...route]/health-check.ts— new health check API endpointapps/web/app/api/desktop/[...route]/route.ts— register health-check routeTest plan
/api/desktop/health-checkendpoint returns correct responses for GET and POSTCloses #73
🤖 Generated with Claude Code
Greptile Summary
Adds a speed test and health check system for the desktop app. A new Rust module (
speed_test.rs) measures upload bandwidth via a 1MB POST to a new/api/desktop/health-checkHono endpoint, maps results to quality presets that cap instant recording resolution, and runs an automatic health check on startup. A SolidJSNetworkStatuscomponent surfaces health and speed status in the main window.is_recordingis set totruewhen recording starts but is never reset on recording failure/crash (only on explicitstop_recording), which permanently blocks speed tests after a crash.set_recording_active(false)is called at the top ofstop_recordingbefore validating a recording exists or successfully stopping it.<NetworkStatus />in the main window JSX has incorrect indentation.Confidence Score: 2/5
apps/desktop/src-tauri/src/recording.rsneeds the most attention — both the error path inspawn_actorand the ordering instop_recordinghave issues with theis_recordingflag lifecycle.Important Files Changed
label()method is unused (dead code).is_recordingflag is never reset on recording failure/crash, andset_recording_active(false)is called before validating a recording exists instop_recording.NetworkStatususes 2 tabs while siblingRecoveryToastuses 3./api/desktop/health-check. Straightforward addition matching existing patterns.Sequence Diagram
sequenceDiagram participant UI as NetworkStatus UI participant Tauri as Tauri Commands participant State as NetworkState participant API as /api/desktop/health-check Note over Tauri: App startup (3s delay) Tauri->>API: GET /health-check API-->>Tauri: {ok, authenticated} Tauri->>API: GET /health-check (X-Auth-Check) API-->>Tauri: 200 / 401 Tauri->>API: POST /health-check (1KB, X-Upload-Test) API-->>Tauri: {ok, bytesReceived} Tauri->>State: Update health_check_result Tauri->>UI: HealthCheckUpdate event UI->>Tauri: runSpeedTest() Tauri->>State: Check is_recording (block if true) Tauri->>State: Set status = Running Tauri->>UI: SpeedTestUpdate(Running) Tauri->>API: POST /health-check (1MB, X-Speed-Test) API-->>Tauri: {ok, bytesReceived} Tauri->>State: Set status = Completed(result) Tauri->>UI: SpeedTestUpdate(Completed) Note over Tauri: start_recording() Tauri->>State: set_recording_active(true) Note over Tauri: Resolution = min(settings, speed_test) Note over Tauri: stop_recording() Tauri->>State: set_recording_active(false)Comments Outside Diff (1)
apps/desktop/src-tauri/src/recording.rs, line 1257-1264 (link)set_recording_active(false)called before state validationset_recording_active(&app, false)is called unconditionally at the start ofstop_recording, before checking whether a recording is actually in progress (line 1260). Ifstop_recordingis called when no recording exists, this will setis_recording = falseeven though it was nevertrue— which is benign but incorrect. More importantly, ifstop_recordingfails partway through (e.g., at line 1264), the flag is already cleared, so the speed test thinks no recording is active while a partially-stopped recording may still be in progress.Consider moving this call after successfully stopping the recording:
Prompt To Fix With AI
Prompt To Fix All With AI
Last reviewed commit: eb8e7b2
(2/5) Greptile learns from your feedback when you react with thumbs up/down!