Skip to content

Commit e181036

Browse files
committed
Share short ID generator across error/crash reports
- Extract `error_reporter::generate_short_id` into a shared `short_id` module that takes a prefix string, so the upcoming `CRASH-XXXXX` flow can reuse the same alphabet, length, and rejection-sampling routine. - Keep `error_reporter::generate_short_id` as a thin wrapper to avoid touching call sites.
1 parent 762d7b9 commit e181036

4 files changed

Lines changed: 87 additions & 26 deletions

File tree

apps/desktop/src-tauri/src/error_reporter/CLAUDE.md

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,11 @@ logs/<next-filename> # ...
2424

2525
Manifest fields (`BundleManifest`):
2626

27-
- `id`: short ID (`ERR-XXXXX`) generated client-side. **Server may regenerate** — UI
28-
always shows the server's response ID, never the local one.
27+
- `id`: short ID (`ERR-XXXXX`) generated client-side via [`crate::short_id::generate`]
28+
(kept as a thin wrapper in `error_reporter::generate_short_id`). The api server
29+
validates the shape and uses this id as-is — the trailing UUID in the R2 key
30+
guarantees object uniqueness, so there's no server-side regeneration. The UI shows
31+
the same id everywhere (dialog preview, toast, server response).
2932
- `kind`: `"user"` or `"auto"` (Phase 5).
3033
- `buildMode`: `"release"` or `"debug"`. Resolved at compile time from
3134
`cfg!(debug_assertions)` via `BuildMode::current()`. Forwarded to the api server so the
@@ -77,7 +80,7 @@ SMB URIs, and UNC paths. See the redact module for the full pattern table.
7780

7881
| File | Purpose |
7982
| -------------------------- | ------------------------------------------------------------------------------------------------------------- |
80-
| `mod.rs` | `build_bundle` (dispatches by scope), `build_bundle_streaming` (Flow A streaming pipeline), `CountingCursor`, `build_zip`, `generate_short_id`, `upload`, `cap_bundle_to_mb`, `save_bundle_to_disk` (debug); also exports the `log_error!` macro |
83+
| `mod.rs` | `build_bundle` (dispatches by scope), `build_bundle_streaming` (Flow A streaming pipeline), `CountingCursor`, `build_zip`, `generate_short_id` (thin wrapper over `crate::short_id::generate("ERR")`), `upload`, `cap_bundle_to_mb`, `save_bundle_to_disk` (debug); also exports the `log_error!` macro |
8184
| `tail_walker.rs` | Reads a log file from the END backward in 64 KB chunks, yields lines newest-first, stops at the timestamp cutoff. Handles long lines that span multiple chunks, lines without leading timestamps (panic continuations), and CRLF defensively. |
8285
| `tests.rs` | Unit tests: zip structure, redaction, ID format/uniqueness, capping, streaming pipeline |
8386
| `auto_dispatcher.rs` | Flow B: opt-in auto-send on user-visible errors (60 s ± 10 s debounce, 1 MB tail, no retry on failure) |
@@ -324,8 +327,10 @@ because breadcrumbs are best-effort instrumentation, not a feature.
324327
- Per-entry mtimes are set explicitly (manifest = `now`, logs = source-file mtime).
325328
Without this, the `zip` crate's `SimpleFileOptions::default()` writes 1980-01-01 for
326329
every entry — extracted bundles look like ancient archives.
327-
- Server-side ID generation may differ from the client's local ID. Always trust the
328-
`id` field in the upload response — that's the one the user reports back to us.
330+
- The server uses the client-supplied `id` verbatim — the upload response echoes it.
331+
Earlier versions regenerated server-side; that was removed because the trailing UUID
332+
in the R2 key already guarantees uniqueness, and it was confusing to show one id in
333+
the preview dialog and a different one in the toast.
329334
- The line-timestamp filter (Flow A's tail walker AND Flow B's per-line filter) relies
330335
on the file chain's ISO-8601 stamp format
331336
(`YYYY-MM-DDTHH:MM:SS.mmm±HH:MM`, see `logging::dispatch::file_timestamp`). Lines

apps/desktop/src-tauri/src/error_reporter/mod.rs

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,8 @@ macro_rules! log_error {
9696
}};
9797
}
9898

99-
/// Same unambiguous alphabet the server uses for license short codes and error report IDs.
100-
/// Kept in sync with `apps/api-server/src/license.ts` — avoids `0`/`O`, `1`/`I`/`L`.
101-
const SHORT_ID_ALPHABET: &[u8] = b"23456789ABCDEFGHJKMNPQRSTUVWXYZ";
102-
const SHORT_ID_LEN: usize = 5;
99+
/// Short ID prefix for error reports. The alphabet, length, and generation routine
100+
/// live in [`crate::short_id`] so the crash reporter can reuse them.
103101
const SHORT_ID_PREFIX: &str = "ERR";
104102

105103
/// Max lines in the first-lines preview sample shown in the dialog.
@@ -834,24 +832,11 @@ fn build_zip(
834832
Ok(buf)
835833
}
836834

