Skip to content

wip feat: improve middleware performance by 50% #1209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sansyrox wants to merge 1 commit into
base: main
Choose a base branch
from
Open

wip feat: improve middleware performance by 50% #1209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions src/executors/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,89 @@ pub async fn execute_after_middleware_function(
}
}

// Execute a chain of before middleware functions with batched GIL acquisition
#[inline]
pub async fn execute_before_middleware_chain(
input: &Request,
middlewares: &[FunctionInfo],
) -> Result<MiddlewareReturn> {
let mut current_request = input.clone();

// Check if all middlewares are sync to optimize GIL usage
let all_sync = middlewares.iter().all(|m| !m.is_async);

if all_sync {
// Execute all sync middlewares in a single GIL acquisition
Python::with_gil(|py| -> Result<MiddlewareReturn> {
for middleware in middlewares {
let output = get_function_output(middleware, py, &current_request)?;

// Try response extraction first, then request
match output.extract::<Response>() {
Ok(response) => return Ok(MiddlewareReturn::Response(response)),
Err(_) => match output.extract::<Request>() {
Ok(request) => current_request = request,
Err(e) => return Err(e.into()),
},
}
}

Ok(MiddlewareReturn::Request(current_request))
})
} else {
// Fall back to individual execution for mixed sync/async middlewares
for middleware in middlewares {
current_request = match execute_middleware_function(&current_request, middleware).await? {
MiddlewareReturn::Request(r) => r,
MiddlewareReturn::Response(r) => return Ok(MiddlewareReturn::Response(r)),
};
}

Ok(MiddlewareReturn::Request(current_request))
}
}

