Merge pull request #2140 from Urgau/axum-follow-up

Urgau · web-flow · commit 16c927e6a605 · 2025-08-08T16:37:41.000Z
Follow-up to the Axum migration
diff --git a/src/github/webhook.rs b/src/github/webhook.rs
@@ -172,8 +172,6 @@ pub async fn webhook(
         return (StatusCode::BAD_REQUEST, "Payload must be UTF-8").into_response();
     };
 
-    tracing::info!(?event, ?payload);
-
     match process_payload(event, payload, &ctx).await {
         Ok(true) => ("processed request",).into_response(),
         Ok(false) => ("ignored request",).into_response(),
diff --git a/src/interactions.rs b/src/interactions.rs
@@ -24,15 +24,20 @@ impl<'a> ErrorComment<'a> {
         }
     }
 
-    pub async fn post(&self, client: &GithubClient) -> anyhow::Result<Comment> {
+    pub fn markdown(message: &str) -> anyhow::Result<String> {
         let mut body = String::new();
-        writeln!(body, "**Error**: {}", self.message)?;
+        writeln!(body, "**Error**: {message}")?;
         writeln!(body)?;
         writeln!(
             body,
             "Please file an issue on GitHub at [triagebot](https://github.com/rust-lang/triagebot) if there's \
             a problem with this bot, or reach out on [#t-infra](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra) on Zulip."
         )?;
+        Ok(body)
+    }
+
+    pub async fn post(&self, client: &GithubClient) -> anyhow::Result<Comment> {
+        let body = Self::markdown(&self.message)?;
         self.issue.post_comment(client, &body).await
     }
 }
diff --git a/src/main.rs b/src/main.rs
@@ -4,7 +4,7 @@ use anyhow::Context as _;
 use axum::body::Body;
 use axum::error_handling::HandleErrorLayer;
 use axum::http::HeaderName;
-use axum::response::Html;
+use axum::response::{Html, Response};
 use axum::routing::{get, post};
 use axum::{BoxError, Router};
 use hyper::{Request, StatusCode};
@@ -132,6 +132,9 @@ async fn run_server(addr: SocketAddr) -> anyhow::Result<()> {
                 })
                 .on_request(|request: &Request<Body>, _span: &tracing::Span| {
                     tracing::info!(?request);
+                })
+                .on_response(|response: &Response<_>, dur, _span: &tracing::Span| {
+                    tracing::info!("response={} in {dur:?}", response.status());
                 }),
         )
         .layer(PropagateRequestIdLayer::new(X_REQUEST_ID))
diff --git a/src/zulip.rs b/src/zulip.rs
@@ -12,6 +12,7 @@ use crate::handlers::Context;
 use crate::handlers::docs_update::docs_update;
 use crate::handlers::pr_tracking::get_assigned_prs;
 use crate::handlers::project_goals::{self, ping_project_goals_owners};
+use crate::interactions::ErrorComment;
 use crate::utils::pluralize;
 use crate::zulip::api::{MessageApiResponse, Recipient};
 use crate::zulip::client::ZulipClient;
@@ -21,6 +22,7 @@ use crate::zulip::commands::{
 use anyhow::{Context as _, format_err};
 use axum::Json;
 use axum::extract::State;
+use axum::extract::rejection::JsonRejection;
 use axum::response::IntoResponse;
 use rust_team_data::v1::{TeamKind, TeamMember};
 use std::cmp::Reverse;
@@ -92,11 +94,24 @@ struct Response {
 /// Top-level handler for Zulip webhooks.
 ///
 /// Returns a JSON response or a 400 with an error message.
-// TODO: log JsonRejection
 pub async fn webhook(
     State(ctx): State<Arc<Context>>,
-    Json(req): Json<Request>,
+    req: Result<Json<Request>, JsonRejection>,
 ) -> axum::response::Response {
+    let Json(req) = match req {
+        Ok(req) => req,
+        Err(rejection) => {
+            tracing::error!(?rejection);
+            return Json(Response {
+                content: ErrorComment::markdown(
+                    "unable to handle this Zulip request: invalid JSON input",
+                )
+                .expect("creating a error message without fail"),
+            })
+            .into_response();
+        }
+    };
+
     tracing::info!(?req);
     let response = process_zulip_request(ctx, req).await;
     tracing::info!(?response);
@@ -111,9 +126,6 @@ pub async fn webhook(
             // We are mixing network errors and "logic" error (like clap errors)
             // so don't return a 500. Long term we should decouple those.
 
-            // Log the full error
-            tracing::error!(?err);
-
             // Reply with a 200 and reply only with outermost error
             Json(Response {
                 content: err.to_string(),
@@ -150,8 +162,6 @@ async fn process_zulip_request(ctx: Arc<Context>, req: Request) -> anyhow::Resul
         anyhow::bail!("Invalid authorization.");
     }
 
-    log::trace!("zulip hook: {req:?}");
-
     // Zulip commands are only available to users in the team database
     let gh_id = match ctx.team.zulip_to_github_id(req.message.sender_id).await {
         Ok(Some(gh_id)) => gh_id,

Original file line number	Diff line number	Diff line change
`@@ -24,15 +24,20 @@ impl<'a> ErrorComment<'a> {`
`24`	`24`	`}`
`25`	`25`	`}`
`26`	`26`
`27`		`- pub async fn post(&self, client: &GithubClient) -> anyhow::Result<Comment> {`
	`27`	`+ pub fn markdown(message: &str) -> anyhow::Result<String> {`
`28`	`28`	`let mut body = String::new();`
`29`		`- writeln!(body, "Error: {}", self.message)?;`
	`29`	`+ writeln!(body, "Error: {message}")?;`
`30`	`30`	`writeln!(body)?;`
`31`	`31`	`writeln!(`
`32`	`32`	`body,`
`33`	`33`	`"Please file an issue on GitHub at [triagebot](https://github.com/rust-lang/triagebot) if there's \`
`34`	`34`	`a problem with this bot, or reach out on [#t-infra](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra) on Zulip."`
`35`	`35`	`)?;`
	`36`	`+ Ok(body)`
	`37`	`+ }`
	`38`	`+`
	`39`	`+ pub async fn post(&self, client: &GithubClient) -> anyhow::Result<Comment> {`
	`40`	`+ let body = Self::markdown(&self.message)?;`
`36`	`41`	`self.issue.post_comment(client, &body).await`
`37`	`42`	`}`
`38`	`43`	`}`