Files
jellytau/docs/specs/diagnostics-and-logging.md
T
dtourolle f3fa45f742
🏗️ Build and Test JellyTau / Run Tests (pull_request) Successful in 22m12s
🏗️ Build and Test JellyTau / Supply Chain (pull_request) Successful in 37s
Traceability Validation / Check Requirement Traces (pull_request) Successful in 11s
🏗️ Build and Test JellyTau / Android Compile Check (pull_request) Successful in 4m10s
feat(diagnostics): persistent redacted logging and an exportable bundle
The app forgot everything it did the moment it exited. The Rust half
logged through env_logger to stdout only -- invisible to anyone who
launched from a desktop icon, and on Android worse than that: stdout is
not logcat, so the backend produced no visible output at all on the
platform carrying this project's hardest bugs. The autoplay deadlock,
the truncated-stream restart and the background-audio stall were all
diagnosed by talking a user through `adb logcat`, because there was no
other way to see anything. A panic left nothing behind at all.

Logs now go to a size-capped rotating file, to logcat on Android, and to
the webview console in dev. A panic is recorded with its backtrace before
the process dies. The frontend's messages are forwarded into the same
file, so one timeline holds both halves of the app in order -- which is
what makes a race between them legible after the fact, and races between
them are the expensive bug class here.

Redaction runs in the log FORMATTER, not at export time. A credential
sitting in a file on the device is already a disclosure; stripping it on
the way out would be too late. The exporter redacts a second time to
cover files written by builds that predate this. api_key, X-Emby-Token,
Authorization, "AccessToken" and Token="..." all reduce to [REDACTED],
while host, item ids and filenames are deliberately kept -- a log scrubbed
of those is one nobody can debug anything from. Server URLs keep scheme
and host and drop any embedded user:pass@.

Two things the tests caught that review would not have:

  - redact_headers recursed on its own output. The replacement keeps the
    header NAME, so the next call matched the same header forever; the
    test died with a stack overflow. It is a forward scan now.
  - The frontend forwarder used `void plugin.error(...)`. `void` discards
    a promise's value but not its rejection, so in any webview without
    IPC -- a unit test, SSR, a browser preview -- every log line became an
    unhandled rejection. 20 of them showed up the first time coverage
    ran. Each call now attaches a catch.

Only info and above cross the IPC boundary: debug is per-tick player
state and forwarding it would be thousands of calls a minute for output
nobody reads. A failing forwarder never propagates and never prevents the
console write.

Nothing is transmitted anywhere. The export writes a zip and reports its
path; the user attaches it themselves, which is also what keeps this from
becoming telemetry. An Android share intent is explicitly out of scope --
it is Kotlin work that belongs with the other native code.

The panic hook chains to the previous hook rather than replacing it,
because utils/lock.rs installs a silencing hook around tests that provoke
poisoned locks on purpose.

Spec in docs/specs/diagnostics-and-logging.md; UR-078 / DR-218 / UT-209.

Verified: 1079 frontend tests and the coverage gate, 759 Rust tests,
clippy -D warnings, svelte-check 0 errors, and cargo check for
aarch64-linux-android.
2026-08-21 18:58:57 +02:00

193 lines
8.7 KiB
Markdown