837-
/// Generate a short ID like `ERR-8F3A2` using rejection sampling (no modulo bias).
835+
/// Generate a short ID like `ERR-8F3A2`. Thin wrapper around [`crate::short_id::generate`]
836+
/// kept for the existing public surface; new code should call `crate::short_id::generate`
837+
/// directly with the prefix it wants.
838838
pub fn generate_short_id() -> String {
839-
let mut rng = rand::rng();
840-
let alphabet_len = SHORT_ID_ALPHABET.len(); // 31
841-
// 256 - (256 % 31) = 232 — bytes at or above this would skew the distribution.
842-
let max_unbiased = 256 - (256 % alphabet_len);
843-
let mut out = String::with_capacity(SHORT_ID_PREFIX.len() + 1 + SHORT_ID_LEN);
844-
out.push_str(SHORT_ID_PREFIX);
845-
out.push('-');
846-
let mut remaining = SHORT_ID_LEN;
847-
while remaining > 0 {
848-
let byte: u8 = rng.random();
849-
if (byte as usize) < max_unbiased {
850-
out.push(SHORT_ID_ALPHABET[(byte as usize) % alphabet_len] as char);
851-
remaining -= 1;
852-
}
853-
}
854-
out
839+
crate::short_id::generate(SHORT_ID_PREFIX)
855840
}
856841

857842
/// POST the bundle to the ingestion server. In CI this skips the network call and

apps/desktop/src-tauri/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ mod redact;
111111
pub mod search;
112112
mod secrets;
113113
mod settings;
114+
mod short_id;
114115
mod space_poller;
115116
mod system_memory;
116117
#[cfg(target_os = "macos")]
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
//! Short ID generation for user-visible report IDs (error reports, crash reports).
2+
//!
3+
//! Produces IDs like `ERR-8F3A2` or `CRASH-K7J4P` from an unambiguous alphabet
4+
//! (`23456789ABCDEFGHJKMNPQRSTUVWXYZ` — no `0`/`O`, no `1`/`I`/`L`). Uses rejection
5+
//! sampling to avoid modulo bias. The alphabet is kept in sync with
6+
//! `apps/api-server/src/license.ts::generateShortId`.
7+
8+
use rand::RngExt;
9+
10+
/// Unambiguous alphabet: no `0`/`O`, no `1`/`I`/`L`. 31 chars.
11+
const ALPHABET: &[u8] = b"23456789ABCDEFGHJKMNPQRSTUVWXYZ";
12+
/// Number of random characters after the prefix and dash.
13+
const SUFFIX_LEN: usize = 5;
14+
15+
/// Generate a short ID like `{prefix}-XXXXX` using rejection sampling.
16+
///
17+
/// `prefix` is something like `"ERR"` or `"CRASH"`. The output shape is
18+
/// `{prefix}-{five-chars-from-alphabet}`. The user sees and reports this ID, so
19+
/// we pick an alphabet that's safe to read aloud or copy by eye.
20+
pub fn generate(prefix: &str) -> String {
21+
let mut rng = rand::rng();
22+
let alphabet_len = ALPHABET.len(); // 31
23+
// 256 - (256 % 31) = 232 — bytes at or above this would skew the distribution.
24+
let max_unbiased = 256 - (256 % alphabet_len);
25+
let mut out = String::with_capacity(prefix.len() + 1 + SUFFIX_LEN);
26+
out.push_str(prefix);
27+
out.push('-');
28+
let mut remaining = SUFFIX_LEN;
29+
while remaining > 0 {
30+
let byte: u8 = rng.random();
31+
if (byte as usize) < max_unbiased {
32+
out.push(ALPHABET[(byte as usize) % alphabet_len] as char);
33+
remaining -= 1;
34+
}
35+
}
36+
out
37+
}
38+
39+
#[cfg(test)]
40+
mod tests {
41+
use super::*;
42+
use std::collections::HashSet;
43+
44+
#[test]
45+
fn err_prefix_matches_shape() {
46+
let re = regex::Regex::new("^ERR-[23456789ABCDEFGHJKMNPQRSTUVWXYZ]{5}$").unwrap();
47+
for _ in 0..200 {
48+
let id = generate("ERR");
49+
assert!(re.is_match(&id), "ID `{id}` didn't match");
50+
}
51+
}
52+
53+
#[test]
54+
fn crash_prefix_matches_shape() {
55+
let re = regex::Regex::new("^CRASH-[23456789ABCDEFGHJKMNPQRSTUVWXYZ]{5}$").unwrap();
56+
for _ in 0..200 {
57+
let id = generate("CRASH");
58+
assert!(re.is_match(&id), "ID `{id}` didn't match");
59+
}
60+
}
61+
62+
#[test]
63+
fn ids_are_statistically_unique() {
64+
let mut seen = HashSet::new();
65+
for _ in 0..1000 {
66+
let id = generate("ERR");
67+
assert!(seen.insert(id), "duplicate ID within 1000 samples");
68+
}
69+
}
70+
}

0 commit comments

Comments
 (0)