# Network Resilience Review — Theseus

> **Scope**: All network-facing code in theseus — axum HTTP/WebSocket server, reqwest HTTP clients (DeepSeek API, internal route helpers, CLI), and browser-side fetch/WebSocket.
>
> **Date**: 2025-07-21 | **Reviewer**: pi-roles assistant

---

## Summary

| # | Criterion | Score | Verdict |
|---|-----------|-------|---------|
| 1 | Reconnection mechanism | **C** | Fixed-delay reconnect, no backoff |
| 2 | Timeout handling | **F** | Zero timeouts configured anywhere |
| 3 | Proxy resilience | **D** | No explicit handling; relies on reqwest defaults |
| 4 | Persistent connection health | **F** | No ping/pong or heartbeat on WebSocket |
| 5 | Network error classification | **D** | All errors flattened to 500; no transient/permanent distinction |
| 6 | Graceful degradation | **C** | Toast on error, but no offline mode or queued mutations |
| 7 | Connection pool management | **D** | `Client::new()` per-call in `deepseek_chat()` and 9 route handlers |

**Overall: D+** — The app works on a reliable local network but will degrade badly under flaky connectivity, proxy interference, or DeepSeek API instability.

---

## 1. Reconnection Mechanism — Score: C

### Evidence

**Client-side** (`static/js/ws.js:30-35`):
```js
ws.onclose = () => {
    wsClient = null;
    dbg('ws: closed, reconnect in 2s');
    if (getApiKey()) setTimeout(connectWs, 2000);
};
ws.onerror = () => ws.close();
```

**Server-side** (`src/routes/ws.rs:19-36`): No reconnect logic — the server handles incoming connections only. This is correct; reconnect responsibility is client-side.

### Analysis

- Fixed 2-second reconnect delay — no exponential backoff, no jitter.
- No maximum retry limit — will retry indefinitely (good) but at fixed intervals (bad for transient outages affecting many clients).
- No distinction between clean close (`1000`) and abnormal close — every close triggers reconnect.
- Deduplication guard at `ws.js:5-7` prevents multiple concurrent connections (good).

### Recommendation

1. Implement exponential backoff with jitter: `min(baseDelay * 2^attempt + random(0, 1000), maxDelay)`.
2. Cap at ~30s max delay; reset attempt counter on successful message receipt.
3. Respect close codes — don't reconnect on `1000` (normal closure) or `1001` (going away).

---

## 2. Timeout Handling — Score: F

### Evidence

**`build_http_client()`** (`src/main.rs:699-707`):
```rust
fn build_http_client(api_key: Option<&str>) -> Result<reqwest::Client, BoxErr> {
    let mut headers = HeaderMap::new();
    // ... header setup ...
    Ok(reqwest::Client::builder()
        .default_headers(headers)
        .build()?)  // ← No .timeout(), .connect_timeout(), .read_timeout()
}
```

**`deepseek_chat()`** (`src/lib.rs:563`):
```rust
let client = reqwest::Client::new();  // ← Default reqwest client, no timeouts
```

**`LlmClient::new()`** (`src/agent/llm.rs:15-22`):
```rust
pub fn new(api_key: String) -> Self {
    Self {
        client: Client::new(),  // ← No timeouts
        // ...
    }
}
```

**Frontend** (`static/js/api.js:28-32`):
```js
function authFetch(url, options) {
    const opts = options ? { ...options } : {};
    opts.headers = authHeaders(opts.headers);
    return fetch(url, opts);  // ← No AbortController, no timeout
}
```

**Search result**: `ffgrep('timeout|Timeout|TIMEOUT')` against `src/` returned **zero matches**.

### Analysis

- `reqwest::Client` defaults: no connect timeout (OS TCP timeout, typically 60-120s), no read timeout (waits forever), no request timeout.
- A hung DeepSeek API call will block the request handler indefinitely.
- The CLI will hang on a non-responsive server until the OS gives up (~2 minutes).
- Browser `fetch()` has no timeout — user sees a spinner forever.

### Recommendation

1. **CLI client** (`build_http_client`): `.connect_timeout(Duration::from_secs(5))`, `.timeout(Duration::from_secs(30))`.
2. **DeepSeek calls** (`deepseek_chat` and `LlmClient`): `.timeout(Duration::from_secs(60))` — AI calls can be slow but shouldn't hang forever.
3. **Frontend**: Wrap `authFetch` with `AbortController` + `setTimeout` (e.g., 30s default).
4. **Server bind**: Add `tcp_keepalive` on the axum listener for idle connections.

---

## 3. Proxy Resilience — Score: D

### Evidence

No proxy-related code exists in the project source (`ffgrep('proxy|PROXY|Proxy|socks|SOCKS|http_proxy')` returned only vendored library hits in `cytoscape.min.js`, `dagre.min.js`, `elk.bundled.js`).

