Skip to main content

max / makenotwork

Observability + API consistency cold spots /audit Run 13 Obs/API cold spots: - custom_domain: distinguish a genuine NotFound (clean 404) from a DB/render error, which was collapsing to a silent 404 — making custom-domain outages look like missing pages. Real errors now log and surface the proper status via the AppError responder. - dashboard code tab: log the repo/collaborator DB-load failures instead of silently rendering "no repos" on a DB error (unwrap_or_default -> unwrap_or_else + warn!). - CLI add_domain: reuse the web path's validate_domain (per-label / charset / hyphen-edge checks) instead of the weaker contains('.') gate, and map the global UNIQUE(domain) violation to a clean 409 — one contract across the CLI and web domain paths (no raw 500). - exports extension_from_key: take the basename before extracting the extension and fall back to "bin" for extensionless keys, instead of returning the whole path as a bogus extension; fixed the tests that pinned the wrong behavior. - health: log a DB-stats query failure instead of silently rendering zero counts (the DB probes remain the authoritative liveness signal). clippy --lib clean; extension unit tests + custom_domains integration (14) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-01 21:46 UTC
Commit: b06bc60d47c91359087b36af785f1c8dbbf3593e
Parent: b7b048f
6 files changed, +84 insertions, -31 deletions
@@ -262,7 +262,7 @@ fn normalize_domain(input: &str) -> Result<String> {
262 262 }
263 263
264 264 /// Basic domain validation: must have at least one dot, no spaces, reasonable length.
265 - fn validate_domain(domain: &str) -> Result<()> {
265 + pub(crate) fn validate_domain(domain: &str) -> Result<()> {
266 266 if domain.len() > 253 {
267 267 return Err(AppError::validation(
268 268 "Domain name is too long.".to_string(),
@@ -417,7 +417,15 @@ async fn build_content_export(
417 417
418 418 /// Extract file extension from an S3 key (e.g. "user/item/audio/track.mp3" -> "mp3").
419 419 fn extension_from_key(key: &str) -> &str {
420 - key.rsplit('.').next().unwrap_or("bin")
420 + // Extract from the basename only: a dot earlier in the path (e.g. a user
421 + // handle like `alice.dev/...`) must not be mistaken for the extension, and an
422 + // extensionless key must fall back to "bin" rather than returning the whole
423 + // path as a bogus extension (audit Run 13).
424 + let basename = key.rsplit('/').next().unwrap_or(key);
425 + match basename.rsplit_once('.') {
426 + Some((stem, ext)) if !stem.is_empty() && !ext.is_empty() => ext,
427 + _ => "bin",
428 + }
421 429 }
422 430
423 431 /// Sanitize a title for use as a filename in the ZIP archive.
@@ -444,20 +452,27 @@ mod tests {
444 452 }
445 453
446 454 #[test]
447 - fn extension_from_key_no_dot_returns_whole_segment() {
448 - // rsplit('.').next() returns the whole string when no dot is present
449 - assert_eq!(extension_from_key("user/item/audio/noext"), "user/item/audio/noext");
455 + fn extension_from_key_no_dot_returns_bin() {
456 + // An extensionless key falls back to "bin", never the whole path.
457 + assert_eq!(extension_from_key("user/item/audio/noext"), "bin");
458 + }
459 +
460 + #[test]
461 + fn extension_from_key_dot_in_path_not_basename() {
462 + // A dot earlier in the path is not the extension.
463 + assert_eq!(extension_from_key("alice.dev/item/noext"), "bin");
464 + assert_eq!(extension_from_key("alice.dev/item/track.wav"), "wav");
450 465 }
451 466
452 467 #[test]
453 468 fn extension_from_key_empty_returns_bin() {
454 - // rsplit('.').next() on "" returns Some(""), which unwrap_or("bin") keeps as ""
455 - assert_eq!(extension_from_key(""), "");
469 + assert_eq!(extension_from_key(""), "bin");
456 470 }
457 471
458 472 #[test]
459 473 fn extension_from_key_dot_only() {
460 - assert_eq!(extension_from_key("file."), "");
474 + // Trailing dot → empty ext → "bin".
475 + assert_eq!(extension_from_key("file."), "bin");
461 476 }
462 477
463 478 #[test]
@@ -430,15 +430,17 @@ pub(super) async fn add_domain(
430 430 Json(req): Json<AddDomainRequest>,
431 431 ) -> Result<impl IntoResponse> {
432 432 let domain = req.domain.to_lowercase().trim().to_string();
433 - if domain.is_empty() || domain.len() > 253 || !domain.contains('.') {
434 - return Err(AppError::validation("Invalid domain".to_string()));
435 - }
436 - if domain.contains("makenot.work") || domain.contains("makenotwork") {
437 - return Err(AppError::validation("Cannot use MNW domains".to_string()));
438 - }
433 + // Reuse the web path's validator (per-label length + charset + hyphen-edge
434 + // checks) instead of the weaker `contains('.')` gate, so the CLI and web
435 + // domain paths enforce one contract (audit Run 13 API-consistency).
436 + crate::routes::api::domains::validate_domain(&domain)?;
439 437
440 438 let token = generate_verification_token();
441 - let record = db::custom_domains::create_custom_domain(&state.db, actor.user_id(), &domain, &token).await?;
439 + // Map the global `UNIQUE(domain)` violation to a clean 409, matching the web
440 + // path — a domain another user already holds must not surface as a raw 500.
441 + let record = db::custom_domains::create_custom_domain(&state.db, actor.user_id(), &domain, &token)
442 + .await
443 + .map_err(|e| crate::helpers::map_unique_violation(e, "That domain is already registered"))?;
442 444
443 445 Ok(Json(serde_json::json!({
444 446 "id": record.id.to_string(),
@@ -82,7 +82,15 @@ pub async fn try_handle(
82 82
83 83 Some(match result {
84 84 Ok(response) => response,
85 - Err(_) => StatusCode::NOT_FOUND.into_response(),
85 + // A genuine miss is a clean 404; a DB/render error must NOT masquerade as
86 + // "not found" (that made custom-domain outages look like missing pages
87 + // with no signal — audit Run 13 Obs). Log it and let the AppError
88 + // responder pick the real status (500 for infra errors).
89 + Err(AppError::NotFound) => StatusCode::NOT_FOUND.into_response(),
90 + Err(e) => {
91 + tracing::error!(error = ?e, host = %host, "custom domain render failed");
92 + e.into_response()
93 + }
86 94 })
87 95 }
88 96
@@ -134,7 +142,13 @@ pub async fn custom_domain_fallback(
134 142
135 143 match result {
136 144 Ok(response) => response,
137 - Err(_) => StatusCode::NOT_FOUND.into_response(),
145 + // Genuine miss → clean 404; a DB/render error must surface (log + real
146 + // status) rather than silently reading as "not found" (audit Run 13 Obs).
147 + Err(AppError::NotFound) => StatusCode::NOT_FOUND.into_response(),
148 + Err(e) => {
149 + tracing::error!(error = ?e, host = %host, "custom domain render failed");
150 + e.into_response()
151 + }
138 152 }
139 153 }
140 154
@@ -464,8 +464,21 @@ pub(super) async fn project_tab_code(
464 464 };
465 465
466 466 let git_enabled = state.config.git_repos_path.is_some();
467 - let db_linked_repos = db::git_repos::get_repos_by_project(&state.db, db_project.id).await.unwrap_or_default();
468 - let all_repos = db::git_repos::get_repos_by_user(&state.db, session_user.id).await.unwrap_or_default();
467 + // Degrade to an empty list on error, but LOG it — a silent `unwrap_or_default`
468 + // renders the Code tab as "no repos" and makes a DB outage look like empty
469 + // state with no signal (audit Run 13 Obs).
470 + let db_linked_repos = db::git_repos::get_repos_by_project(&state.db, db_project.id)
471 + .await
472 + .unwrap_or_else(|e| {
473 + tracing::warn!(error = ?e, project_id = %db_project.id, "code tab: failed to load linked repos; rendering as none");
474 + Vec::new()
475 + });
476 + let all_repos = db::git_repos::get_repos_by_user(&state.db, session_user.id)
477 + .await
478 + .unwrap_or_else(|e| {
479 + tracing::warn!(error = ?e, user_id = %session_user.id, "code tab: failed to load user repos; rendering as none");
480 + Vec::new()
481 + });
469 482 let available_repos: Vec<_> = all_repos.into_iter().filter(|r| r.project_id.is_none()).collect();
470 483
471 484 // Build linked repo views with collaborators. One batched query for all
@@ -475,7 +488,10 @@ pub(super) async fn project_tab_code(
475 488 std::collections::HashMap::new();
476 489 for c in db::repo_collaborators::list_collaborators_for_repos(&state.db, &repo_ids)
477 490 .await
478 - .unwrap_or_default()
491 + .unwrap_or_else(|e| {
492 + tracing::warn!(error = ?e, "code tab: failed to load repo collaborators; rendering none");
493 + Vec::new()
494 + })
479 495 {
480 496 collabs_by_repo.entry(c.repo_id).or_default().push(RepoCollaboratorView {
481 497 user_id: c.user_id.to_string(),
@@ -268,17 +268,23 @@ async fn collect_health(state: &AppState) -> HealthData {
268 268 );
269 269
270 270 // Get actual counts
271 - let stats = db::health::get_health_stats(&state.db).await.unwrap_or(db::health::DbHealthStats {
272 - user_count: 0,
273 - project_count: 0,
274 - item_count: 0,
275 - active_session_count: 0,
276 - active_creator_count: 0,
277 - transaction_count: 0,
278 - blog_post_count: 0,
279 - sync_app_count: 0,
280 - sync_device_count: 0,
281 - sync_log_entries: 0,
271 + // On failure the counts render as zeros, but that must not be SILENT — log it
272 + // so a stats-query failure is visible in the ops logs (the DB probes above are
273 + // the authoritative liveness signal; a real DB outage turns them red too).
274 + let stats = db::health::get_health_stats(&state.db).await.unwrap_or_else(|e| {
275 + tracing::error!(error = ?e, "health: DB stats query failed; rendering zero counts (see DB probes for liveness)");
276 + db::health::DbHealthStats {
277 + user_count: 0,
278 + project_count: 0,
279 + item_count: 0,
280 + active_session_count: 0,
281 + active_creator_count: 0,
282 + transaction_count: 0,
283 + blog_post_count: 0,
284 + sync_app_count: 0,
285 + sync_device_count: 0,
286 + sync_log_entries: 0,
287 + }
282 288 });
283 289
284 290 // Database status