Skip to content

Commit 2a645a8

Browse files
feat: error handling to avoid panics
1 parent 5f42def commit 2a645a8

File tree

8 files changed

+657
-373
lines changed

8 files changed

+657
-373
lines changed

confidence-cloudflare-resolver/src/lib.rs

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ static CONFIDENCE_CLIENT_SECRET: OnceLock<String> = OnceLock::new();
3131

3232
static FLAG_LOGGER: Lazy<Logger> = Lazy::new(|| Logger::new());
3333

34-
static RESOLVER_STATE: Lazy<ResolverState> =
35-
Lazy::new(|| ResolverState::from_proto(STATE_JSON.to_owned().into(), ACCOUNT_ID));
34+
static RESOLVER_STATE: Lazy<ResolverState> = Lazy::new(|| {
35+
ResolverState::from_proto(STATE_JSON.to_owned().try_into().unwrap(), ACCOUNT_ID).unwrap()
36+
});
3637

3738
trait ResponseExt {
3839
fn with_cors_headers(self, allowed_origin: &str) -> Result<Self>
@@ -156,17 +157,16 @@ pub async fn main(req: Request, env: Env, _ctx: Context) -> Result<Response> {
156157
evaluation_context,
157158
&Bytes::from(STANDARD.decode(ENCRYPTION_KEY_BASE64).unwrap()),
158159
) {
159-
Some(resolver) => match resolver.resolve_flags(&resolver_request) {
160+
Ok(resolver) => match resolver.resolve_flags(&resolver_request) {
160161
Ok(response) => Response::from_json(&response)?
161162
.with_cors_headers(&allowed_origin),
162-
Err(err) => Response::error(
163-
format!("Failed to resolve flags: {}", err),
164-
500,
165-
)?
166-
.with_cors_headers(&allowed_origin),
163+
Err(msg) => {
164+
Response::error(msg, 500)?.with_cors_headers(&allowed_origin)
165+
}
167166
},
168-
None => Response::error("Error setting up the resolver", 500)?
169-
.with_cors_headers(&allowed_origin),
167+
Err(msg) => {
168+
Response::error(msg, 500)?.with_cors_headers(&allowed_origin)
169+
}
170170
}
171171
}
172172
"flags:apply" => {
@@ -187,21 +187,20 @@ pub async fn main(req: Request, env: Env, _ctx: Context) -> Result<Response> {
187187
Struct::default(),
188188
&Bytes::from(STANDARD.decode(ENCRYPTION_KEY_BASE64).unwrap()),
189189
) {
190-
Some(resolver) => match resolver.apply_flags(&apply_flag_req) {
191-
Ok(_response) => {
190+
Ok(resolver) => match resolver.apply_flags(&apply_flag_req) {
191+
Ok(()) => {
192192
return Response::from_json(&ApplyFlagsResponse::default());
193193
}
194-
Err(err) => {
195-
Response::error(format!("Failed to apply flags: {}", err), 500)?
196-
.with_cors_headers(&allowed_origin)
194+
Err(msg) => {
195+
Response::error(msg, 500)?.with_cors_headers(&allowed_origin)
197196
}
198197
},
199-
None => Response::error("Error setting up the resolver", 500)?
200-
.with_cors_headers(&allowed_origin),
198+
Err(msg) => {
199+
Response::error(msg, 500)?.with_cors_headers(&allowed_origin)
200+
}
201201
}
202202
}
203-
_ => Response::error("The URL is invalid", 404)?
204-
.with_cors_headers(&allowed_origin),
203+
_ => Response::error("Not found", 404)?.with_cors_headers(&allowed_origin),
205204
}
206205
}
207206
})