`reqwest` respects system proxy environment variables (`HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`, `ALL_PROXY`) by default via its `hyper-util` proxy connector. This is implicit — not tested or documented.

### Analysis

- If a proxy is configured system-wide, reqwest will use it transparently — but there's no fallback if the proxy is down.
- No `NO_PROXY` configuration for the local server connection (CLI → `127.0.0.1:3080`). If `HTTP_PROXY` is set, CLI traffic to the local server may route through the proxy unnecessarily.
- DeepSeek API calls through a corporate proxy will silently fail if the proxy requires authentication — no helpful error message.
- The pentest use case often involves network pivoting — proxy awareness is critical.

### Recommendation

1. For CLI → local server: explicitly bypass proxy via `reqwest::Client::builder().no_proxy()`.
2. Document proxy environment variables in README/config.
3. For DeepSeek calls: log the resolved proxy (if any) at startup for debugging.
4. Consider adding a `theseus.toml` config option for explicit proxy URL.

---

## 4. Persistent Connection Health — Score: F

### Evidence

**Server-side** (`src/routes/ws.rs:19-36`):
```rust
async fn handle_socket(mut socket: WebSocket, state: AppState) {
    let mut rx = state.tx.subscribe();
    loop {
        tokio::select! {
            msg = rx.recv() => {
                if let Ok(data) = msg {
                    if socket.send(Message::Text(data.into())).await.is_err() { break; }
                }
            }
            incoming = socket.recv() => {
                match incoming {
                    Some(Ok(Message::Close(_))) | None => break,
                    _ => {}
                }
            }
        }
    }
}
```

**Search result**: `ffgrep('keep.?alive|keepalive|ping|pong|heartbeat')` returned **zero matches** in project source.

### Analysis

- No `Message::Ping` sent by either server or client.
- No heartbeat interval — if the TCP connection silently dies (NAT timeout, WiFi handoff, VPN reconnect), neither side detects it until the next actual message.
- In a pentest environment with VPN tunnels and network pivoting, silent connection death is common.
- `tokio::select!` will wait indefinitely on both branches — a dead socket will linger until the OS TCP keepalive kicks in (default ~2 hours on Linux).

### Recommendation

1. **Server**: Add a ping interval (e.g., every 30s):
   ```rust
   // In handle_socket, add a third select branch:
   _ = tokio::time::sleep(Duration::from_secs(30)) => {
       if socket.send(Message::Ping(vec![])).await.is_err() { break; }
   }
   ```
2. **Client**: Handle `onping` or use a `setInterval` heartbeat check; close and reconnect if no message received within ~60s.
3. Configure TCP keepalive on the axum `TcpListener` (e.g., `socket2::Socket::set_keepalive`).

---

## 5. Network Error Classification — Score: D

### Evidence

**`deepseek_chat()`** (`src/lib.rs:570-584`):
```rust
.map_err(|e| {
    tracing::error!("DeepSeek API request failed: {}", e);
    StatusCode::INTERNAL_SERVER_ERROR  // ← All errors → 500
})?;
// ...
401 => StatusCode::INTERNAL_SERVER_ERROR,
402 => StatusCode::INTERNAL_SERVER_ERROR,
429 => StatusCode::INTERNAL_SERVER_ERROR,  // ← Rate limit = same as auth failure
_ => StatusCode::INTERNAL_SERVER_ERROR,
```

**`LlmError`** (`src/agent/llm.rs:168-173`):
```rust
pub enum LlmError {
    RequestFailed(String),      // Network-level
    ApiError { status: u16, message: String },  // HTTP-level
    ParseError(String),         // Response-level
}
```

### Analysis

- `deepseek_chat()` treats all errors identically — a transient `429` (rate limit) and a permanent `401` (bad key) get the same treatment. No retry for rate limits.
- `LlmError` has three variants but no `is_retryable()` method or transient/permanent classification.
- The `RequestFailed` variant captures network errors (DNS failure, connection refused, timeout) as a flat string — no structured error type to match on.
- Browser-side `apiFetch()` (`api.js:72-86`) throws on any non-200 or `!j.success` — no retry, no offline detection.

### Recommendation

1. Classify errors in `deepseek_chat()`:
   - `429` → retry with backoff (respect `Retry-After` header).
   - `401`/`402` → fail immediately, surface clear message to user.
   - Network errors → retry once with backoff.
2. Add `is_retryable()` to `LlmError` for the agent loop to decide retry behavior.
3. Frontend: detect `fetch` `TypeError` (network failure) vs HTTP error status; show "offline" indicator for network failures.

---

## 6. Graceful Degradation — Score: C

### Evidence