// Execute a chain of after middleware functions with batched GIL acquisition
#[inline]
pub async fn execute_after_middleware_chain(
input: &Response,
middlewares: &[FunctionInfo],
) -> Result<MiddlewareReturn> {
let mut current_response = input.clone();

// Check if all middlewares are sync to optimize GIL usage
let all_sync = middlewares.iter().all(|m| !m.is_async);

if all_sync {
// Execute all sync middlewares in a single GIL acquisition
Python::with_gil(|py| -> Result<MiddlewareReturn> {
for middleware in middlewares {
let output = get_function_output(middleware, py, &current_response)?;

// After middleware should return Response
match output.extract::<Response>() {
Ok(response) => current_response = response,
Err(e) => return Err(e.into()),
}
}

Ok(MiddlewareReturn::Response(current_response))
})
} else {
// Fall back to individual execution for mixed sync/async middlewares
for middleware in middlewares {
current_response = match execute_middleware_function(&current_response, middleware).await? {
MiddlewareReturn::Response(r) => r,
MiddlewareReturn::Request(_) => {
return Err(anyhow::anyhow!("After middleware returned a request"))
}
};
}

Ok(MiddlewareReturn::Response(current_response))
Comment on lines +263 to +298

@coderabbitai coderabbitai Bot Mar 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Preserve the (request, response) contract for after middleware.

This chain runner only passes a Response: the sync path calls get_function_output(...), and the async path calls execute_middleware_function(...). That regresses the behavior implemented by execute_after_middleware_function at Lines 181-217, which passes both request and response. Any after_request(req, resp) middleware will now fail at runtime, and those callbacks can no longer inspect request-scoped data such as path params.

🔧 Suggested fix 
-pub async fn execute_after_middleware_chain(
-    input: &Response,
+pub async fn execute_after_middleware_chain(
+    request: &Request,
+    input: &Response,
     middlewares: &[FunctionInfo],
 ) -> Result<MiddlewareReturn> {
     let mut current_response = input.clone();
@@
     if all_sync {
         Python::with_gil(|py| -> Result<MiddlewareReturn> {
             for middleware in middlewares {
-                let output = get_function_output(middleware, py, &current_response)?;
+                let output = get_function_output_with_two_args(
+                    middleware,
+                    py,
+                    request,
+                    &current_response,
+                )?;
@@
     } else {
         for middleware in middlewares {
-            current_response = match execute_middleware_function(&current_response, middleware).await? {
+            current_response = match execute_after_middleware_function(
+                request,
+                &current_response,
+                middleware,
+            )
+            .await? {
                 MiddlewareReturn::Response(r) => r,
                 MiddlewareReturn::Request(_) => {
                     return Err(anyhow::anyhow!("After middleware returned a request"))
                 }
             };
         }

src/server.rs will need to pass &request into this helper as well.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/executors/mod.rs` around lines 263 - 298, The after-middleware chain
currently only forwards a Response, breaking the (request, response) contract
used by execute_after_middleware_function; update execute_after_middleware_chain
to accept and pass both &Request and &Response to middleware invocations (use
get_function_output and execute_middleware_function with both references so
after_request(req, resp) callbacks can access request-scoped data), ensure you
handle the returned MiddlewareReturn the same way (only accept
MiddlewareReturn::Response) and update the caller in server.rs to pass &request
into this helper accordingly.

@sansyrox sansyrox Apr 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve — noted, WIP PR. Will fix before merge.

@coderabbitai coderabbitai Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate Limit Exceeded

@sansyrox have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 37 seconds before sending another message.

@coderabbitai coderabbitai Bot May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

}
}

#[inline]
pub async fn execute_http_function(
request: &Request,
Expand Down
124 changes: 57 additions & 67 deletions src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::executors::{
execute_after_middleware_function, execute_http_function, execute_middleware_function,
execute_after_middleware_chain, execute_before_middleware_chain, execute_http_function,
execute_startup_handler,
};

Expand Down Expand Up @@ -503,42 +503,37 @@ async fn index(
let route = format!("{}{}", req.method(), request.url.path);

// Before middleware
let before_middlewares =
let mut before_middlewares =
middleware_router.get_global_middlewares(&MiddlewareType::BeforeRequest);
let route_before = middleware_router.get_route(&MiddlewareType::BeforeRequest, &route);

let mut early_response: Option<Response> = None;
if !before_middlewares.is_empty() || route_before.is_some() {
let mut all_before = before_middlewares;
if let Some((function, route_params)) = route_before {
all_before.push(function);
request.path_params = route_params;
}
for before_middleware in all_before {
request = match execute_middleware_function(&request, &before_middleware).await {
Ok(MiddlewareReturn::Request(r)) => r,
Ok(MiddlewareReturn::Response(r)) => {
early_response = Some(r);
break;
}
Err(e) => {
let msg = match e.downcast_ref::<PyErr>() {
Some(py_err) => get_traceback(py_err),
None => format!("{e:?}"),
};
error!(
"Error executing before middleware for `{}`: {}",
request.url.path, msg
);
return ResponseType::Standard(Response::internal_server_error(None));
}
};
}
if let Some((function, route_params)) = route_before {
before_middlewares.push(function);
request.path_params = route_params;
}

// Execute all before middlewares with batched GIL acquisition
if !before_middlewares.is_empty() {
request = match execute_before_middleware_chain(&request, &before_middlewares).await {
Ok(MiddlewareReturn::Request(r)) => r,
Ok(MiddlewareReturn::Response(r)) => {
return ResponseType::Standard(r);
Comment on lines +517 to +520

@coderabbitai coderabbitai Bot Mar 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid returning short-circuited before-middleware responses here.

This exits index() before the common response path at Lines 567-609, so responses produced by before middleware no longer receive global response headers or the after-middleware chain. That will show up on auth/preflight-style short circuits as missing CORS/security headers. If you keep these responses on the normal path, the chain result also needs to preserve the latest mutated Request.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/server.rs` around lines 517 - 520, The current early return in index()
when execute_before_middleware_chain returns MiddlewareReturn::Response(r) skips
global response header application and the after-middleware chain; instead,
change the handling so index() continues down the common response path: if
execute_before_middleware_chain yields MiddlewareReturn::Response, store that
Response as the "short-circuit" response while also preserving the most-recent
mutated Request (from MiddlewareReturn::Request or a sidecar), then proceed
through the same global header application and execute_after_middleware_chain on
the response before returning; update the match on
execute_before_middleware_chain (and any variables around before_middlewares) to
propagate both the latest Request and the Response into the standard response
flow rather than returning ResponseType::Standard immediately.

@sansyrox sansyrox Apr 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve — noted, will address before merge.

@coderabbitai coderabbitai Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate Limit Exceeded

@sansyrox have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 52 seconds before sending another message.

}
Err(e) => {
let msg = match e.downcast_ref::<PyErr>() {
Some(py_err) => get_traceback(py_err),
None => format!("{e:?}"),
};
error!(
"Error executing before middleware for `{}`: {}",
request.url.path, msg
);
return ResponseType::Standard(Response::internal_server_error(None));
}
};
}

let mut response = if let Some(r) = early_response {
ResponseType::Standard(r)
} else if let Some(cached) = const_router.get_cached_route(&http_method, &request.url.path) {
let mut response = if let Some(cached) = const_router.get_cached_route(&http_method, &request.url.path) {
let mut resp = Response {
status_code: cached.status.as_u16(),
response_type: "text".to_string(),
Expand Down Expand Up @@ -579,43 +574,38 @@ async fn index(
}

// After middleware
let after_middlewares = middleware_router.get_global_middlewares(&MiddlewareType::AfterRequest);
let mut after_middlewares =
middleware_router.get_global_middlewares(&MiddlewareType::AfterRequest);
let route_after = middleware_router.get_route(&MiddlewareType::AfterRequest, &route);

if !after_middlewares.is_empty() || route_after.is_some() {
let mut all_after = after_middlewares;
if let Some((function, _)) = route_after {
all_after.push(function);
}
for after_middleware in all_after {
if let ResponseType::Standard(std_response) = response {
response = match execute_after_middleware_function(
&request,
&std_response,
&after_middleware,
)
.await
{
Ok(MiddlewareReturn::Request(_)) => {
error!("After middleware returned a request");
return ResponseType::Standard(Response::internal_server_error(None));
}
Ok(MiddlewareReturn::Response(r)) => ResponseType::Standard(r),
Err(e) => {
let msg = match e.downcast_ref::<PyErr>() {
Some(py_err) => get_traceback(py_err),
None => format!("{e:?}"),
};
error!(
"Error executing after middleware for `{}`: {}",
request.url.path, msg
);
return ResponseType::Standard(Response::internal_server_error(Some(
&std_response.headers,
)));
}
};
}
if let Some((function, _)) = route_after {
after_middlewares.push(function);
}

// Execute all after middlewares with batched GIL acquisition
if !after_middlewares.is_empty() {
if let ResponseType::Standard(std_response) = response {
response = match execute_after_middleware_chain(&std_response, &after_middlewares).await
{
Ok(MiddlewareReturn::Request(_)) => {
error!("After middleware returned a request");
return ResponseType::Standard(Response::internal_server_error(None));
}
Ok(MiddlewareReturn::Response(r)) => ResponseType::Standard(r),
Err(e) => {
let msg = match e.downcast_ref::<PyErr>() {
Some(py_err) => get_traceback(py_err),
None => format!("{e:?}"),
};
error!(
"Error executing after middleware for `{}`: {}",
request.url.path, msg
);
return ResponseType::Standard(Response::internal_server_error(Some(
&std_response.headers,
)));
}
};
}
}

Expand Down
Loading