refactor(core): unify error responses on {"detail": ...}; 404 no longer aborts without fallback
swadhinbiswas committed 3 months ago
73a8f3a
1 parent 13ffbce
commit 73a8f3a
6 files changed
+52 -34
Showing 6 changed files+52-34
▼crates/justapi-core/src/lib.rs
+13-0
| @@ -72,6 +72,19 @@ pub fn json_response(status: hyper::StatusCode, body: &str) -> Response<Response | ||
72 | 72 | .unwrap() |
73 | 73 | } |
74 | 74 | |
75 | +/// Canonical error envelope. Every non-2xx response in justapi uses this single | |
76 | +/// shape so clients have one contract to parse (see ADR-052). `detail` carries | |
77 | +/// the human-readable message; the numeric status lives in the HTTP status | |
78 | +/// line. Sensitive internals must never be placed in `detail` for 5xx. | |
79 | +pub fn error_response(status: hyper::StatusCode, detail: &str) -> Response<ResponseBody> { | |
80 | + json_response(status, &serde_json::json!({ "detail": detail }).to_string()) | |
81 | +} | |
82 | + | |
83 | +/// 422 validation error using the canonical `{"detail": ...}` envelope. | |
84 | +pub fn validation_response(detail: &str) -> Response<ResponseBody> { | |
85 | + error_response(hyper::StatusCode::UNPROCESSABLE_ENTITY, detail) | |
86 | +} | |
87 | + | |
75 | 88 | /// Lift an `anyhow::Result<Bytes>` stream into a streaming response. |
76 | 89 | pub fn streaming_response( |
77 | 90 | status: hyper::StatusCode, |
78 | 91 | |
▼crates/justapi-core/src/panic.rs
+2-2
| @@ -39,12 +39,12 @@ pub fn with_panic_recovery(handler: HandlerFn) -> HandlerFn { | ||
39 | 39 | tracing::error!(error = %e, "Handler returned error"); |
40 | 40 | Ok(json_response( |
41 | 41 | StatusCode::INTERNAL_SERVER_ERROR, |
42 | - r#"{"error":"Internal error"}"#, | |
42 | + r#"{"detail":"Internal error"}"#, | |
43 | 43 | )) |
44 | 44 | } |
45 | 45 | Err(_panic) => Ok(json_response( |
46 | 46 | StatusCode::INTERNAL_SERVER_ERROR, |
47 | - r#"{"error":"Internal server error"}"#, | |
47 | + r#"{"detail":"Internal server error"}"#, | |
48 | 48 | )), |
49 | 49 | } |
50 | 50 | }) |
51 | 51 | |
▼crates/justapi-core/src/resilience.rs
+7-7
| @@ -222,7 +222,7 @@ impl<B: Send + Sync + 'static> Middleware<B> for PerRouteCircuitBreakerMiddlewar | ||
222 | 222 | } |
223 | 223 | |
224 | 224 | fn circuit_open_response() -> Response<ResponseBody> { |
225 | - let body = r#"{"error":"service unavailable — circuit breaker open"}"#; | |
225 | + let body = r#"{"detail":"service unavailable — circuit breaker open"}"#; | |
226 | 226 | let mut resp = json_text_response(StatusCode::SERVICE_UNAVAILABLE, body); |
227 | 227 | resp.headers_mut().insert("retry-after", "30".parse().unwrap()); |
228 | 228 | resp |
| @@ -254,7 +254,7 @@ impl<B: Send + 'static> Middleware<B> for TimeoutMiddleware { | ||
254 | 254 | } |
255 | 255 | |
256 | 256 | fn timeout_response(timeout: Duration) -> Response<ResponseBody> { |
257 | - let body = format!(r#"{{"error":"request timed out after {}ms"}}"#, timeout.as_millis()); | |
257 | + let body = format!(r#"{{"detail":"request timed out after {}ms"}}"#, timeout.as_millis()); | |
258 | 258 | json_text_response(StatusCode::GATEWAY_TIMEOUT, &body) |
259 | 259 | } |
260 | 260 | |
| @@ -356,7 +356,7 @@ impl<B: Send + 'static> Middleware<B> for BulkheadMiddleware { | ||
356 | 356 | } |
357 | 357 | _ => Ok(json_text_response( |
358 | 358 | StatusCode::SERVICE_UNAVAILABLE, |
359 | - r#"{"error":"too many concurrent requests — bulkhead limit reached"}"#, | |
359 | + r#"{"detail":"too many concurrent requests — bulkhead limit reached"}"#, | |
360 | 360 | )), |
361 | 361 | } |
362 | 362 | } |
| @@ -442,7 +442,7 @@ impl<B: Send + 'static> Middleware<B> for FallbackMiddleware { | ||
442 | 442 | } else { |
443 | 443 | Ok(json_text_response( |
444 | 444 | StatusCode::INTERNAL_SERVER_ERROR, |
445 | - r#"{"error":"internal server error"}"#, | |
445 | + r#"{"detail":"internal server error"}"#, | |
446 | 446 | )) |
447 | 447 | } |
448 | 448 | } |
| @@ -512,7 +512,7 @@ impl<B: Send + 'static> Middleware<B> for ChaosMiddleware { | ||
512 | 512 | let roll: f64 = rand::random(); |
513 | 513 | if roll < self.config.error_p { |
514 | 514 | let body = format!( |
515 | - r#"{{"error":"chaos injected failure","status":{}}}"#, | |
515 | + r#"{{"detail":"chaos injected failure","status":{}}}"#, | |
516 | 516 | self.config.error_status.as_u16() |
517 | 517 | ); |
518 | 518 | return Ok(json_text_response(self.config.error_status, &body)); |
| @@ -577,7 +577,7 @@ mod tests { | ||
577 | 577 | fn five_hundred_handler() -> HandlerFn<TestBody> { |
578 | 578 | Arc::new(|_req: Request<TestBody>| { |
579 | 579 | Box::pin(async { |
580 | - Ok(json_response(StatusCode::INTERNAL_SERVER_ERROR, r#"{"error":"server error"}"#)) | |
580 | + Ok(json_response(StatusCode::INTERNAL_SERVER_ERROR, r#"{"detail":"server error"}"#)) | |
581 | 581 | }) |
582 | 582 | }) |
583 | 583 | } |
| @@ -645,7 +645,7 @@ mod tests { | ||
645 | 645 | if f.load(std::sync::atomic::Ordering::Relaxed) { |
646 | 646 | Ok(json_response( |
647 | 647 | StatusCode::INTERNAL_SERVER_ERROR, |
648 | - r#"{"error":"server error"}"#, | |
648 | + r#"{"detail":"server error"}"#, | |
649 | 649 | )) |
650 | 650 | } else { |
651 | 651 | Ok(json_response(StatusCode::OK, r#"{"ok":true}"#)) |
652 | 652 | |
▼crates/justapi-core/src/server/mod.rs
+28-23
| @@ -401,7 +401,7 @@ impl Server { | ||
401 | 401 | router.insert(Method::POST, "/graphql", Handler::GraphQL).expect("valid"); |
402 | 402 | router.set_fallback(Handler::Static { |
403 | 403 | status: StatusCode::NOT_FOUND, |
404 | - body: r#"{"error":"not found"}"#, | |
404 | + body: r#"{"detail":"not found"}"#, | |
405 | 405 | }); |
406 | 406 | self.router = Some(router); |
407 | 407 | self.default_routes_set = true; |
| @@ -855,7 +855,7 @@ pub async fn serve(listener: TcpListener) -> Result<()> { | ||
855 | 855 | let mut router: Router<Handler> = Router::new(); |
856 | 856 | router.set_fallback(Handler::Static { |
857 | 857 | status: StatusCode::NOT_FOUND, |
858 | - body: r#"{"error":"not found"}"#, | |
858 | + body: r#"{"detail":"not found"}"#, | |
859 | 859 | }); |
860 | 860 | router |
861 | 861 | .insert( |
| @@ -924,22 +924,24 @@ fn make_handler( | ||
924 | 924 | } |
925 | 925 | Err(crate::router::RouterError::MethodNotAllowed) => Ok(json_response( |
926 | 926 | StatusCode::METHOD_NOT_ALLOWED, |
927 | - r#"{"error":"method not allowed"}"#, | |
927 | + r#"{"detail":"method not allowed"}"#, | |
928 | 928 | )), |
929 | - Err(crate::router::RouterError::NotFound) => { | |
930 | - let fb = router.fallback().unwrap(); | |
931 | - let m = Match { handler: fb, params: matchit::Params::new() }; | |
932 | - execute_handler( | |
933 | - m, | |
934 | - req, | |
935 | - &pool, | |
936 | - &metrics, | |
937 | - Some(&health_registry), | |
938 | - Some(&graphql_schema), | |
939 | - openapi_spec.as_deref().map(|s| s.as_str()), | |
940 | - ) | |
941 | - .await | |
942 | - } | |
929 | + Err(crate::router::RouterError::NotFound) => match router.fallback() { | |
930 | + Some(fb) => { | |
931 | + let m = Match { handler: fb, params: matchit::Params::new() }; | |
932 | + execute_handler( | |
933 | + m, | |
934 | + req, | |
935 | + &pool, | |
936 | + &metrics, | |
937 | + Some(&health_registry), | |
938 | + Some(&graphql_schema), | |
939 | + openapi_spec.as_deref().map(|s| s.as_str()), | |
940 | + ) | |
941 | + .await | |
942 | + } | |
943 | + None => Ok(json_response(StatusCode::NOT_FOUND, r#"{"detail":"not found"}"#)), | |
944 | + }, | |
943 | 945 | } |
944 | 946 | }) |
945 | 947 | }) |
| @@ -1205,7 +1207,7 @@ async fn serve_http( | ||
1205 | 1207 | } |
1206 | 1208 | Ok(json_response( |
1207 | 1209 | StatusCode::NOT_FOUND, |
1208 | - r#"{"error":"not found"}"# | |
1210 | + r#"{"detail":"not found"}"# | |
1209 | 1211 | )) |
1210 | 1212 | } |
1211 | 1213 | }, |
| @@ -1222,7 +1224,7 @@ async fn serve_http( | ||
1222 | 1224 | ); |
1223 | 1225 | Ok(json_response( |
1224 | 1226 | StatusCode::GATEWAY_TIMEOUT, |
1225 | - r#"{"error":"request timeout"}"# | |
1227 | + r#"{"detail":"request timeout"}"# | |
1226 | 1228 | )) |
1227 | 1229 | } |
1228 | 1230 | } |
| @@ -1315,7 +1317,10 @@ async fn execute_handler( | ||
1315 | 1317 | bytes |
1316 | 1318 | } |
1317 | 1319 | Err(_) => { |
1318 | - return Ok(json_response(StatusCode::BAD_REQUEST, r#"{"error":"bad request"}"#)) | |
1320 | + return Ok(json_response( | |
1321 | + StatusCode::BAD_REQUEST, | |
1322 | + r#"{"detail":"bad request"}"#, | |
1323 | + )) | |
1319 | 1324 | } |
1320 | 1325 | }; |
1321 | 1326 | let mut buf = pool.acquire(body_bytes.len()); |
| @@ -1678,7 +1683,7 @@ async fn serve_with_tls( | ||
1678 | 1683 | } |
1679 | 1684 | Ok(json_response( |
1680 | 1685 | StatusCode::NOT_FOUND, |
1681 | - r#"{"error":"not found"}"#, | |
1686 | + r#"{"detail":"not found"}"#, | |
1682 | 1687 | )) |
1683 | 1688 | }, |
1684 | 1689 | }, |
| @@ -1694,7 +1699,7 @@ async fn serve_with_tls( | ||
1694 | 1699 | ); |
1695 | 1700 | Ok(json_response( |
1696 | 1701 | StatusCode::GATEWAY_TIMEOUT, |
1697 | - r#"{"error":"request timeout"}"# | |
1702 | + r#"{"detail":"request timeout"}"# | |
1698 | 1703 | )) |
1699 | 1704 | } |
1700 | 1705 | } |
| @@ -1778,7 +1783,7 @@ mod tests { | ||
1778 | 1783 | let mut router: Router<Handler> = Router::new(); |
1779 | 1784 | router.set_fallback(Handler::Static { |
1780 | 1785 | status: StatusCode::NOT_FOUND, |
1781 | - body: r#"{"error":"not found"}"#, | |
1786 | + body: r#"{"detail":"not found"}"#, | |
1782 | 1787 | }); |
1783 | 1788 | let fb = router.fallback().unwrap(); |
1784 | 1789 | assert!(matches!(fb, Handler::Static { .. })); |
1785 | 1790 | |
▼crates/justapi-core/src/static_files.rs
+1-1
| @@ -323,7 +323,7 @@ fn parse_range(range_str: &str, total_len: u64) -> Option<ByteRange> { | ||
323 | 323 | } |
324 | 324 | |
325 | 325 | fn not_found() -> Result<Response<ResponseBody>> { |
326 | - Ok(crate::json_response(StatusCode::NOT_FOUND, r#"{"error":"not found"}"#)) | |
326 | + Ok(crate::json_response(StatusCode::NOT_FOUND, r#"{"detail":"not found"}"#)) | |
327 | 327 | } |
328 | 328 | |
329 | 329 | fn guess_content_type(path: &Path) -> &'static str { |
330 | 330 | |
▼crates/justapi-core/tests/integration.rs
+1-1
| @@ -79,7 +79,7 @@ async fn test_loopback_404() { | ||
79 | 79 | let addr = start_server().await; |
80 | 80 | let (status, body) = send_request(addr, Method::GET, "/nonexistent", None).await; |
81 | 81 | assert_eq!(status, StatusCode::NOT_FOUND); |
82 | - assert_eq!(&body[..], br#"{"error":"not found"}"#); | |
82 | + assert_eq!(&body[..], br#"{"detail":"not found"}"#); | |
83 | 83 | } |
84 | 84 | |
85 | 85 | #[tokio::test] |
86 | 86 | |