Skip to content

Commit f6608ce

Browse files
refactor: consolidate modules and improve error handling
- Consolidate request.rs + response.rs into http_helpers.rs - Move UpstreamTarget from types.rs into upstream/mod.rs - Replace production unwrap() calls with expect() or pattern matching - Split static_files/mod.rs into range.rs and compression.rs - Add integration tests common module with shared fixtures Files changed: - http_helpers.rs: new consolidated HTTP utilities module - static_files/compression.rs: content encoding utilities - static_files/range.rs: HTTP Range request handling - tests/common/mod.rs: shared test utilities - Deleted: request.rs, response.rs, types.rs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3cf742b commit f6608ce

17 files changed

Lines changed: 879 additions & 533 deletions

File tree

crates/proxy/src/builtin_handlers.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ fn status_handler(state: &BuiltinHandlerState, request_id: &str) -> Response<Ful
119119
.header("X-Request-Id", request_id)
120120
.header("Cache-Control", "no-cache, no-store, must-revalidate")
121121
.body(Full::new(Bytes::from(body)))
122-
.unwrap()
122+
.expect("static response builder with valid headers cannot fail")
123123
}
124124

125125
/// Health check handler
@@ -139,7 +139,7 @@ fn health_handler(request_id: &str) -> Response<Full<Bytes>> {
139139
.header("X-Request-Id", request_id)
140140
.header("Cache-Control", "no-cache, no-store, must-revalidate")
141141
.body(Full::new(Bytes::from(body)))
142-
.unwrap()
142+
.expect("static response builder with valid headers cannot fail")
143143
}
144144

145145
/// Prometheus metrics handler
@@ -161,7 +161,7 @@ fn metrics_handler(request_id: &str) -> Response<Full<Bytes>> {
161161
.header("Content-Type", "text/plain; version=0.0.4; charset=utf-8")
162162
.header("X-Request-Id", request_id)
163163
.body(Full::new(Bytes::from(metrics)))
164-
.unwrap()
164+
.expect("static response builder with valid headers cannot fail")
165165
}
166166

167167
/// 404 Not Found handler
@@ -183,7 +183,7 @@ fn not_found_handler(request_id: &str) -> Response<Full<Bytes>> {
183183
.header("Content-Type", "application/json; charset=utf-8")
184184
.header("X-Request-Id", request_id)
185185
.body(Full::new(Bytes::from(body_bytes)))
186-
.unwrap()
186+
.expect("static response builder with valid headers cannot fail")
187187
}
188188

189189
#[cfg(test)]

