refactor: improve error handling and logging in plugin and account services

- Added detailed error messages for plugin data read/write failures, including file paths for better debugging.
- Updated logging levels for upstream request rejections in account services to debug for less critical issues.
- Enhanced error handling in the plugin worker to provide clearer context when starting the plugin worker fails.
- Introduced new functions for merging extra parameters and applying body allowlists in provider services, improving request validation.
This commit is contained in:
leookun
2026-08-30 21:10:08 +08:00
parent 43a18377b6
commit 3a2d47954e
5 changed files with 98 additions and 65 deletions
+1 -1
View File
@@ -254,7 +254,7 @@ async fn forward_or(
match proxy::forward_buffered(&upstream, request).await { match proxy::forward_buffered(&upstream, request).await {
Ok(response) if response.status.is_success() => Ok(response.into_response()), Ok(response) if response.status.is_success() => Ok(response.into_response()),
Ok(response) => { Ok(response) => {
tracing::warn!(status = %response.status, "Cursor identity upstream rejected request; using local identity"); tracing::debug!(status = %response.status, "Cursor identity upstream rejected request; using local identity");
fallback() fallback()
} }
Err(error) => { Err(error) => {
+2
View File
@@ -57,6 +57,8 @@ impl IntoResponse for Error {
| Self::Encode(_) | Self::Encode(_)
| Self::Io(_) => StatusCode::INTERNAL_SERVER_ERROR, | Self::Io(_) => StatusCode::INTERNAL_SERVER_ERROR,
}; };
// 所有回给 UI 的错误统一落日志,否则失败原因只出现在前端提示里。
tracing::warn!(%status, error = %self, "request failed");
let code = match status { let code = match status {
StatusCode::BAD_REQUEST => "invalid_argument", StatusCode::BAD_REQUEST => "invalid_argument",
StatusCode::NOT_FOUND => "not_found", StatusCode::NOT_FOUND => "not_found",
+35 -9
View File
@@ -39,12 +39,15 @@ impl PluginDataStore {
let path = self.path(plugin_id, key)?; let path = self.path(plugin_id, key)?;
let lock = self.lock(plugin_id); let lock = self.lock(plugin_id);
let _guard = lock.lock().await; let _guard = lock.lock().await;
match tokio::fs::read(path).await { match tokio::fs::read(&path).await {
Ok(bytes) => Ok(serde_json::from_slice(&bytes)?), Ok(bytes) => Ok(serde_json::from_slice(&bytes)?),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => { Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
Ok(serde_json::Value::Null) Ok(serde_json::Value::Null)
} }
Err(error) => Err(error.into()), Err(error) => Err(Error::Config(format!(
"plugin data read failed at {}: {error}",
path.display()
))),
} }
} }
@@ -57,6 +60,18 @@ impl PluginDataStore {
let path = self.path(plugin_id, key)?; let path = self.path(plugin_id, key)?;
let lock = self.lock(plugin_id); let lock = self.lock(plugin_id);
let _guard = lock.lock().await; let _guard = lock.lock().await;
self.write_locked(&path, key, value)
.await
// 带上具体路径,Windows 上的拒绝访问才能定位到是哪一步。
.map_err(|error| {
Error::Config(format!(
"plugin data write failed at {}: {error}",
path.display()
))
})
}
async fn write_locked(&self, path: &Path, key: &str, value: &serde_json::Value) -> Result<()> {
let directory = path.parent().expect("plugin data path has a parent"); let directory = path.parent().expect("plugin data path has a parent");
tokio::fs::create_dir_all(directory).await?; tokio::fs::create_dir_all(directory).await?;
set_directory_permissions(directory)?; set_directory_permissions(directory)?;
@@ -70,8 +85,8 @@ impl PluginDataStore {
.await?; .await?;
file.sync_all().await?; file.sync_all().await?;
drop(file); drop(file);
replace_file(&temporary, &path).await?; replace_file(&temporary, path).await?;
set_file_permissions(&path)?; set_file_permissions(path)?;
Ok(()) Ok(())
} }
@@ -80,10 +95,13 @@ impl PluginDataStore {
let lock = self.lock(plugin_id); let lock = self.lock(plugin_id);
let _guard = lock.lock().await; let _guard = lock.lock().await;
let path = self.root.join(plugin_id); let path = self.root.join(plugin_id);
match tokio::fs::remove_dir_all(path).await { match tokio::fs::remove_dir_all(&path).await {
Ok(()) => Ok(()), Ok(()) => Ok(()),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()),
Err(error) => Err(error.into()), Err(error) => Err(Error::Config(format!(
"plugin data cleanup failed at {}: {error}",
path.display()
))),
} }
} }
@@ -117,16 +135,24 @@ async fn replace_file(temporary: &Path, path: &Path) -> Result<()> {
match tokio::fs::rename(temporary, path).await { match tokio::fs::rename(temporary, path).await {
Ok(()) => return Ok(()), Ok(()) => return Ok(()),
Err(error) Err(error)
if attempts < 10 if attempts < 20
&& matches!( && matches!(
error.raw_os_error(), error.raw_os_error(),
Some(ACCESS_DENIED | SHARING_VIOLATION) Some(ACCESS_DENIED | SHARING_VIOLATION)
) => ) =>
{ {
attempts += 1; attempts += 1;
tokio::time::sleep(std::time::Duration::from_millis(50)).await; tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
Err(error) => {
tracing::warn!(
path = %path.display(),
attempts,
%error,
"plugin data file replacement failed"
);
return Err(error.into());
} }
Err(error) => return Err(error.into()),
} }
} }
} }
+6 -1
View File
@@ -250,7 +250,12 @@ impl PluginWorker {
.stdout(Stdio::piped()) .stdout(Stdio::piped())
.stderr(Stdio::piped()) .stderr(Stdio::piped())
.kill_on_drop(true); .kill_on_drop(true);
let mut child = command.spawn()?; let mut child = command.spawn().map_err(|error| {
Error::Config(format!(
"cannot start plugin worker {}: {error}",
self.inner.executable.display()
))
})?;
let stdin = let stdin =
Arc::new(Mutex::new(child.stdin.take().ok_or_else(|| { Arc::new(Mutex::new(child.stdin.take().ok_or_else(|| {
Error::Config("cannot open plugin worker stdin".into()) Error::Config("cannot open plugin worker stdin".into())
+54 -54
View File
@@ -78,6 +78,60 @@ fn provider_event_error(label: &str, value: &serde_json::Value) -> Option<crate:
Some(crate::Error::Provider(format!("{label} error: {message}"))) Some(crate::Error::Provider(format!("{label} error: {message}")))
} }
fn merge_extra_params(body: &mut serde_json::Value, extra: &serde_json::Value) -> Result<()> {
let extra = extra
.as_object()
.ok_or_else(|| crate::Error::Config("model extra params must be an object".into()))?;
let body = body
.as_object_mut()
.ok_or_else(|| crate::Error::Provider("provider request body must be an object".into()))?;
for (name, value) in extra {
if matches!(
name.as_str(),
"model"
| "stream"
| "messages"
| "input"
| "tools"
| "system"
| "instructions"
| "prompt_cache_key"
) {
return Err(crate::Error::Config(format!(
"model extra params cannot replace {name}"
)));
}
body.insert(name.clone(), value.clone());
}
Ok(())
}
fn apply_body_allowlist(
body: &mut serde_json::Value,
allowed: Option<&std::collections::HashSet<String>>,
) -> Result<()> {
let Some(allowed) = allowed else {
return Ok(());
};
body.as_object_mut()
.ok_or_else(|| crate::Error::Provider("provider request body must be an object".into()))?
.retain(|name, _| allowed.contains(name));
Ok(())
}
fn apply_openai_prompt_cache_key(body: &mut serde_json::Value, model_id: &str) -> Result<()> {
if !model_id.to_ascii_lowercase().contains("gpt") {
return Ok(());
}
body.as_object_mut()
.ok_or_else(|| crate::Error::Provider("provider request body must be an object".into()))?
.insert(
"prompt_cache_key".into(),
serde_json::Value::String("cursor-byok".into()),
);
Ok(())
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
@@ -148,57 +202,3 @@ mod tests {
assert_eq!(message, expected); assert_eq!(message, expected);
} }
} }
fn merge_extra_params(body: &mut serde_json::Value, extra: &serde_json::Value) -> Result<()> {
let extra = extra
.as_object()
.ok_or_else(|| crate::Error::Config("model extra params must be an object".into()))?;
let body = body
.as_object_mut()
.ok_or_else(|| crate::Error::Provider("provider request body must be an object".into()))?;
for (name, value) in extra {
if matches!(
name.as_str(),
"model"
| "stream"
| "messages"
| "input"
| "tools"
| "system"
| "instructions"
| "prompt_cache_key"
) {
return Err(crate::Error::Config(format!(
"model extra params cannot replace {name}"
)));
}
body.insert(name.clone(), value.clone());
}
Ok(())
}
fn apply_body_allowlist(
body: &mut serde_json::Value,
allowed: Option<&std::collections::HashSet<String>>,
) -> Result<()> {
let Some(allowed) = allowed else {
return Ok(());
};
body.as_object_mut()
.ok_or_else(|| crate::Error::Provider("provider request body must be an object".into()))?
.retain(|name, _| allowed.contains(name));
Ok(())
}
fn apply_openai_prompt_cache_key(body: &mut serde_json::Value, model_id: &str) -> Result<()> {
if !model_id.to_ascii_lowercase().contains("gpt") {
return Ok(());
}
body.as_object_mut()
.ok_or_else(|| crate::Error::Provider("provider request body must be an object".into()))?
.insert(
"prompt_cache_key".into(),
serde_json::Value::String("cursor-byok".into()),
);
Ok(())
}