mirror of
https://github.com/openai/codex.git
synced 2026-08-26 13:38:49 +00:00
Report detailed session import error types (#33863)
## What changed - Add stable `sub_error_type` values for external-agent session import failures, including detection, preparation, configuration, thread storage, and ledger update errors. - Preserve these values through import results and analytics while leaving unrelated plugin and memory errors unchanged. ## Testing - Verify that a missing session reports `session_not_detected` in both the completed import result and its analytics event. GitOrigin-RevId: cbf1f52df4bb1ac107442c4afa0b130fa32ad0a5
This commit is contained in:
committed by
copyberry
parent
20465d30ae
commit
ab0f71aed6
@@ -270,6 +270,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"plugin_import",
|
||||
/*sub_error_type*/ None,
|
||||
error.to_string(),
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -400,6 +401,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"session_missing",
|
||||
Some("session_not_detected"),
|
||||
format!(
|
||||
"external agent session was not detected for import: {}",
|
||||
session.path.display()
|
||||
@@ -412,6 +414,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"session_source_path",
|
||||
Some("failed_to_resolve_session_source_path"),
|
||||
err.to_string(),
|
||||
Some(session.path.display().to_string()),
|
||||
);
|
||||
|
||||
@@ -85,6 +85,7 @@ impl ExternalAgentSessionImporter {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"session_permit",
|
||||
Some("failed_to_acquire_import_permit"),
|
||||
"external agent session import permit could not be acquired",
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -114,11 +115,18 @@ impl ExternalAgentSessionImporter {
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(failure) => {
|
||||
let SessionImportFailure {
|
||||
source_path,
|
||||
message,
|
||||
stage,
|
||||
sub_error_type,
|
||||
} = failure;
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
failure.stage,
|
||||
failure.message.clone(),
|
||||
Some(failure.source_path.display().to_string()),
|
||||
stage,
|
||||
Some(sub_error_type),
|
||||
message,
|
||||
Some(source_path.display().to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -141,6 +149,7 @@ impl ExternalAgentSessionImporter {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"session_connector_detection_task",
|
||||
Some("session_connector_detection_task_failed"),
|
||||
err.to_string(),
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -163,6 +172,7 @@ impl ExternalAgentSessionImporter {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"session_ledger_update",
|
||||
Some("failed_to_update_session_ledger"),
|
||||
err.to_string(),
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -179,10 +189,11 @@ impl ExternalAgentSessionImporter {
|
||||
let Some(pending_import) = self
|
||||
.prepare_session_import(session, metadata_mode)
|
||||
.await
|
||||
.map_err(|message| SessionImportFailure {
|
||||
.map_err(|failure| SessionImportFailure {
|
||||
source_path: source_path.clone(),
|
||||
message,
|
||||
message: failure.message,
|
||||
stage: "session_prepare",
|
||||
sub_error_type: failure.sub_error_type,
|
||||
})?
|
||||
else {
|
||||
return Ok(None);
|
||||
@@ -200,10 +211,11 @@ impl ExternalAgentSessionImporter {
|
||||
let imported_thread_id =
|
||||
self.persist_session(pending_import.session)
|
||||
.await
|
||||
.map_err(|message| SessionImportFailure {
|
||||
.map_err(|failure| SessionImportFailure {
|
||||
source_path: pending_import.source_path.clone(),
|
||||
message,
|
||||
message: failure.message,
|
||||
stage: "session_persist",
|
||||
sub_error_type: failure.sub_error_type,
|
||||
})?;
|
||||
Ok(Some(CompletedSessionImport {
|
||||
import: CompletedExternalAgentSessionImport {
|
||||
@@ -220,20 +232,30 @@ impl ExternalAgentSessionImporter {
|
||||
&self,
|
||||
session: ExternalAgentSessionMigration,
|
||||
metadata_mode: SessionMetadataMode,
|
||||
) -> Result<Option<PendingSessionImport>, String> {
|
||||
) -> Result<Option<PendingSessionImport>, SessionImportStepFailure> {
|
||||
let codex_home = self.codex_home.clone();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
prepare_validated_session_import_with_metadata_mode(&codex_home, session, metadata_mode)
|
||||
})
|
||||
.await
|
||||
.map_err(|err| format!("external agent session preparation task failed: {err}"))?
|
||||
.map_err(|err| format!("failed to prepare external agent session: {err}"))
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"session_preparation_task_failed",
|
||||
format!("external agent session preparation task failed: {err}"),
|
||||
)
|
||||
})?
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"failed_to_prepare_session",
|
||||
format!("failed to prepare external agent session: {err}"),
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
async fn persist_session(
|
||||
&self,
|
||||
session: ImportedExternalAgentSession,
|
||||
) -> Result<ThreadId, String> {
|
||||
) -> Result<ThreadId, SessionImportStepFailure> {
|
||||
let ImportedExternalAgentSession {
|
||||
cwd,
|
||||
title,
|
||||
@@ -252,7 +274,12 @@ impl ExternalAgentSessionImporter {
|
||||
},
|
||||
)
|
||||
.await
|
||||
.map_err(|err| format!("failed to load imported session config: {err}"))?;
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"failed_to_load_session_config",
|
||||
format!("failed to load imported session config: {err}"),
|
||||
)
|
||||
})?;
|
||||
let models_manager = self.thread_manager.get_models_manager();
|
||||
let model = models_manager
|
||||
.get_default_model(
|
||||
@@ -327,7 +354,12 @@ impl ExternalAgentSessionImporter {
|
||||
self.thread_store
|
||||
.create_thread(create_params)
|
||||
.await
|
||||
.map_err(|err| format!("failed to import session: {err}"))?;
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"failed_to_create_thread",
|
||||
format!("failed to import session: {err}"),
|
||||
)
|
||||
})?;
|
||||
if !rollout_items.is_empty()
|
||||
&& let Err(err) = self
|
||||
.thread_store
|
||||
@@ -338,7 +370,10 @@ impl ExternalAgentSessionImporter {
|
||||
.await
|
||||
{
|
||||
let _ = self.thread_store.discard_thread(thread_id).await;
|
||||
return Err(format!("failed to import session: {err}"));
|
||||
return Err(SessionImportStepFailure::new(
|
||||
"failed_to_append_thread_items",
|
||||
format!("failed to import session: {err}"),
|
||||
));
|
||||
}
|
||||
|
||||
self.thread_store
|
||||
@@ -348,15 +383,30 @@ impl ExternalAgentSessionImporter {
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.map_err(|err| format!("failed to update imported session: {err}"))?;
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"failed_to_update_thread_metadata",
|
||||
format!("failed to update imported session: {err}"),
|
||||
)
|
||||
})?;
|
||||
self.thread_store
|
||||
.persist_thread(thread_id)
|
||||
.await
|
||||
.map_err(|err| format!("failed to persist imported session: {err}"))?;
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"failed_to_persist_thread",
|
||||
format!("failed to persist imported session: {err}"),
|
||||
)
|
||||
})?;
|
||||
self.thread_store
|
||||
.shutdown_thread(thread_id)
|
||||
.await
|
||||
.map_err(|err| format!("failed to shutdown imported session: {err}"))?;
|
||||
.map_err(|err| {
|
||||
SessionImportStepFailure::new(
|
||||
"failed_to_shutdown_thread",
|
||||
format!("failed to shutdown imported session: {err}"),
|
||||
)
|
||||
})?;
|
||||
Ok(thread_id)
|
||||
}
|
||||
}
|
||||
@@ -365,4 +415,19 @@ struct SessionImportFailure {
|
||||
source_path: PathBuf,
|
||||
message: String,
|
||||
stage: &'static str,
|
||||
sub_error_type: &'static str,
|
||||
}
|
||||
|
||||
struct SessionImportStepFailure {
|
||||
sub_error_type: &'static str,
|
||||
message: String,
|
||||
}
|
||||
|
||||
impl SessionImportStepFailure {
|
||||
fn new(sub_error_type: &'static str, message: String) -> Self {
|
||||
Self {
|
||||
sub_error_type,
|
||||
message,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1243,6 +1243,12 @@ async fn external_agent_config_import_completed_tracks_analytics_event() -> Resu
|
||||
assert_eq!(completed.item_type_results.len(), 1);
|
||||
assert_eq!(completed.item_type_results[0].successes.len(), 0);
|
||||
assert_eq!(completed.item_type_results[0].failures.len(), 1);
|
||||
assert_eq!(
|
||||
completed.item_type_results[0].failures[0]
|
||||
.sub_error_type
|
||||
.as_deref(),
|
||||
Some("session_not_detected")
|
||||
);
|
||||
|
||||
let event = wait_for_analytics_event(
|
||||
&analytics_server,
|
||||
@@ -1270,6 +1276,7 @@ async fn external_agent_config_import_completed_tracks_analytics_event() -> Resu
|
||||
assert_eq!(event_params["type"], "SESSIONS");
|
||||
assert_eq!(event_params["failure_stage"], "session_missing");
|
||||
assert_eq!(event_params["error_type"], "session_missing");
|
||||
assert_eq!(event_params["sub_error_type"], "session_not_detected");
|
||||
assert!(event_params.get("raw_errors").is_none());
|
||||
assert!(event_params.get("message").is_none());
|
||||
|
||||
|
||||
@@ -23,13 +23,14 @@ fn migration_item_type_label(item_type: ExternalAgentConfigMigrationItemType) ->
|
||||
pub fn record_import_error(
|
||||
result: &mut ExternalAgentConfigImportItemResult,
|
||||
failure_stage: &'static str,
|
||||
sub_error_type: Option<&str>,
|
||||
message: impl Into<String>,
|
||||
source: Option<String>,
|
||||
) {
|
||||
result.record_error(ExternalAgentConfigImportRawError {
|
||||
item_type: result.item_type,
|
||||
error_type: None,
|
||||
sub_error_type: None,
|
||||
sub_error_type: sub_error_type.map(str::to_string),
|
||||
failure_stage: failure_stage.to_string(),
|
||||
message: message.into(),
|
||||
cwd: result.cwd.clone(),
|
||||
|
||||
@@ -208,6 +208,7 @@ impl ExternalAgentConfigService {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"plugin_import",
|
||||
/*sub_error_type*/ None,
|
||||
err.to_string(),
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -222,6 +223,7 @@ impl ExternalAgentConfigService {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"plugin_import",
|
||||
/*sub_error_type*/ None,
|
||||
err.to_string(),
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -239,6 +241,7 @@ impl ExternalAgentConfigService {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"plugin_import",
|
||||
/*sub_error_type*/ None,
|
||||
err.to_string(),
|
||||
/*source*/ None,
|
||||
);
|
||||
@@ -350,6 +353,7 @@ impl ExternalAgentConfigService {
|
||||
record_import_error(
|
||||
&mut item_result,
|
||||
"memory_import",
|
||||
/*sub_error_type*/ None,
|
||||
failure.message,
|
||||
Some(failure.project_key),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user