fix: check Gitea API response status and fallback for PR reviews (#47)
All checks were successful
All checks were successful
## Summary - Add HTTP response status checking to all Gitea tracker methods that were silently swallowing errors - Add fallback in create_pr_review: if inline comments fail, retry as plain PR comment ## Test plan - [ ] Deploy and trigger a PR review, check logs for actual error details - [ ] Verify fallback posts summary comment when inline comments fail Co-authored-by: Sharang Parnerkar <parnerkarsharang@gmail.com> Co-authored-by: Sharang Parnerkar <30073382+mighty840@users.noreply.github.com> Reviewed-on: #47
This commit was merged in pull request #47.
This commit is contained in:
6
Cargo.lock
generated
6
Cargo.lock
generated
@@ -4699,9 +4699,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rustls-webpki"
|
||||
version = "0.103.9"
|
||||
version = "0.103.10"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "d7df23109aa6c1567d1c575b9952556388da57401e4ace1d15f79eedad0d8f53"
|
||||
checksum = "df33b2b81ac578cabaf06b89b0631153a3f416b0a886e8a7a1707fb51abbd1ef"
|
||||
dependencies = [
|
||||
"ring",
|
||||
"rustls-pki-types",
|
||||
@@ -5171,7 +5171,7 @@ version = "0.8.9"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "c1c97747dbf44bb1ca44a561ece23508e99cb592e862f22222dcf42f51d1e451"
|
||||
dependencies = [
|
||||
"heck 0.5.0",
|
||||
"heck 0.4.1",
|
||||
"proc-macro2",
|
||||
"quote",
|
||||
"syn",
|
||||
|
||||
@@ -98,7 +98,8 @@ impl IssueTracker for GiteaTracker {
|
||||
_ => "open",
|
||||
};
|
||||
|
||||
self.http
|
||||
let resp = self
|
||||
.http
|
||||
.patch(&url)
|
||||
.header(
|
||||
"Authorization",
|
||||
@@ -109,6 +110,14 @@ impl IssueTracker for GiteaTracker {
|
||||
.await
|
||||
.map_err(|e| CoreError::IssueTracker(format!("Gitea update issue failed: {e}")))?;
|
||||
|
||||
if !resp.status().is_success() {
|
||||
let status = resp.status();
|
||||
let text = resp.text().await.unwrap_or_default();
|
||||
return Err(CoreError::IssueTracker(format!(
|
||||
"Gitea update issue returned {status}: {text}"
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -123,7 +132,8 @@ impl IssueTracker for GiteaTracker {
|
||||
"/repos/{owner}/{repo}/issues/{external_id}/comments"
|
||||
));
|
||||
|
||||
self.http
|
||||
let resp = self
|
||||
.http
|
||||
.post(&url)
|
||||
.header(
|
||||
"Authorization",
|
||||
@@ -134,6 +144,14 @@ impl IssueTracker for GiteaTracker {
|
||||
.await
|
||||
.map_err(|e| CoreError::IssueTracker(format!("Gitea add comment failed: {e}")))?;
|
||||
|
||||
if !resp.status().is_success() {
|
||||
let status = resp.status();
|
||||
let text = resp.text().await.unwrap_or_default();
|
||||
return Err(CoreError::IssueTracker(format!(
|
||||
"Gitea add comment returned {status}: {text}"
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -158,7 +176,8 @@ impl IssueTracker for GiteaTracker {
|
||||
})
|
||||
.collect();
|
||||
|
||||
self.http
|
||||
let resp = self
|
||||
.http
|
||||
.post(&url)
|
||||
.header(
|
||||
"Authorization",
|
||||
@@ -173,6 +192,48 @@ impl IssueTracker for GiteaTracker {
|
||||
.await
|
||||
.map_err(|e| CoreError::IssueTracker(format!("Gitea PR review failed: {e}")))?;
|
||||
|
||||
if !resp.status().is_success() {
|
||||
let status = resp.status();
|
||||
let text = resp.text().await.unwrap_or_default();
|
||||
|
||||
// If inline comments caused the failure, retry with just the summary body
|
||||
if !comments.is_empty() {
|
||||
tracing::warn!(
|
||||
"Gitea PR review with inline comments failed ({status}): {text}, retrying as plain comment"
|
||||
);
|
||||
let fallback_url = self.api_url(&format!(
|
||||
"/repos/{owner}/{repo}/issues/{pr_number}/comments"
|
||||
));
|
||||
let fallback_resp = self
|
||||
.http
|
||||
.post(&fallback_url)
|
||||
.header(
|
||||
"Authorization",
|
||||
format!("token {}", self.token.expose_secret()),
|
||||
)
|
||||
.json(&serde_json::json!({ "body": body }))
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| {
|
||||
CoreError::IssueTracker(format!("Gitea PR comment fallback failed: {e}"))
|
||||
})?;
|
||||
|
||||
if !fallback_resp.status().is_success() {
|
||||
let fb_status = fallback_resp.status();
|
||||
let fb_text = fallback_resp.text().await.unwrap_or_default();
|
||||
return Err(CoreError::IssueTracker(format!(
|
||||
"Gitea PR comment fallback returned {fb_status}: {fb_text}"
|
||||
)));
|
||||
}
|
||||
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
return Err(CoreError::IssueTracker(format!(
|
||||
"Gitea PR review returned {status}: {text}"
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -113,6 +113,72 @@ pub async fn add_mcp_server(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Probe each MCP server's health endpoint and update status in MongoDB.
|
||||
#[server]
|
||||
pub async fn refresh_mcp_status() -> Result<(), ServerFnError> {
|
||||
use chrono::Utc;
|
||||
use compliance_core::models::McpServerStatus;
|
||||
use mongodb::bson::doc;
|
||||
|
||||
let state: super::server_state::ServerState =
|
||||
dioxus_fullstack::FullstackContext::extract().await?;
|
||||
|
||||
let mut cursor = state
|
||||
.db
|
||||
.mcp_servers()
|
||||
.find(doc! {})
|
||||
.await
|
||||
.map_err(|e| ServerFnError::new(e.to_string()))?;
|
||||
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(std::time::Duration::from_secs(5))
|
||||
.build()
|
||||
.map_err(|e| ServerFnError::new(e.to_string()))?;
|
||||
|
||||
while cursor
|
||||
.advance()
|
||||
.await
|
||||
.map_err(|e| ServerFnError::new(e.to_string()))?
|
||||
{
|
||||
let server: compliance_core::models::McpServerConfig = cursor
|
||||
.deserialize_current()
|
||||
.map_err(|e| ServerFnError::new(e.to_string()))?;
|
||||
|
||||
let Some(oid) = server.id else { continue };
|
||||
|
||||
// Derive health URL from the endpoint (replace trailing /mcp with /health)
|
||||
let health_url = if server.endpoint_url.ends_with("/mcp") {
|
||||
format!(
|
||||
"{}health",
|
||||
&server.endpoint_url[..server.endpoint_url.len() - 3]
|
||||
)
|
||||
} else {
|
||||
format!("{}/health", server.endpoint_url.trim_end_matches('/'))
|
||||
};
|
||||
|
||||
let new_status = match client.get(&health_url).send().await {
|
||||
Ok(resp) if resp.status().is_success() => McpServerStatus::Running,
|
||||
_ => McpServerStatus::Stopped,
|
||||
};
|
||||
|
||||
let status_bson = match bson::to_bson(&new_status) {
|
||||
Ok(b) => b,
|
||||
Err(_) => continue,
|
||||
};
|
||||
|
||||
let _ = state
|
||||
.db
|
||||
.mcp_servers()
|
||||
.update_one(
|
||||
doc! { "_id": oid },
|
||||
doc! { "$set": { "status": status_bson, "updated_at": Utc::now().to_rfc3339() } },
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[server]
|
||||
pub async fn delete_mcp_server(server_id: String) -> Result<(), ServerFnError> {
|
||||
use mongodb::bson::doc;
|
||||
|
||||
@@ -5,7 +5,7 @@ use dioxus_free_icons::Icon;
|
||||
use crate::components::page_header::PageHeader;
|
||||
use crate::components::toast::{ToastType, Toasts};
|
||||
use crate::infrastructure::mcp::{
|
||||
add_mcp_server, delete_mcp_server, fetch_mcp_servers, regenerate_mcp_token,
|
||||
add_mcp_server, delete_mcp_server, fetch_mcp_servers, refresh_mcp_status, regenerate_mcp_token,
|
||||
};
|
||||
|
||||
#[component]
|
||||
@@ -22,6 +22,17 @@ pub fn McpServersPage() -> Element {
|
||||
let mut new_mongo_uri = use_signal(String::new);
|
||||
let mut new_mongo_db = use_signal(String::new);
|
||||
|
||||
// Probe health of all MCP servers on page load, then refresh the list
|
||||
let mut refreshing = use_signal(|| true);
|
||||
use_effect(move || {
|
||||
spawn(async move {
|
||||
refreshing.set(true);
|
||||
let _ = refresh_mcp_status().await;
|
||||
servers.restart();
|
||||
refreshing.set(false);
|
||||
});
|
||||
});
|
||||
|
||||
// Track which server's token is visible
|
||||
let mut visible_token: Signal<Option<String>> = use_signal(|| None);
|
||||
// Track which server is pending delete confirmation
|
||||
|
||||
@@ -41,7 +41,9 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
StreamableHttpServerConfig::default(),
|
||||
);
|
||||
|
||||
let router = axum::Router::new().nest_service("/mcp", service);
|
||||
let router = axum::Router::new()
|
||||
.route("/health", axum::routing::get(|| async { "ok" }))
|
||||
.nest_service("/mcp", service);
|
||||
let listener = tokio::net::TcpListener::bind(("0.0.0.0", port)).await?;
|
||||
tracing::info!("MCP HTTP server listening on 0.0.0.0:{port}");
|
||||
axum::serve(listener, router).await?;
|
||||
|
||||
Reference in New Issue
Block a user