confidence-resolver/src/err.rs

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
use alloc::string::{String, ToString};
2+
use core::panic::Location;
3+
4+
/// A minimal error type suitable as a replacement for runtime panics.
5+
///
6+
/// - Its only state is a 48‑bit code intended to be unique per call site or tag.
7+
/// - Renders as an 8‑char base64url code (no padding).
8+
/// - Use `ErrorCode::from_location()` (#[track_caller]) to derive a code from the call site, or
9+
/// `ErrorCode::from_tag("module.feature.case")` for a stable tag.
10+
/// - Typical internal use: return `Fallible<T>` (alias for `Result<T, ErrorCode>`) and propagate with `?`.
11+
/// - At API boundaries that return `Result<T, String>`, `?` works via `From<ErrorCode> for String`
12+
/// and renders as `internal error [XXXXXXXX]`.
13+
/// - `Option<T>` or `Result<T,_>` can be converted to `Fallible<T>` via `.or_fail()` See `OrFailExt`
14+
///
15+
/// Note: We do not (yet) ship a code→location/tag table; that can be generated in a separate build if needed.
16+
///
17+
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
18+
pub struct ErrorCode([u8; 6]);
19+
20+
impl ErrorCode {
21+
pub const fn new(code: u64) -> Self {
22+
let b = code.to_le_bytes();
23+
ErrorCode([b[0], b[1], b[2], b[3], b[4], b[5]])
24+
}
25+
26+
/// Builds a code from a stable tag string.
27+
///
28+
/// - Useful when you want stability across refactors (line moves).
29+
/// - Keep tags short and unique within the crate; consider a CI check for duplicates.
30+
pub const fn from_tag(tag: &str) -> Self {
31+
ErrorCode::new(fnv1a64([tag.as_bytes()]))
32+
}
33+
34+
/// Builds a code from the caller’s file/line/column.
35+
///
36+
/// - Uses `#[track_caller]` so the code reflects the call site (moves if lines change).
37+
/// - Prefer this where you’d otherwise `panic!`.
38+
#[track_caller]
39+
pub const fn from_location() -> Self {
40+
let loc = Location::caller();
41+
let parts = [
42+
loc.file().as_bytes(),
43+
&loc.line().to_le_bytes(),
44+
&loc.column().to_le_bytes(),
45+
];
46+
ErrorCode::new(fnv1a64(parts))
47+
}
48+
49+
/// Returns the 8‑character base64url encoding of the 48‑bit code (no padding).
50+
pub fn b64(self) -> [u8; 8] {
51+
// load to 48-bit value
52+
let mut v: u64 = 0;
53+
for b in self.0 {
54+
v = (v << 8) | (b as u64);
55+
}
56+
57+
// emit 8 sextets (MSB first)
58+
core::array::from_fn(|i| {
59+
let shift = 42 - i * 6;
60+
let sextet = ((v >> shift) & 0x3F) as u8;
61+
b64u6(sextet)
62+
})
63+
}
64+
65+
/// Returns the raw 48‑bit code in the low bits of a `u64` (little‑endian packing).
66+
pub fn code(self) -> u64 {
67+
let mut b = [0u8; 8];
68+
b[..6].copy_from_slice(&self.0);
69+
u64::from_le_bytes(b)
70+
}
71+
72+
pub fn b64_str(self) -> impl core::fmt::Display {
73+
struct D([u8; 8]);
74+
impl core::fmt::Display for D {
75+
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
76+
// safe because base 64 is ASCII so identical bytes in utf8
77+
write!(f, "{}", unsafe { core::str::from_utf8_unchecked(&self.0) })
78+
}
79+
}
80+
D(self.b64())
81+
}
82+
}
83+
84+
impl From<ErrorCode> for String {
85+
fn from(e: ErrorCode) -> String {
86+
e.to_string()
87+
}
88+
}
89+
90+
/// Convenience alias for `Result<T, ErrorCode>` used in internal APIs.
91+
pub type Fallible<T> = core::result::Result<T, ErrorCode>;
92+
93+
/// Extension methods to collapse `Option<T>`/`Result<T, E>` into `Fallible<T>`.
94+
///
95+
/// - `Option<T>::or_fail()` → code from call site if `None`.
96+
/// - `Result<T, E>::or_fail()` → maps any `Err(E)` to a call‑site code.
97+
/// Prefer plain `?` when the error is already `ErrorCode`.
98+
pub trait OrFailExt<T> {
99+
#[track_caller]
100+
fn or_fail(self) -> Fallible<T>;
101+
}
102+
103+
/// Macro: derive an `ErrorCode` from a module‑qualified tag.
104+
///
105+
/// - Usage: `module_err!(":subsystem.case")`
106+
/// - Expands to `ErrorCode::from_tag(concat!(module_path!(), tag))`.
107+
/// - Returns an `ErrorCode` value (not a `Result`); use with `ok_or(...)` / `map_err(...)`.
108+
///
109+
/// Examples:
110+
/// ```rust
111+
/// let code = module_err!(":gzip.crc_mismatch");
112+
/// let crc = buf.get(..4).ok_or(module_err!(":gzip.truncated_crc"))?;
113+
/// ```
114+
#[macro_export]
115+
macro_rules! module_err {
116+
($tag:literal) => {
117+
$crate::ErrorCode::from_tag(concat!(module_path!(), $tag))
118+
};
119+
}
120+
121+
/// Macro: early‑return with `Err(ErrorCode)`.
122+
///
123+
/// Forms:
124+
/// - `fail!()` → `return Err(ErrorCode::from_location())`
125+
/// - `fail!(":tag")` → `return Err(module_err!(":tag"))`
126+
///
127+
/// Notes:
128+
/// - Add a semicolon at the call site when used as a statement: `fail!();`.
129+
/// - Prefer `.or_fail()?` on `Option`/`Result` when you’re not immediately returning.
130+
///
131+
/// Examples:
132+
/// ```rust
133+
/// if bad_magic { fail!(":parser.bad_magic"); }
134+
/// let v = maybe_val.or_fail()?; // alternative for Option
135+
/// ```
136+
#[macro_export]
137+
macro_rules! fail {
138+
() => {
139+
return Err($crate::ErrorCode::from_location())
140+
};
141+
($tag:literal) => {
142+
return Err($crate::module_err!($tag))
143+
};
144+
}
145+
146+
impl<T> OrFailExt<T> for Option<T> {
147+
#[track_caller]
148+
fn or_fail(self) -> Fallible<T> {
149+
self.ok_or(ErrorCode::from_location())
150+
}
151+
}
152+
153+
impl<T, E> OrFailExt<T> for Result<T, E> {
154+
#[track_caller]
155+
fn or_fail(self) -> Fallible<T> {
156+
self.map_err(|_| ErrorCode::from_location())
157+
}
158+
}
159+
160+
impl core::fmt::Display for ErrorCode {
161+
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
162+
write!(f, "internal error [{}]", self.b64_str())
163+
}
164+
}
165+
166+
#[allow(clippy::indexing_slicing)]
167+
const fn fnv1a64<const N: usize>(parts: [&[u8]; N]) -> u64 {
168+
const FNV64_INIT: u64 = 0xCBF2_9CE4_8422_2325;
169+
const FNV64_PRIME: u64 = 0x1000_0000_01B3;
170+
171+
let mut h = FNV64_INIT;
172+
let mut i = 0;
173+
while i < N {
174+
let b = parts[i];
175+
let mut j = 0;
176+
while j < b.len() {
177+
h ^= b[j] as u64;
178+
h = h.wrapping_mul(FNV64_PRIME);
179+
j += 1;
180+
}
181+
i += 1;
182+
}
183+
h
184+
}
185+
186+
#[inline]
187+
fn b64u6(x: u8) -> u8 {
188+
match x {
189+
0..=25 => b'A' + x,
190+
26..=51 => b'a' + (x - 26),
191+
52..=61 => b'0' + (x - 52),
192+
62 => b'-',
193+
_ => b'_', // 63
194+
}
195+
}
196+
197+
#[cfg(test)]
198+
mod tests {
199+
use super::*;
200+
201+
#[test]
202+
fn display() {
203+
let e = ErrorCode::from_location();
204+
let s = e.to_string();
205+
assert!(s.starts_with("internal error ["));
206+
}
207+
208+
#[test]
209+
fn different_call_sites_differ() {
210+
let a = ErrorCode::from_location();
211+
let b = ErrorCode::from_location(); // different line ⇒ different site
212+
assert_ne!(a, b);
213+
}
214+
}