# Spec: Diagnostics and persistent logging
**Status:** Proposed
**Requirements:** UR-078 → DR-218; tests UT-209
**UX spec:** n/a (one Settings section; no new flow)
**Destination on completion:** [09-security.md](../architecture/09-security.md)
for the redaction rules, and a new "Logging and diagnostics" section in
[01-rust-backend.md](../architecture/01-rust-backend.md) for the capture path.
## Summary
JellyTau records what it does, keeps it in a size-capped file on disk, survives a
crash, and can hand the whole thing to the user as one file to attach to a bug
report. Credentials never reach that file.
## Motivation
Today the app forgets everything the moment it exits.
The Rust half logs through `env_logger` to **stdout only**. A user who launched
from a desktop icon has no stdout. On Android it is worse than useless:
`env_logger` writes to stdout, which is not logcat, so **the Rust backend's
output is invisible on the platform where most of the hard bugs have been** — the
autoplay deadlock, the truncated-stream restart, the background-audio stall. The
frontend has a proper leveled facade (`logger.ts`, DR-204) but it only reaches
the webview console, which nobody can read on a phone.
The practical consequence is visible in this project's history: several bugs took
multiple rounds of "can you reproduce it under `adb logcat`" before anyone could
even see what happened. A user reporting "the episode randomly restarted" is
reporting the symptom of a race whose evidence was discarded microseconds later.
There is also no crash record at all. If the app panics, the user sees it vanish
and we learn nothing.
## Layer assignment
| Logic / responsibility | Layer | Why it belongs there |
|---|---|---|
| What is captured, at what level, and where it is written | Rust | Retention and capture policy is backend behaviour; it must work identically whether the UI is open, backgrounded, or gone |
| Log rotation and the size cap | Rust | Storage management, same class as the download and image caches |
| **Redaction of credentials** | Rust | Security-critical, and the values (tokens, `api_key`, keyring payloads) are domain vocabulary owned by the auth layer. A frontend that redacted its own messages would still not cover anything Rust wrote |
| Panic capture and persistence | Rust | Only Rust can install a panic hook |
| Assembling the export (archive + environment summary) | Rust | Touches the filesystem and the app's own paths; also the last point at which redaction can be enforced over everything |
| Which log level is active | Rust owns the *stored* setting and applies it; the frontend renders the picker | Same split as every other setting: the value is state the backend acts on, the control is presentation |
| Showing the export path / opening the folder | Frontend | Pure presentation |
| Formatting a log line for the webview console | Frontend | `logger.ts` already owns this; unchanged |
Borderline: **the frontend forwarding its own messages into the Rust sink.**
Arguably presentation "sending data down". Placed as: the frontend calls a
plugin, and the *decision of what to persist and how to redact it* stays in Rust
— which is the tie-breaker, because a bug report containing a token would be a
security defect regardless of which half wrote the line.
## Design
### Capture
Replace the `env_logger` init in `lib.rs` with `tauri-plugin-log`, which is the
official plugin and already does the three things we would otherwise hand-roll
(CLAUDE.md: prefer official plugins before writing native code):
| Target | Purpose |
|---|---|
| `Stdout` | unchanged behaviour for `bun run tauri dev` |
| `LogDir { file_name: "jellytau" }` | the persistent, rotating file |
| `Webview` (dev only) | Rust lines visible in the webview console while developing |
On Android the plugin routes to **logcat**, which is the single largest
improvement here and needs no code of ours.
Rotation: `RotationStrategy::KeepAll` is wrong for a phone. Use a size cap
(5 MB) with one retained previous file, so a long session cannot fill a device
and yesterday's evidence still exists.
Level: default `Info`, `RUST_LOG` still honoured, and a stored user preference
that survives restart (a user reproducing a bug needs debug logging *across* the
restart that reproduces it).
### Redaction
A pure function in a new `src-tauri/src/utils/diagnostics.rs`:
```rust
pub fn redact(line: &str) -> String
```
It replaces the value in each of these with `[REDACTED]`, case-insensitively:
- `api_key=…` and `ApiKey=…` in URLs and query strings
- `X-Emby-Token: …`, `X-MediaBrowser-Token: …`, `Authorization: …` headers
- `"AccessToken":"…"` in JSON bodies
- `MediaBrowser Token="…"` in the Emby auth header form
What it deliberately does **not** remove: the server host, item ids, and
filenames. Those are what make a log useful, they are not secrets, and stripping
them would produce a diagnostic bundle nobody can diagnose anything from.
Applied at two points: on every line the export copies, and — because the export
is not the only way a file leaves a device — inside the log formatter itself, so
the token never reaches disk in the first place. The export-time pass exists to
cover files written before an upgrade.
### Export
```rust
#[tauri::command]
pub async fn diagnostics_export(app: AppHandle) -> Result<DiagnosticsBundle, String>
```
Writes a single `.zip` and returns where it went:
```rust
#[derive(Serialize, Type)]
#[serde(rename_all = "camelCase")]
pub struct DiagnosticsBundle {
pub path: String,
pub size_bytes: u64,
pub file_count: usize,
}
```
Contents: the current and previous log files (redacted), plus `environment.txt`
— app version, OS and arch, whether the build is debug, the active log level, and
the *scheme and host* of the configured server. No token, no username, no path
inside the user's home beyond the app's own directories.
### Frontend
`logger.ts` keeps its `console.*` pass-through untouched — live object references
in devtools are a stated design goal of DR-204 — and *additionally* forwards a
stringified copy at `info` and above to the plugin, so one timeline contains both
halves of the app. Forwarding is fire-and-forget and never throws into a caller:
a logging failure must not become an application failure.
A `Diagnostics` section in Settings shows the log location, a level picker, and
an **Export diagnostics** button that reports the resulting path and, on desktop,
offers to reveal it.
## Out of scope
- **An Android share sheet.** Export writes to the app's files directory and
reports the path; wiring a native `ACTION_SEND` intent is a Kotlin change that
belongs with the other native work, not here.
- **Uploading anywhere.** Nothing is transmitted. The user attaches the file
themselves, which is also what keeps this from becoming telemetry.
- **Frontend `debug` forwarding.** Only `info`+ crosses the IPC boundary; per-tick
player debug would be thousands of calls a minute.
## Acceptance criteria
- [ ] Rust logs reach a rotating file on Linux and **logcat** on Android.
- [ ] A panic is recorded and is present in the next export.
- [ ] Frontend `info`/`warn`/`error` appear in the same file as Rust's lines.
- [ ] An export containing a request URL with `api_key=` shows `[REDACTED]`, and
a test greps the produced bundle for the token to prove it.
- [ ] The log file cannot exceed the cap.
- [ ] `bun run check`, `bun run test`, `cargo fmt`, `cargo clippy -D warnings`,
`bun run test:rust`, `bun run check:boundary` all pass.
- [ ] `bindings.ts` regenerated (new command and struct).
- [ ] New code carries `TRACES:` comments.
## Testing
**Rust** (`cargo test`): `redact` over each credential shape, including one
already-redacted line (idempotent) and a line containing no secret (unchanged);
that the environment summary contains a host but no token; that rotation respects
the cap.
**Frontend** (`vitest`): that the forwarder is called for `info`+ and not for
`debug`; that a rejected forward does not propagate to the caller.
## TRACES
| Piece | Tag |
|---|---|
| `utils/diagnostics.rs` | `UR-078 \| DR-218` |
| `commands/diagnostics.rs` | `UR-078 \| DR-218` |
| logging init in `lib.rs` | `UR-078 \| DR-218` |
| `logger.ts` forwarding | `UR-078 \| DR-204, DR-218` |
| Settings section | `UR-078 \| DR-218` |
| tests | `\| DR-218 \| UT-209` |
## Notes for the implementer
- A parallel Claude session may be active — `git diff` before "repairing"
anything unexpected.
- `utils/lock.rs` already sets and restores a panic hook in its tests. The
diagnostics hook must chain to the previous hook rather than replace it, or
those tests start reporting panics they deliberately suppress.
- Do not call the exporter from an event callback that can re-enter the player:
it does blocking file I/O (see the deadlock note in CLAUDE.md).