fix(plugins): diagnostics crash minimales pour l'install de plugin (backend)
Écran noir hello-plugin (#120) récidivant après 3 fixes déjà mergés sans capturer la cause réelle : on ne pouvait pas savoir si le crash venait de l'install, du reconcile MCP ou d'un panic silencieux. Ajoute un panic hook qui logge thread/location/backtrace, trace les étapes install/reconcile/ asset-protocol dans idea.log, et remplace les `.expect()` du superviseur MCP plugin par une erreur typée au lieu d'un panic sur mutex empoisonné. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@ -47,6 +47,7 @@ use uuid::Uuid;
|
|||||||
use state::AppState;
|
use state::AppState;
|
||||||
|
|
||||||
static EXIT_GUARD_CONFIRMED: AtomicBool = AtomicBool::new(false);
|
static EXIT_GUARD_CONFIRMED: AtomicBool = AtomicBool::new(false);
|
||||||
|
static PANIC_HOOK_INSTALLED: AtomicBool = AtomicBool::new(false);
|
||||||
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
enum MainCloseAction {
|
enum MainCloseAction {
|
||||||
@ -69,6 +70,33 @@ fn should_install_exit_guard(window_label: &str) -> bool {
|
|||||||
window_label == "main"
|
window_label == "main"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn install_panic_diagnostics_hook() {
|
||||||
|
if PANIC_HOOK_INSTALLED.swap(true, Ordering::SeqCst) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let previous = std::panic::take_hook();
|
||||||
|
std::panic::set_hook(Box::new(move |info| {
|
||||||
|
let message = info
|
||||||
|
.payload()
|
||||||
|
.downcast_ref::<&str>()
|
||||||
|
.copied()
|
||||||
|
.or_else(|| info.payload().downcast_ref::<String>().map(String::as_str))
|
||||||
|
.unwrap_or("<non-string panic payload>");
|
||||||
|
let location = info
|
||||||
|
.location()
|
||||||
|
.map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column()))
|
||||||
|
.unwrap_or_else(|| "<unknown>".to_owned());
|
||||||
|
let thread = std::thread::current();
|
||||||
|
let thread_name = thread.name().unwrap_or("<unnamed>");
|
||||||
|
application::diag!("[panic] thread={thread_name} location={location} message={message}");
|
||||||
|
application::diag!(
|
||||||
|
"[panic] backtrace:\n{}",
|
||||||
|
std::backtrace::Backtrace::force_capture()
|
||||||
|
);
|
||||||
|
previous(info);
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
fn apply_main_close_decision(
|
fn apply_main_close_decision(
|
||||||
guard: application::AppExitWorkGuardState,
|
guard: application::AppExitWorkGuardState,
|
||||||
already_confirmed: bool,
|
already_confirmed: bool,
|
||||||
@ -151,6 +179,7 @@ pub fn run() {
|
|||||||
// click-launched AppImage (whose stderr is otherwise discarded). Best-effort:
|
// click-launched AppImage (whose stderr is otherwise discarded). Best-effort:
|
||||||
// if the file can't be opened the beacons simply stay on stderr.
|
// if the file can't be opened the beacons simply stay on stderr.
|
||||||
application::diag::set_log_path(app_data_dir.join("logs").join("idea.log"));
|
application::diag::set_log_path(app_data_dir.join("logs").join("idea.log"));
|
||||||
|
install_panic_diagnostics_hook();
|
||||||
application::diag!("[startup] IdeA launched; diagnostics log armed");
|
application::diag!("[startup] IdeA launched; diagnostics log armed");
|
||||||
let app_state = AppState::build_with_resource_dir(app_data_dir, resource_dir);
|
let app_state = AppState::build_with_resource_dir(app_data_dir, resource_dir);
|
||||||
|
|
||||||
|
|||||||
@ -57,12 +57,28 @@ pub async fn plugin_install_from_archive(
|
|||||||
path: String,
|
path: String,
|
||||||
state: State<'_, AppState>,
|
state: State<'_, AppState>,
|
||||||
) -> Result<PluginInstallResultDto, ErrorDto> {
|
) -> Result<PluginInstallResultDto, ErrorDto> {
|
||||||
state
|
application::diag!("[plugins] install archive start path={path}");
|
||||||
|
let result = state
|
||||||
.install_plugin_from_archive
|
.install_plugin_from_archive
|
||||||
.execute(path)
|
.execute(path)
|
||||||
.await
|
.await
|
||||||
.map(PluginInstallResultDto::from)
|
.map(PluginInstallResultDto::from)
|
||||||
.map_err(ErrorDto::from)
|
.map_err(ErrorDto::from);
|
||||||
|
match &result {
|
||||||
|
Ok(result) => application::diag!(
|
||||||
|
"[plugins] install archive ok plugin={} version={} hash={} lifecycle={:?}",
|
||||||
|
result.plugin.id,
|
||||||
|
result.plugin.version,
|
||||||
|
result.review.content_hash,
|
||||||
|
result.plugin.lifecycle_state
|
||||||
|
),
|
||||||
|
Err(err) => application::diag!(
|
||||||
|
"[plugins] install archive failed code={} message={}",
|
||||||
|
err.code,
|
||||||
|
err.message
|
||||||
|
),
|
||||||
|
}
|
||||||
|
result
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Installs a plugin from a local directory snapshot.
|
/// Installs a plugin from a local directory snapshot.
|
||||||
@ -71,12 +87,28 @@ pub async fn plugin_install_from_directory(
|
|||||||
path: String,
|
path: String,
|
||||||
state: State<'_, AppState>,
|
state: State<'_, AppState>,
|
||||||
) -> Result<PluginInstallResultDto, ErrorDto> {
|
) -> Result<PluginInstallResultDto, ErrorDto> {
|
||||||
state
|
application::diag!("[plugins] install directory start path={path}");
|
||||||
|
let result = state
|
||||||
.install_plugin_from_directory
|
.install_plugin_from_directory
|
||||||
.execute(path)
|
.execute(path)
|
||||||
.await
|
.await
|
||||||
.map(PluginInstallResultDto::from)
|
.map(PluginInstallResultDto::from)
|
||||||
.map_err(ErrorDto::from)
|
.map_err(ErrorDto::from);
|
||||||
|
match &result {
|
||||||
|
Ok(result) => application::diag!(
|
||||||
|
"[plugins] install directory ok plugin={} version={} hash={} lifecycle={:?}",
|
||||||
|
result.plugin.id,
|
||||||
|
result.plugin.version,
|
||||||
|
result.review.content_hash,
|
||||||
|
result.plugin.lifecycle_state
|
||||||
|
),
|
||||||
|
Err(err) => application::diag!(
|
||||||
|
"[plugins] install directory failed code={} message={}",
|
||||||
|
err.code,
|
||||||
|
err.message
|
||||||
|
),
|
||||||
|
}
|
||||||
|
result
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Enables or disables a plugin.
|
/// Enables or disables a plugin.
|
||||||
@ -145,13 +177,30 @@ pub fn plugin_asset_protocol(
|
|||||||
app: &AppHandle,
|
app: &AppHandle,
|
||||||
request: http::Request<Vec<u8>>,
|
request: http::Request<Vec<u8>>,
|
||||||
) -> Response<Vec<u8>> {
|
) -> Response<Vec<u8>> {
|
||||||
|
let uri = request.uri().to_string();
|
||||||
match plugin_asset_response(app, request) {
|
match plugin_asset_response(app, request) {
|
||||||
Ok(response) => response,
|
Ok(response) => {
|
||||||
Err((status, message)) => Response::builder()
|
application::diag!(
|
||||||
.status(status)
|
"[plugins] asset ok uri={} status={} bytes={}",
|
||||||
.header(header::CONTENT_TYPE, "text/plain; charset=utf-8")
|
uri,
|
||||||
.body(message.into_bytes())
|
response.status(),
|
||||||
.expect("valid protocol error response"),
|
response.body().len()
|
||||||
|
);
|
||||||
|
response
|
||||||
|
}
|
||||||
|
Err((status, message)) => {
|
||||||
|
application::diag!(
|
||||||
|
"[plugins] asset failed uri={uri} status={status} message={message}"
|
||||||
|
);
|
||||||
|
Response::builder()
|
||||||
|
.status(status)
|
||||||
|
.header(header::CONTENT_TYPE, "text/plain; charset=utf-8")
|
||||||
|
.body(message.into_bytes())
|
||||||
|
.unwrap_or_else(|e| {
|
||||||
|
application::diag!("[plugins] asset error response build failed: {e}");
|
||||||
|
Response::new(Vec::new())
|
||||||
|
})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -502,10 +502,18 @@ async fn install_from_staged(
|
|||||||
) -> Result<PluginInstallResult, AppError> {
|
) -> Result<PluginInstallResult, AppError> {
|
||||||
let review = review_staged(packages, validator, &staged).await?;
|
let review = review_staged(packages, validator, &staged).await?;
|
||||||
let plugin_id = review.manifest.id.clone();
|
let plugin_id = review.manifest.id.clone();
|
||||||
|
crate::diag!(
|
||||||
|
"[plugins] install staged reviewed plugin={} version={} hash={} source={}",
|
||||||
|
plugin_id.as_str(),
|
||||||
|
review.manifest.version.as_str(),
|
||||||
|
review.content_hash,
|
||||||
|
review.source.kind()
|
||||||
|
);
|
||||||
packages
|
packages
|
||||||
.commit_install(staged, &plugin_id)
|
.commit_install(staged, &plugin_id)
|
||||||
.await
|
.await
|
||||||
.map_err(map_store)?;
|
.map_err(map_store)?;
|
||||||
|
crate::diag!("[plugins] install committed plugin={}", plugin_id.as_str());
|
||||||
let mut registry = registry_store.load_registry().await.map_err(map_registry)?;
|
let mut registry = registry_store.load_registry().await.map_err(map_registry)?;
|
||||||
let mut entry = PluginRegistryEntry {
|
let mut entry = PluginRegistryEntry {
|
||||||
id: plugin_id.clone(),
|
id: plugin_id.clone(),
|
||||||
@ -530,6 +538,11 @@ async fn install_from_staged(
|
|||||||
.save_registry(®istry)
|
.save_registry(®istry)
|
||||||
.await
|
.await
|
||||||
.map_err(map_registry)?;
|
.map_err(map_registry)?;
|
||||||
|
crate::diag!(
|
||||||
|
"[plugins] install registry saved plugin={} lifecycle={:?}",
|
||||||
|
plugin_id.as_str(),
|
||||||
|
entry.lifecycle_state
|
||||||
|
);
|
||||||
events.publish(DomainEvent::PluginInstalled {
|
events.publish(DomainEvent::PluginInstalled {
|
||||||
plugin_id: plugin_id.clone(),
|
plugin_id: plugin_id.clone(),
|
||||||
version: review.manifest.version.clone(),
|
version: review.manifest.version.clone(),
|
||||||
@ -537,7 +550,21 @@ async fn install_from_staged(
|
|||||||
let (active_servers, invalid_plugins) =
|
let (active_servers, invalid_plugins) =
|
||||||
active_mcp_specs(packages, validator, ®istry).await?;
|
active_mcp_specs(packages, validator, ®istry).await?;
|
||||||
persist_invalid_runtime_plugins(registry_store, &mut registry, invalid_plugins).await;
|
persist_invalid_runtime_plugins(registry_store, &mut registry, invalid_plugins).await;
|
||||||
let _ = mcp.reconcile(active_servers).await;
|
let active_server_count = active_servers.len();
|
||||||
|
match mcp.reconcile(active_servers).await {
|
||||||
|
Ok(statuses) => crate::diag!(
|
||||||
|
"[plugins] install MCP reconcile ok plugin={} requested={} statuses={}",
|
||||||
|
plugin_id.as_str(),
|
||||||
|
active_server_count,
|
||||||
|
statuses.servers.len()
|
||||||
|
),
|
||||||
|
Err(err) => crate::diag!(
|
||||||
|
"[plugins] install MCP reconcile failed plugin={} requested={} error={}",
|
||||||
|
plugin_id.as_str(),
|
||||||
|
active_server_count,
|
||||||
|
err
|
||||||
|
),
|
||||||
|
}
|
||||||
let admin = admin_from_descriptor(
|
let admin = admin_from_descriptor(
|
||||||
PluginDescriptor {
|
PluginDescriptor {
|
||||||
manifest: review.manifest.clone(),
|
manifest: review.manifest.clone(),
|
||||||
@ -846,7 +873,19 @@ impl ReconcilePluginMcpServers {
|
|||||||
active_mcp_specs(self.packages.as_ref(), self.validator.as_ref(), ®istry).await?;
|
active_mcp_specs(self.packages.as_ref(), self.validator.as_ref(), ®istry).await?;
|
||||||
persist_invalid_runtime_plugins(self.registry.as_ref(), &mut registry, invalid_plugins)
|
persist_invalid_runtime_plugins(self.registry.as_ref(), &mut registry, invalid_plugins)
|
||||||
.await;
|
.await;
|
||||||
self.mcp.reconcile(specs).await.map_err(map_mcp)
|
let requested = specs.len();
|
||||||
|
let result = self.mcp.reconcile(specs).await.map_err(map_mcp);
|
||||||
|
match &result {
|
||||||
|
Ok(statuses) => crate::diag!(
|
||||||
|
"[plugins] MCP reconcile ok requested={} statuses={}",
|
||||||
|
requested,
|
||||||
|
statuses.servers.len()
|
||||||
|
),
|
||||||
|
Err(err) => {
|
||||||
|
crate::diag!("[plugins] MCP reconcile failed requested={requested} error={err}")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
result
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -5,7 +5,7 @@ use std::fs;
|
|||||||
use std::io::Read;
|
use std::io::Read;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use std::process::Stdio;
|
use std::process::Stdio;
|
||||||
use std::sync::{Arc, Mutex};
|
use std::sync::{Arc, Mutex, MutexGuard};
|
||||||
use std::time::{SystemTime, UNIX_EPOCH};
|
use std::time::{SystemTime, UNIX_EPOCH};
|
||||||
|
|
||||||
use async_trait::async_trait;
|
use async_trait::async_trait;
|
||||||
@ -386,6 +386,8 @@ trait ExternalMcpServerBridge: Send + Sync {
|
|||||||
) -> Result<Box<dyn ExternalMcpServerHandle>, PluginMcpError>;
|
) -> Result<Box<dyn ExternalMcpServerHandle>, PluginMcpError>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type ExternalMcpChildren = HashMap<String, Box<dyn ExternalMcpServerHandle>>;
|
||||||
|
|
||||||
struct ProcessMcpServerHandle {
|
struct ProcessMcpServerHandle {
|
||||||
child: Child,
|
child: Child,
|
||||||
}
|
}
|
||||||
@ -433,7 +435,7 @@ impl ExternalMcpServerBridge for StdioExternalMcpServerBridge {
|
|||||||
/// External process supervisor for plugin MCP servers.
|
/// External process supervisor for plugin MCP servers.
|
||||||
pub struct ExternalMcpPluginSupervisor {
|
pub struct ExternalMcpPluginSupervisor {
|
||||||
bridge: Arc<dyn ExternalMcpServerBridge>,
|
bridge: Arc<dyn ExternalMcpServerBridge>,
|
||||||
children: Mutex<HashMap<String, Box<dyn ExternalMcpServerHandle>>>,
|
children: Mutex<ExternalMcpChildren>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ExternalMcpPluginSupervisor {
|
impl ExternalMcpPluginSupervisor {
|
||||||
@ -453,6 +455,12 @@ impl ExternalMcpPluginSupervisor {
|
|||||||
children: Mutex::new(HashMap::new()),
|
children: Mutex::new(HashMap::new()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn children(&self) -> Result<MutexGuard<'_, ExternalMcpChildren>, PluginMcpError> {
|
||||||
|
self.children
|
||||||
|
.lock()
|
||||||
|
.map_err(|_| PluginMcpError::Process("plugin mcp supervisor mutex poisoned".to_owned()))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for ExternalMcpPluginSupervisor {
|
impl Default for ExternalMcpPluginSupervisor {
|
||||||
@ -469,10 +477,7 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor {
|
|||||||
) -> Result<PluginMcpStatusSet, PluginMcpError> {
|
) -> Result<PluginMcpStatusSet, PluginMcpError> {
|
||||||
let desired: HashSet<String> = active_servers.iter().map(|s| s.identity.clone()).collect();
|
let desired: HashSet<String> = active_servers.iter().map(|s| s.identity.clone()).collect();
|
||||||
let to_stop = {
|
let to_stop = {
|
||||||
let children = self
|
let children = self.children()?;
|
||||||
.children
|
|
||||||
.lock()
|
|
||||||
.expect("plugin mcp supervisor poisoned");
|
|
||||||
children
|
children
|
||||||
.keys()
|
.keys()
|
||||||
.filter(|id| !desired.contains(*id))
|
.filter(|id| !desired.contains(*id))
|
||||||
@ -480,22 +485,14 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor {
|
|||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
};
|
};
|
||||||
for id in to_stop {
|
for id in to_stop {
|
||||||
let child = self
|
let child = self.children()?.remove(&id);
|
||||||
.children
|
|
||||||
.lock()
|
|
||||||
.expect("plugin mcp supervisor poisoned")
|
|
||||||
.remove(&id);
|
|
||||||
if let Some(mut child) = child {
|
if let Some(mut child) = child {
|
||||||
let _ = child.stop().await;
|
let _ = child.stop().await;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
let mut statuses = Vec::new();
|
let mut statuses = Vec::new();
|
||||||
for spec in active_servers {
|
for spec in active_servers {
|
||||||
let already = self
|
let already = self.children()?.contains_key(&spec.identity);
|
||||||
.children
|
|
||||||
.lock()
|
|
||||||
.expect("plugin mcp supervisor poisoned")
|
|
||||||
.contains_key(&spec.identity);
|
|
||||||
if already {
|
if already {
|
||||||
statuses.push(PluginMcpStatus {
|
statuses.push(PluginMcpStatus {
|
||||||
identity: spec.identity,
|
identity: spec.identity,
|
||||||
@ -506,10 +503,12 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor {
|
|||||||
}
|
}
|
||||||
match self.bridge.start(&spec).await {
|
match self.bridge.start(&spec).await {
|
||||||
Ok(handle) => {
|
Ok(handle) => {
|
||||||
self.children
|
match self.children() {
|
||||||
.lock()
|
Ok(mut children) => {
|
||||||
.expect("plugin mcp supervisor poisoned")
|
children.insert(spec.identity.clone(), handle);
|
||||||
.insert(spec.identity.clone(), handle);
|
}
|
||||||
|
Err(err) => return Err(err),
|
||||||
|
}
|
||||||
statuses.push(PluginMcpStatus {
|
statuses.push(PluginMcpStatus {
|
||||||
identity: spec.identity,
|
identity: spec.identity,
|
||||||
running: true,
|
running: true,
|
||||||
@ -529,10 +528,7 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor {
|
|||||||
async fn stop_plugin(&self, plugin_id: &PluginId) -> Result<(), PluginMcpError> {
|
async fn stop_plugin(&self, plugin_id: &PluginId) -> Result<(), PluginMcpError> {
|
||||||
let prefix = format!("plugin:{}:", plugin_id.as_str());
|
let prefix = format!("plugin:{}:", plugin_id.as_str());
|
||||||
let ids = {
|
let ids = {
|
||||||
let children = self
|
let children = self.children()?;
|
||||||
.children
|
|
||||||
.lock()
|
|
||||||
.expect("plugin mcp supervisor poisoned");
|
|
||||||
children
|
children
|
||||||
.keys()
|
.keys()
|
||||||
.filter(|id| id.starts_with(&prefix))
|
.filter(|id| id.starts_with(&prefix))
|
||||||
@ -540,11 +536,7 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor {
|
|||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
};
|
};
|
||||||
for id in ids {
|
for id in ids {
|
||||||
let child = self
|
let child = self.children()?.remove(&id);
|
||||||
.children
|
|
||||||
.lock()
|
|
||||||
.expect("plugin mcp supervisor poisoned")
|
|
||||||
.remove(&id);
|
|
||||||
if let Some(mut child) = child {
|
if let Some(mut child) = child {
|
||||||
child.stop().await?;
|
child.stop().await?;
|
||||||
}
|
}
|
||||||
@ -751,4 +743,20 @@ mod tests {
|
|||||||
assert_eq!(statuses.servers[0].identity, other.identity);
|
assert_eq!(statuses.servers[0].identity, other.identity);
|
||||||
assert_eq!(bridge.started.lock().unwrap().len(), 2);
|
assert_eq!(bridge.started.lock().unwrap().len(), 2);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn supervisor_poisoned_registry_returns_typed_error() {
|
||||||
|
let supervisor = ExternalMcpPluginSupervisor::new();
|
||||||
|
let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||||
|
let _guard = supervisor.children.lock().unwrap();
|
||||||
|
panic!("poison plugin mcp supervisor registry");
|
||||||
|
}));
|
||||||
|
|
||||||
|
let err = supervisor.reconcile(Vec::new()).await.unwrap_err();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
err,
|
||||||
|
PluginMcpError::Process("plugin mcp supervisor mutex poisoned".to_owned())
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user