confidence-resolver/src/flag_logger.rs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,9 @@ impl FlagLogger for Logger {
111111
}
112112

113113
for f in &flag_logs_message.flag_resolve_info {
114-
let flag_info =
115-
if let Some(map_item_resolve_info) = flag_resolve_map.get_mut(&f.flag) {
116-
map_item_resolve_info
117-
} else {
118-
flag_resolve_map.insert(f.flag.clone(), VariantRuleResolveInfo::new());
119-
flag_resolve_map.get_mut(&f.flag).unwrap()
120-
};
114+
let flag_info = flag_resolve_map
115+
.entry(f.flag.clone())
116+
.or_insert_with(VariantRuleResolveInfo::new);
121117
update_rule_variant_info(flag_info, f);
122118
}
123119
for fa in &flag_logs_message.flag_assigned {
@@ -142,7 +138,7 @@ impl FlagLogger for Logger {
142138
.iter()
143139
.map(|r| VariantResolveInfo {
144140
variant: r.0.clone(),
145-
count: r.1.clone(),
141+
count: *r.1,
146142
})
147143
.collect();
148144

@@ -156,7 +152,7 @@ impl FlagLogger for Logger {
156152
.assignment_count
157153
.iter()
158154
.map(|(assignment_id, count)| AssignmentResolveInfo {
159-
count: count.clone(),
155+
count: *count,
160156
assignment_id: assignment_id.clone(),
161157
})
162158
.collect(),
@@ -255,12 +251,11 @@ fn convert_to_write_assign_request(
255251
},
256252
};
257253
// Create the `FlagAssigned` instance
258-
return FlagAssigned {
254+
FlagAssigned {
259255
client_info: Some(client_info.clone()),
260256
resolve_id: ToString::to_string(resolve_id),
261257
flags: vec![applied_flag], // Add the `AppliedFlag` to the repeated `flags` field
262-
..Default::default()
263-
};
258+
}
264259
})
265260
.collect();
266261

@@ -419,7 +414,7 @@ fn update_rule_variant_info(
419414
for aa in &rule_info.assignment_resolve_info {
420415
let count = match current_assignments.get(&aa.assignment_id) {
421416
None => 0,
422-
Some(a) => a.clone(),
417+
Some(a) => *a,
423418
} + aa.count;
424419
new_assignment_count.insert(aa.clone().assignment_id, count);
425420
}
@@ -435,7 +430,7 @@ fn update_rule_variant_info(
435430
for variant_info in &rule_resolve_info.variant_resolve_info {
436431
let count = match flag_info.variant_resolve_info.get(&variant_info.variant) {
437432
None => 0,
438-
Some(v) => v.clone(),
433+
Some(v) => *v,
439434
} + variant_info.count;
440435
flag_info
441436
.variant_resolve_info

0 commit comments

Comments
 (0)