**Frontend error handling** (`static/js/api.js:72-86`):
```js
async function apiFetch(url, options) {
    try {
        const res = await authFetch(url, options);
        const j = await res.json();
        if (res.status === 401) {
            showAuthOverlay(j.error || 'API Key 无效或缺失');
            throw new Error(j.error || 'API Key 无效或缺失');
        }
        if (!j.success) throw new Error(j.error || '请求失败');
        return j.data;
    } catch (e) {
        toast('错误: ' + e.message);
        throw e;
    }
}
```

**WebSocket reconnect** (`ws.js:30-35`): reconnects on close, but no "offline" state shown to user.

**Various module error handling**: Most catch blocks toast the error and rethrow (`nodes.js:81`, `evidence.js:52`, `import.js:35,47,79`, `graph.js:613`).

### Analysis

- **Partial**: Errors surface as toasts (good for awareness) but the UI doesn't enter a degraded/offline mode.
- **No mutation queue**: If the WebSocket is down, the user can still create nodes/edges via the REST API, but won't see real-time updates. There's no "reconnecting" banner or queued mutation replay.
- **No stale data indication**: If the graph hasn't been updated due to a missed WebSocket message, the user has no visual cue.
- **Server-side**: Routes that call DeepSeek (`analysis`, `report`, `chat`) return 500 on API failure — the user gets a generic error. No fallback to "AI unavailable, showing raw data" mode.
- **Auth overlay**: Good UX for 401 errors — prompts for API key.

### Recommendation

1. Add a "connection lost" banner when WebSocket is disconnected, with a spinner during reconnect attempts.
2. For DeepSeek-dependent features, implement a fallback: "AI service unavailable — showing raw analysis" instead of a 500 error.
3. Consider queuing failed mutations in the frontend for replay on reconnect.

---

## 7. Connection Pool Management — Score: D

### Evidence

**`deepseek_chat()`** (`src/lib.rs:563`):
```rust
let client = reqwest::Client::new();  // ← New client (and connection pool) per call
```

**Route handlers creating per-request clients**:
- `src/routes/analysis.rs:542` — `reqwest::Client::new()`
- `src/routes/credentials.rs:141` — `reqwest::Client::new()`
- `src/routes/edges.rs:312` — `reqwest::Client::new()`
- `src/routes/evidence.rs:280` — `reqwest::Client::new()`
- `src/routes/nodes.rs:558` — `reqwest::Client::new()`
- `src/routes/projects.rs:171` — `reqwest::Client::new()`
- `src/routes/report.rs:253` — `reqwest::Client::new()`
- `src/routes/search.rs:88` — `reqwest::Client::new()`
- `src/routes/timeline.rs:100` — `reqwest::Client::new()`

**`LlmClient`** (`src/agent/llm.rs:7,17`):
```rust
pub struct LlmClient {
    client: Client,  // ← Reused across turns (good)
    // ...
}
impl LlmClient {
    pub fn new(api_key: String) -> Self {
        Self { client: Client::new(), ... }
    }
}
```

**CLI** (`src/main.rs:704-706`): One client per command invocation — acceptable for CLI usage.

### Analysis

- `reqwest::Client::new()` creates a new connection pool each time. In `deepseek_chat()` (called by analysis, report, chat routes), this means a new TLS handshake for every AI call.
- The 9 route handlers that create `Client::new()` appear to be for internal CLI-to-server calls (used in `build_http_client` patterns in the CLI path), but when called as server-side route handlers, they create unnecessary client instances.
- `LlmClient` correctly reuses its client across agent turns.
- `reqwest::Client` is `Clone` and uses `Arc` internally — sharing a single client across the application is the intended usage pattern.

### Recommendation

1. **Add a shared `reqwest::Client` to `AppState`**:
   ```rust
   pub(crate) struct AppState {
       pub(crate) http_client: reqwest::Client,  // ← Shared, pooled
       // ... existing fields ...
   }
   ```
2. Replace all `reqwest::Client::new()` calls in route handlers with `state.http_client.clone()`.
3. Use the same shared client in `deepseek_chat()` — pass it as a parameter instead of creating a new one.
4. This enables TCP connection reuse, TLS session resumption, and centralized timeout configuration.

---

## Priority Action Items

| Priority | Action | Impact | Effort |
|----------|--------|--------|--------|
| **P0** | Add timeouts to all `reqwest::Client` instances | Prevents indefinite hangs | Low |
| **P0** | Add WebSocket ping/pong heartbeat | Detects dead connections | Low |
| **P1** | Exponential backoff on WebSocket reconnect | Reduces server load during outages | Low |
| **P1** | Shared `reqwest::Client` in `AppState` | Eliminates per-request TLS handshakes | Medium |
| **P1** | Classify DeepSeek errors as transient/permanent | Enables retry for 429/network errors | Low |
| **P2** | Frontend connection status indicator | Improves UX during network issues | Medium |
| **P2** | Bypass proxy for CLI → localhost | Prevents misrouted local traffic | Low |
| **P3** | Fallback mode for AI-unavailable features | Degrades gracefully when DeepSeek is down | Medium |