crates/proxy/src/http3.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ impl Http3Config {
542542
impl Default for Http3Config {
543543
fn default() -> Self {
544544
Self {
545-
listen_addr: "0.0.0.0:443".parse().unwrap(),
545+
listen_addr: "0.0.0.0:443".parse().expect("hardcoded valid address"),
546546
tls: Http3TlsConfig::default(),
547547
transport: QuicTransportConfig::default(),
548548
http3: Http3Settings::default(),

crates/proxy/src/http_helpers.rs

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
//! HTTP request and response helpers for Sentinel proxy
2+
//!
3+
//! This module provides utilities for:
4+
//! - Extracting request information from Pingora sessions
5+
//! - Writing HTTP responses to Pingora sessions
6+
//! - Generating and managing correlation IDs
7+
//!
8+
//! These helpers reduce boilerplate in the main proxy logic and ensure
9+
//! consistent handling of HTTP operations.
10+
11+
use bytes::Bytes;
12+
use http::Response;
13+
use http_body_util::{BodyExt, Full};
14+
use pingora::http::ResponseHeader;
15+
use pingora::prelude::*;
16+
use pingora::proxy::Session;
17+
use std::collections::HashMap;
18+
19+
use crate::routing::RequestInfo;
20+
21+
// ============================================================================
22+
// Request Helpers
23+
// ============================================================================
24+
25+
/// Extract request info from a Pingora session
26+
///
27+
/// Builds a `RequestInfo` struct from the session's request headers,
28+
/// suitable for route matching and processing.
29+
///
30+
/// # Example
31+
///
32+
/// ```ignore
33+
/// let request_info = extract_request_info(session);
34+
/// let route = router.match_request(&request_info);
35+
/// ```
36+
pub fn extract_request_info(session: &Session) -> RequestInfo {
37+
let req_header = session.req_header();
38+
39+
let mut headers = HashMap::new();
40+
for (name, value) in req_header.headers.iter() {
41+
if let Ok(value_str) = value.to_str() {
42+
headers.insert(name.as_str().to_lowercase(), value_str.to_string());
43+
}
44+
}
45+
46+
let host = headers.get("host").cloned().unwrap_or_default();
47+
let path = req_header.uri.path().to_string();
48+
49+
RequestInfo {
50+
method: req_header.method.as_str().to_string(),
51+
path: path.clone(),
52+
host,
53+
headers,
54+
query_params: RequestInfo::parse_query_params(&path),
55+
}
56+
}
57+
58+
/// Extract or generate a correlation ID from request headers
59+
///
60+
/// Looks for existing correlation ID headers in order of preference:
61+
/// 1. `x-correlation-id`
62+
/// 2. `x-request-id`
63+
/// 3. `x-trace-id`
64+
///
65+
/// If none are found, generates a new UUID v4.
66+
///
67+
/// # Example
68+
///
69+
/// ```ignore
70+
/// let correlation_id = get_or_create_correlation_id(session);
71+
/// tracing::info!(correlation_id = %correlation_id, "Processing request");
72+
/// ```
73+
pub fn get_or_create_correlation_id(session: &Session) -> String {
74+
let req_header = session.req_header();
75+
76+
// Check for existing correlation ID headers (in order of preference)
77+
const CORRELATION_HEADERS: [&str; 3] = ["x-correlation-id", "x-request-id", "x-trace-id"];
78+
79+
for header_name in &CORRELATION_HEADERS {
80+
if let Some(value) = req_header.headers.get(*header_name) {
81+
if let Ok(id) = value.to_str() {
82+
return id.to_string();
83+
}
84+
}
85+
}
86+
87+
// Generate new correlation ID
88+
uuid::Uuid::new_v4().to_string()
89+
}
90+
91+
// ============================================================================
92+
// Response Helpers
93+
// ============================================================================
94+
95+
/// Write an HTTP response to a Pingora session
96+
///
97+
/// Handles the conversion from `http::Response<Full<Bytes>>` to Pingora's
98+
/// format and writes it to the session.
99+
///
100+
/// # Arguments
101+
///
102+
/// * `session` - The Pingora session to write to
103+
/// * `response` - The HTTP response to write
104+
/// * `keepalive_secs` - Keepalive timeout in seconds (None = disable keepalive)
105+
///
106+
/// # Returns
107+
///
108+
/// Returns `Ok(())` on success or an error if writing fails.
109+
///
110+
/// # Example
111+
///
112+
/// ```ignore
113+
/// let response = Response::builder()
114+
/// .status(200)
115+
/// .body(Full::new(Bytes::from("OK")))?;
116+
/// write_response(session, response, Some(60)).await?;
117+
/// ```
118+
pub async fn write_response(
119+
session: &mut Session,
120+
response: Response<Full<Bytes>>,
121+
keepalive_secs: Option<u64>,
122+
) -> Result<(), Box<Error>> {
123+
let status = response.status().as_u16();
124+
125+
// Collect headers to owned strings to avoid lifetime issues
126+
let headers_owned: Vec<(String, String)> = response
127+
.headers()
128+
.iter()
129+
.map(|(k, v)| {
130+
(
131+
k.as_str().to_string(),
132+
v.to_str().unwrap_or("").to_string(),
133+
)
134+
})
135+
.collect();
136+
137+
// Extract body bytes
138+
let full_body = response.into_body();
139+
let body_bytes: Bytes = BodyExt::collect(full_body)
140+
.await
141+
.map(|collected| collected.to_bytes())
142+
.unwrap_or_default();
143+
144+
// Build Pingora response header
145+
let mut resp_header = ResponseHeader::build(status, None)?;
146+
for (key, value) in headers_owned {
147+
resp_header.insert_header(key, &value)?;
148+
}
149+
150+
// Write response to session
151+
session.set_keepalive(keepalive_secs);
152+
session
153+
.write_response_header(Box::new(resp_header), false)
154+
.await?;
155+
session.write_response_body(Some(body_bytes), true).await?;
156+
157+
Ok(())
158+
}
159+
160+
/// Write an error response to a Pingora session
161+
///
162+
/// Convenience wrapper for error responses with status code, body, and content type.
163+
///
164+
/// # Arguments
165+
///
166+
/// * `session` - The Pingora session to write to
167+
/// * `status` - HTTP status code
168+
/// * `body` - Response body as string
169+
/// * `content_type` - Content-Type header value
170+
pub async fn write_error(
171+
session: &mut Session,
172+
status: u16,
173+
body: &str,
174+
content_type: &str,
175+
) -> Result<(), Box<Error>> {
176+
let mut resp_header = ResponseHeader::build(status, None)?;
177+
resp_header.insert_header("Content-Type", content_type)?;
178+
resp_header.insert_header("Content-Length", &body.len().to_string())?;
179+
180+
session.set_keepalive(None);
181+
session
182+
.write_response_header(Box::new(resp_header), false)
183+
.await?;
184+
session
185+
.write_response_body(Some(Bytes::copy_from_slice(body.as_bytes())), true)
186+
.await?;
187+
188+
Ok(())
189+
}
190+
191+
/// Write a plain text error response
192+
///
193+
/// Shorthand for `write_error` with `text/plain; charset=utf-8` content type.
194+
pub async fn write_text_error(
195+
session: &mut Session,
196+
status: u16,
197+
message: &str,
198+
) -> Result<(), Box<Error>> {
199+
write_error(session, status, message, "text/plain; charset=utf-8").await
200+
}
201+
202+
/// Write a JSON error response
203+
///
204+
/// Creates a JSON object with `error` and optional `message` fields.
205+
///
206+
/// # Example
207+
///
208+
/// ```ignore
209+
/// // Produces: {"error":"not_found","message":"Resource does not exist"}
210+
/// write_json_error(session, 404, "not_found", Some("Resource does not exist")).await?;
211+
/// ```
212+
pub async fn write_json_error(
213+
session: &mut Session,
214+
status: u16,
215+
error: &str,
216+
message: Option<&str>,
217+
) -> Result<(), Box<Error>> {
218+
let body = match message {
219+
Some(msg) => format!(r#"{{"error":"{}","message":"{}"}}"#, error, msg),
220+
None => format!(r#"{{"error":"{}"}}"#, error),
221+
};
222+
write_error(session, status, &body, "application/json").await
223+
}
224+
225+
// ============================================================================
226+
// Tests
227+
// ============================================================================
228+
229+
#[cfg(test)]
230+
mod tests {
231+
// Integration tests require mocking Pingora session.
232+
// See crates/proxy/tests/ for integration test examples.
233+
}

crates/proxy/src/main.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,11 @@ mod config;
2626
mod errors;
2727
mod health;
2828
mod http3;
29+
mod http_helpers;
2930
mod metrics;
3031
mod reload;
31-
mod request;
32-
mod response;
3332
mod routing;
3433
mod static_files;
35-
mod types;
3634
mod upstream;
3735
mod validation;
3836

@@ -300,7 +298,7 @@ impl SentinelProxy {
300298

301299
/// Get or generate correlation ID
302300
fn get_correlation_id(&self, session: &Session) -> String {
303-
request::get_or_create_correlation_id(session)
301+
http_helpers::get_or_create_correlation_id(session)
304302
}
305303

306304
/// Apply security headers
@@ -546,7 +544,7 @@ impl ProxyHttp for SentinelProxy {
546544
.method(req_header.method.clone())
547545
.uri(req_header.uri.clone())
548546
.body(())
549-
.unwrap();
547+
.expect("request builder with valid method and uri cannot fail");
550548
(path, static_req)
551549
};
552550

@@ -746,7 +744,7 @@ impl ProxyHttp for SentinelProxy {
746744
.method(method)
747745
.uri(uri)
748746
.body(())
749-
.unwrap(),
747+
.expect("request builder with valid method and uri cannot fail"),
750748
body_slice,
751749
&path,
752750
&ctx.correlation_id,

crates/proxy/src/request.rs

Lines changed: 0 additions & 61 deletions
This file was deleted.

0 commit comments

Comments
 (0)