Skip to content

Commit ca2e77a

Browse files
committed
Address PR review: accurate chunk-read hint and retryable 503 body
- read_config_entry: reword to cover unseeded/missing/transient reads, since it is the shared seam for both the blob key and each chunk key. - user_message: return 'Service temporarily unavailable' for the retryable 503 variants (ConfigStoreUnavailable, KvStore) instead of the 500-flavored generic body; still leaks no internal detail. Tests updated.
1 parent bca5e22 commit ca2e77a

2 files changed

Lines changed: 37 additions & 8 deletions

File tree

crates/trusted-server-core/src/error.rs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ impl IntoHttpResponse for TrustedServerError {
148148
// Consent strings may contain user data; return category only.
149149
Self::GdprConsent { .. } => "GDPR consent error".to_string(),
150150
Self::InvalidHeaderValue { .. } => "Invalid header value".to_string(),
151+
// Retryable 503s: signal transient unavailability so clients and
152+
// monitoring can distinguish retry-me from terminal, without leaking
153+
// any internal detail (full details are logged in to_error_response).
154+
Self::ConfigStoreUnavailable { .. } | Self::KvStore { .. } => {
155+
"Service temporarily unavailable".to_string()
156+
}
151157
// Server/integration errors (5xx/502/503) — generic message only.
152158
// Full details are already logged via log::error! in to_error_response.
153159
_ => "An internal error occurred".to_string(),
@@ -333,10 +339,6 @@ mod tests {
333339
TrustedServerError::Configuration {
334340
message: "secret db path".into(),
335341
},
336-
TrustedServerError::KvStore {
337-
store_name: "users".into(),
338-
message: "timeout".into(),
339-
},
340342
TrustedServerError::Proxy {
341343
message: "upstream 10.0.0.1 refused".into(),
342344
},
@@ -427,14 +429,41 @@ mod tests {
427429
StatusCode::SERVICE_UNAVAILABLE,
428430
"config-store read failure should map to 503"
429431
);
430-
// Detail stays server-side; the public body is the generic catch-all.
432+
// Detail stays server-side; the public body signals retryable
433+
// unavailability without leaking internal config-store detail.
431434
assert_eq!(
432435
error.user_message(),
433-
"An internal error occurred",
434-
"503 client body must not leak internal config-store detail"
436+
"Service temporarily unavailable",
437+
"503 client body must be retry-flavored and leak no internal detail"
435438
);
436439
}
437440

441+
#[test]
442+
fn retryable_503_variants_return_service_unavailable_body() {
443+
let cases = [
444+
TrustedServerError::ConfigStoreUnavailable {
445+
store_name: "app_config".into(),
446+
message: "unseeded".into(),
447+
},
448+
TrustedServerError::KvStore {
449+
store_name: "users".into(),
450+
message: "timeout".into(),
451+
},
452+
];
453+
for error in &cases {
454+
assert_eq!(
455+
error.status_code(),
456+
StatusCode::SERVICE_UNAVAILABLE,
457+
"should map to 503 for {error:?}"
458+
);
459+
assert_eq!(
460+
error.user_message(),
461+
"Service temporarily unavailable",
462+
"503 body should be retry-flavored and leak no detail for {error:?}"
463+
);
464+
}
465+
}
466+
438467
#[test]
439468
fn status_code_maps_each_error_variant_to_expected_http_response() {
440469
// Compile-time guard: adding a TrustedServerError variant without

crates/trusted-server-core/src/settings_data.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ fn read_config_entry(
9494
.change_context(TrustedServerError::ConfigStoreUnavailable {
9595
store_name: store_name.to_string(),
9696
message: format!(
97-
"unavailable or not seeded (failed to read `{key}`) — run `ts config push`"
97+
"read failed for `{key}` (unseeded, missing, or transient) — run `ts config push` to (re)seed"
9898
),
9999
})
100100
}

0 commit comments

Comments
 (0)