Skip to content

Commit 1e396ac

Browse files
committed
Fix missing server timing in some cases
- Updated response handling to use a builder pattern for better clarity and consistency. - Enhanced server timing format to include microseconds in the output. - Added a new test for server timing in redirect responses to ensure proper header inclusion.
1 parent c38a109 commit 1e396ac

File tree

4 files changed

+61
-18
lines changed

4 files changed

+61
-18
lines changed

src/render.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,7 @@ impl HeaderContext {
243243
let link = get_object_str(data, "link")
244244
.with_context(|| "The redirect component requires a 'link' property")?;
245245
self.response.insert_header((header::LOCATION, link));
246-
let response = self.response.body(());
247-
Ok(response)
246+
Ok(self.into_response_builder().body(()))
248247
}
249248

250249
/// Answers to the HTTP request with a single json object
@@ -257,7 +256,9 @@ impl HeaderContext {
257256
} else {
258257
serde_json::to_vec(contents)?
259258
};
260-
Ok(PageContext::Close(self.response.body(json_response)))
259+
Ok(PageContext::Close(
260+
self.into_response_builder().body(json_response),
261+
))
261262
} else {
262263
let body_type = get_object_str(data, "type");
263264
let json_renderer = match body_type {
@@ -314,21 +315,20 @@ impl HeaderContext {
314315
}
315316
log::debug!("Authentication failed");
316317
// The authentication failed
317-
let http_response: HttpResponse = if let Some(link) = get_object_str(&data, "link") {
318+
if let Some(link) = get_object_str(&data, "link") {
318319
self.response
319320
.status(StatusCode::FOUND)
320-
.insert_header((header::LOCATION, link))
321-
.body(
322-
"Sorry, but you are not authorized to access this page. \
323-
Redirecting to the login page...",
324-
)
321+
.insert_header((header::LOCATION, link));
322+
self.has_status = true;
323+
Ok(PageContext::Close(self.into_response_builder().body(
324+
"Sorry, but you are not authorized to access this page. \
325+
Redirecting to the login page...",
326+
)))
325327
} else {
326328
anyhow::bail!(ErrorWithStatus {
327329
status: StatusCode::UNAUTHORIZED
328330
})
329-
};
330-
self.has_status = true;
331-
Ok(PageContext::Close(http_response))
331+
}
332332
}
333333

334334
fn download(mut self, options: &JsonValue) -> anyhow::Result<PageContext> {
@@ -359,7 +359,7 @@ impl HeaderContext {
359359
.insert_header((header::CONTENT_TYPE, content_type));
360360
}
361361
Ok(PageContext::Close(
362-
self.response.body(body_bytes.into_owned()),
362+
self.into_response_builder().body(body_bytes.into_owned()),
363363
))
364364
}
365365

@@ -374,6 +374,11 @@ impl HeaderContext {
374374
}
375375
}
376376

377+
fn into_response_builder(mut self) -> HttpResponseBuilder {
378+
self.add_server_timing_header();
379+
self.response
380+
}
381+
377382
async fn start_body(mut self, data: JsonValue) -> anyhow::Result<PageContext> {
378383
self.add_server_timing_header();
379384
let html_renderer =
@@ -388,9 +393,8 @@ impl HeaderContext {
388393
})
389394
}
390395

391-
pub fn close(mut self) -> HttpResponse {
392-
self.add_server_timing_header();
393-
self.response.finish()
396+
pub fn close(self) -> HttpResponse {
397+
self.into_response_builder().finish()
394398
}
395399
}
396400

src/webserver/server_timing.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ impl ServerTiming {
6161
if i > 0 {
6262
s.push_str(", ");
6363
}
64-
let millis = time.saturating_duration_since(last).as_millis();
65-
write!(&mut s, "{name};dur={millis}").ok()?;
64+
let micros = time.saturating_duration_since(last).as_micros();
65+
let millis = micros / 1000;
66+
let micros = micros % 1000;
67+
write!(&mut s, "{name};dur={millis}.{micros:03}").ok()?;
6668
last = *time;
6769
}
6870
Some(s)

tests/server_timing/mod.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,38 @@ async fn test_server_timing_format() -> actix_web::Result<()> {
9898

9999
Ok(())
100100
}
101+
102+
#[actix_web::test]
103+
async fn test_server_timing_in_redirect() -> actix_web::Result<()> {
104+
let mut config = test_config();
105+
config.environment = sqlpage::app_config::DevOrProd::Development;
106+
let app_data = make_app_data_from_config(config).await;
107+
108+
let req =
109+
crate::common::get_request_to_with_data("/tests/server_timing/redirect_test.sql", app_data)
110+
.await?
111+
.to_srv_request();
112+
let resp = main_handler(req).await?;
113+
114+
assert_eq!(
115+
resp.status(),
116+
StatusCode::FOUND,
117+
"Response should be a redirect"
118+
);
119+
let server_timing_header = resp
120+
.headers()
121+
.get("Server-Timing")
122+
.expect("Server-Timing header should be present in redirect responses");
123+
let header_value = server_timing_header.to_str().unwrap();
124+
125+
assert!(
126+
!header_value.is_empty(),
127+
"Server-Timing header should not be empty: {header_value}"
128+
);
129+
assert!(
130+
header_value.contains(";dur="),
131+
"Server-Timing header should contain timing events: {header_value}"
132+
);
133+
134+
Ok(())
135+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
select 'redirect' as component, '/destination.sql' as link;
2+

0 commit comments

Comments
 (0)