Preserve plugin install failure subtypes during imports (#33182)

## What changed

- Add `subErrorType` to external agent config import failures and carry it through progress and completion notifications, persisted import histories, and analytics.
- Expose plugin store failure subtypes from `PluginInstallError` so manual installs and imported plugins report details such as `failed_to_copy_plugin_file` while failures without a subtype remain `null`.
- Keep existing persisted import records compatible by defaulting the new field when it is absent.

## Testing

- Cover store I/O subtype extraction, analytics serialization, persisted import histories, and migration notification fixtures.

GitOrigin-RevId: ec1a85623fc826cbd7cfe9f55c2dd2bc86665937
This commit is contained in:
charlesgong-openai
2026-07-14 22:21:27 +00:00
committed by copyberry
parent 44954d1b4b
commit 3fae30dd85
22 changed files with 114 additions and 29 deletions
@@ -3251,6 +3251,7 @@ fn plugin_install_failed_event_serializes_expected_shape() {
plugin: codex_plugin_metadata(sample_plugin_metadata()),
source: PluginInstallSource::Manual,
error_type: "store_io".to_string(),
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
},
});
@@ -3270,7 +3271,8 @@ fn plugin_install_failed_event_serializes_expected_shape() {
"connector_ids": ["calendar", "drive"],
"product_client_id": originator().value,
"source": "manual",
"error_type": "store_io"
"error_type": "store_io",
"sub_error_type": "failed_to_copy_plugin_file"
}
})
);
@@ -3703,6 +3705,7 @@ async fn reducer_ingests_plugin_install_failed_fact() {
plugin: sample_plugin_metadata(),
source: PluginInstallSource::ExternalAgentMigration,
error_type: "invalid_plugin".to_string(),
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
},
)),
&mut events,
@@ -3724,7 +3727,8 @@ async fn reducer_ingests_plugin_install_failed_fact() {
"connector_ids": ["calendar", "drive"],
"product_client_id": originator().value,
"source": "external_agent_migration",
"error_type": "invalid_plugin"
"error_type": "invalid_plugin",
"sub_error_type": "failed_to_copy_plugin_file"
}
}])
);
@@ -3747,6 +3751,7 @@ async fn reducer_ingests_plugin_install_failed_fact_without_detail() {
plugin,
source: PluginInstallSource::Manual,
error_type: "remote_catalog_unexpected_status".to_string(),
sub_error_type: None,
},
)),
&mut events,
@@ -3768,7 +3773,8 @@ async fn reducer_ingests_plugin_install_failed_fact_without_detail() {
"connector_ids": null,
"product_client_id": originator().value,
"source": "manual",
"error_type": "remote_catalog_unexpected_status"
"error_type": "remote_catalog_unexpected_status",
"sub_error_type": null
}
}])
);
@@ -3819,9 +3825,10 @@ fn external_agent_config_import_failure_event_serializes_expected_shape() {
event_params: CodexOnboardingExternalAgentImportFailureMetadata {
import_id: "import-1".to_string(),
source: "app_server".to_string(),
item_type: "SESSIONS".to_string(),
failure_stage: "session_missing".to_string(),
error_type: "session_missing".to_string(),
item_type: "PLUGINS".to_string(),
failure_stage: "plugin_import".to_string(),
error_type: "plugin_import".to_string(),
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
product_client_id: Some(originator().value),
},
},
@@ -3836,9 +3843,10 @@ fn external_agent_config_import_failure_event_serializes_expected_shape() {
"event_params": {
"import_id": "import-1",
"source": "app_server",
"type": "SESSIONS",
"failure_stage": "session_missing",
"error_type": "session_missing",
"type": "PLUGINS",
"failure_stage": "plugin_import",
"error_type": "plugin_import",
"sub_error_type": "failed_to_copy_plugin_file",
"product_client_id": originator().value,
}
})
@@ -3856,9 +3864,10 @@ async fn reducer_ingests_external_agent_config_import_failure_fact() {
ExternalAgentConfigImportFailureInput {
import_id: "import-1".to_string(),
source: "app_server".to_string(),
item_type: "SESSIONS".to_string(),
failure_stage: "session_missing".to_string(),
error_type: "session_missing".to_string(),
item_type: "PLUGINS".to_string(),
failure_stage: "plugin_import".to_string(),
error_type: "plugin_import".to_string(),
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
},
)),
&mut events,
@@ -3873,9 +3882,10 @@ async fn reducer_ingests_external_agent_config_import_failure_fact() {
"event_params": {
"import_id": "import-1",
"source": "app_server",
"type": "SESSIONS",
"failure_stage": "session_missing",
"error_type": "session_missing",
"type": "PLUGINS",
"failure_stage": "plugin_import",
"error_type": "plugin_import",
"sub_error_type": "failed_to_copy_plugin_file",
"product_client_id": originator().value,
}
}])
+2
View File
@@ -376,12 +376,14 @@ impl AnalyticsEventsClient {
plugin: PluginTelemetryMetadata,
source: PluginInstallSource,
error_type: String,
sub_error_type: Option<String>,
) {
self.record_fact(AnalyticsFact::Custom(
CustomAnalyticsFact::PluginInstallFailed(PluginInstallFailedInput {
plugin,
source,
error_type,
sub_error_type,
}),
));
}
+2
View File
@@ -1004,6 +1004,7 @@ pub(crate) struct CodexPluginInstallFailedMetadata {
pub(crate) plugin: CodexPluginMetadata,
pub(crate) source: crate::facts::PluginInstallSource,
pub(crate) error_type: String,
pub(crate) sub_error_type: Option<String>,
}
#[derive(Serialize)]
@@ -1037,6 +1038,7 @@ pub(crate) struct CodexOnboardingExternalAgentImportFailureMetadata {
pub(crate) item_type: String,
pub(crate) failure_stage: String,
pub(crate) error_type: String,
pub(crate) sub_error_type: Option<String>,
pub(crate) product_client_id: Option<String>,
}
+2
View File
@@ -590,6 +590,7 @@ pub(crate) struct PluginInstallFailedInput {
pub plugin: PluginTelemetryMetadata,
pub source: PluginInstallSource,
pub error_type: String,
pub sub_error_type: Option<String>,
}
pub struct ExternalAgentConfigImportCompletedInput {
@@ -606,6 +607,7 @@ pub struct ExternalAgentConfigImportFailureInput {
pub item_type: String,
pub failure_stage: String,
pub error_type: String,
pub sub_error_type: Option<String>,
}
#[derive(Clone, Copy)]
+3
View File
@@ -841,6 +841,7 @@ impl AnalyticsReducer {
plugin,
source,
error_type,
sub_error_type,
} = input;
out.push(TrackEventRequest::PluginInstallFailed(
CodexPluginInstallFailedEventRequest {
@@ -849,6 +850,7 @@ impl AnalyticsReducer {
plugin: codex_plugin_metadata(plugin),
source,
error_type,
sub_error_type,
},
},
));
@@ -888,6 +890,7 @@ impl AnalyticsReducer {
item_type: input.item_type,
failure_stage: input.failure_stage,
error_type: input.error_type,
sub_error_type: input.sub_error_type,
product_client_id: Some(originator().value),
},
},
@@ -1209,6 +1209,12 @@
"string",
"null"
]
},
"subErrorType": {
"type": [
"string",
"null"
]
}
},
"required": [
@@ -9366,6 +9366,12 @@
"string",
"null"
]
},
"subErrorType": {
"type": [
"string",
"null"
]
}
},
"required": [
@@ -5599,6 +5599,12 @@
"string",
"null"
]
},
"subErrorType": {
"type": [
"string",
"null"
]
}
},
"required": [
@@ -29,6 +29,12 @@
"string",
"null"
]
},
"subErrorType": {
"type": [
"string",
"null"
]
}
},
"required": [
@@ -59,6 +59,12 @@
"string",
"null"
]
},
"subErrorType": {
"type": [
"string",
"null"
]
}
},
"required": [
@@ -29,6 +29,12 @@
"string",
"null"
]
},
"subErrorType": {
"type": [
"string",
"null"
]
}
},
"required": [
@@ -3,4 +3,4 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { ExternalAgentConfigMigrationItemType } from "./ExternalAgentConfigMigrationItemType";
export type ExternalAgentConfigImportItemTypeFailure = { itemType: ExternalAgentConfigMigrationItemType, errorType: string | null, failureStage: string, message: string, cwd: string | null, source: string | null, };
export type ExternalAgentConfigImportItemTypeFailure = { itemType: ExternalAgentConfigMigrationItemType, errorType: string | null, subErrorType: string | null, failureStage: string, message: string, cwd: string | null, source: string | null, };
@@ -715,6 +715,7 @@ pub struct ExternalAgentConfigImportResponse {
pub struct ExternalAgentConfigImportItemTypeFailure {
pub item_type: ExternalAgentConfigMigrationItemType,
pub error_type: Option<String>,
pub sub_error_type: Option<String>,
pub failure_stage: String,
pub message: String,
pub cwd: Option<PathBuf>,
@@ -640,6 +640,7 @@ fn track_completed_import_notification(
item_type: item_type.clone(),
failure_stage: failure.failure_stage.clone(),
error_type: import_failure_error_type(failure),
sub_error_type: failure.sub_error_type.clone(),
},
);
}
@@ -692,6 +693,7 @@ async fn record_completed_import_notification(
Ok(ExternalAgentConfigImportFailureRecord {
item_type: serde_json::from_value(serde_json::to_value(failure.item_type)?)?,
error_type: failure.error_type.clone(),
sub_error_type: failure.sub_error_type.clone(),
failure_stage: failure.failure_stage.clone(),
message: failure.message.clone(),
cwd: failure.cwd.clone(),
@@ -747,6 +749,7 @@ fn protocol_import_failure_record(
Ok(ProtocolImportFailure {
item_type: protocol_import_record_item_type(record.item_type)?,
error_type: record.error_type,
sub_error_type: record.sub_error_type,
failure_stage: record.failure_stage,
message: record.message,
cwd: record.cwd,
@@ -844,6 +847,7 @@ fn protocol_import_raw_error(raw_error: &CoreImportRawError) -> ProtocolImportFa
ProtocolImportFailure {
item_type: protocol_migration_item_type(raw_error.item_type),
error_type: raw_error.error_type.clone(),
sub_error_type: raw_error.sub_error_type.clone(),
failure_stage: raw_error.failure_stage.clone(),
message: raw_error.message.clone(),
cwd: raw_error.cwd.clone(),
@@ -170,6 +170,7 @@ pub(crate) struct ExternalAgentConfigImportSuccess {
pub(crate) struct ExternalAgentConfigImportRawError {
pub item_type: ExternalAgentConfigMigrationItemType,
pub error_type: Option<String>,
pub sub_error_type: Option<String>,
pub failure_stage: String,
pub message: String,
pub cwd: Option<PathBuf>,
@@ -448,6 +449,7 @@ impl ExternalAgentConfigService {
item_result.record_error(ExternalAgentConfigImportRawError {
item_type,
error_type: Some(error_type.to_string()),
sub_error_type: None,
failure_stage: "import_request_failed".to_string(),
message,
cwd: item_result.cwd.clone(),
@@ -1068,12 +1070,14 @@ impl ExternalAgentConfigService {
Err(err) => {
let plugin_id = format!("{plugin_name}@{marketplace_name}");
outcome.failed_plugin_ids.push(plugin_id.clone());
let sub_error_type = err.sub_error_type();
let mut raw_error = plugin_import_raw_error(
cwd,
"plugin_import",
err.to_string(),
Some(plugin_id),
);
raw_error.sub_error_type = sub_error_type;
if matches!(
err,
PluginInstallError::Marketplace(
@@ -1660,6 +1664,7 @@ pub(crate) fn record_import_error(
result.record_error(ExternalAgentConfigImportRawError {
item_type: result.item_type,
error_type: None,
sub_error_type: None,
failure_stage: failure_stage.to_string(),
message: message.into(),
cwd: result.cwd.clone(),
@@ -1691,6 +1696,7 @@ fn plugin_import_raw_error(
ExternalAgentConfigImportRawError {
item_type: ExternalAgentConfigMigrationItemType::Plugins,
error_type: None,
sub_error_type: None,
failure_stage: failure_stage.to_string(),
message,
cwd: cwd.map(Path::to_path_buf),
@@ -49,6 +49,7 @@ fn assert_single_plugin_raw_error(
);
assert_eq!(raw_error.failure_stage, failure_stage);
assert_eq!(raw_error.error_type.as_deref(), error_type);
assert_eq!(raw_error.sub_error_type, None);
assert_eq!(raw_error.cwd, None);
assert_eq!(raw_error.source.as_deref(), Some(source));
assert!(!raw_error.message.is_empty());
@@ -1750,6 +1750,7 @@ impl PluginRequestProcessor {
plugin,
PluginInstallSource::Manual,
error_type.to_string(),
sub_error_type,
);
}
+11 -13
View File
@@ -1312,7 +1312,7 @@ impl PluginsManager {
self.track_plugin_install_failed(
&plugin_id,
plugin_install_error_type(&err),
plugin_install_sub_error_type(&err),
err.sub_error_type(),
err.to_string(),
);
Err(err)
@@ -1375,7 +1375,7 @@ impl PluginsManager {
self.track_plugin_install_failed(
&resolved.plugin_id,
plugin_install_error_type(&err),
plugin_install_sub_error_type(&err),
err.sub_error_type(),
err.to_string(),
);
return Err(err);
@@ -1387,7 +1387,7 @@ impl PluginsManager {
self.track_plugin_install_failed(
&plugin_id,
plugin_install_error_type(&err),
plugin_install_sub_error_type(&err),
err.sub_error_type(),
err.to_string(),
);
Err(err)
@@ -1450,6 +1450,7 @@ impl PluginsManager {
self.telemetry_metadata_for_plugin_id(plugin_id),
self.plugin_install_source,
error_type.to_string(),
sub_error_type,
);
}
}
@@ -2728,6 +2729,13 @@ impl PluginInstallError {
) | Self::Store(PluginStoreError::Invalid(_))
)
}
pub fn sub_error_type(&self) -> Option<String> {
match self {
Self::Store(err) => err.sub_error_type(),
Self::Marketplace(_) | Self::Remote(_) | Self::Config(_) | Self::Join(_) => None,
}
}
}
fn plugin_install_error_type(err: &PluginInstallError) -> &'static str {
@@ -2740,16 +2748,6 @@ fn plugin_install_error_type(err: &PluginInstallError) -> &'static str {
}
}
fn plugin_install_sub_error_type(err: &PluginInstallError) -> Option<String> {
match err {
PluginInstallError::Store(err) => err.sub_error_type(),
PluginInstallError::Marketplace(_)
| PluginInstallError::Remote(_)
| PluginInstallError::Config(_)
| PluginInstallError::Join(_) => None,
}
}
fn marketplace_error_type(err: &MarketplaceError) -> &'static str {
match err {
MarketplaceError::Io { .. } => "marketplace_io",
@@ -5950,3 +5950,16 @@ fn remote_installed_plugins_cache_refresh_coalesces_materializations() {
0
);
}
#[test]
fn plugin_install_error_preserves_store_io_sub_error_type() {
let error = PluginInstallError::Store(PluginStoreError::Io {
context: "failed to copy plugin file",
source: std::io::Error::other("copy failed"),
});
assert_eq!(
error.sub_error_type(),
Some("failed_to_copy_plugin_file".to_string())
);
}
@@ -18,6 +18,8 @@ pub struct ExternalAgentConfigImportSuccessRecord {
pub struct ExternalAgentConfigImportFailureRecord {
pub item_type: String,
pub error_type: Option<String>,
#[serde(default)]
pub sub_error_type: Option<String>,
pub failure_stage: String,
pub message: String,
pub cwd: Option<PathBuf>,
@@ -38,6 +38,7 @@ async fn records_completion_by_import_id() -> anyhow::Result<()> {
&[ExternalAgentConfigImportFailureRecord {
item_type: "MCP_SERVER_CONFIG".to_string(),
error_type: None,
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
failure_stage: "import".to_string(),
message: "failed".to_string(),
cwd: None,
@@ -68,6 +69,7 @@ async fn records_completion_by_import_id() -> anyhow::Result<()> {
failures: vec![ExternalAgentConfigImportFailureRecord {
item_type: "MCP_SERVER_CONFIG".to_string(),
error_type: None,
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
failure_stage: "import".to_string(),
message: "failed".to_string(),
cwd: None,
@@ -106,6 +108,7 @@ async fn records_completion_by_import_id() -> anyhow::Result<()> {
vec![ExternalAgentConfigImportFailureRecord {
item_type: "MCP_SERVER_CONFIG".to_string(),
error_type: None,
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
failure_stage: "import".to_string(),
message: "failed".to_string(),
cwd: None,
@@ -123,6 +123,7 @@ fn completed_notification() -> ExternalAgentConfigImportCompletedNotification {
failures: vec![ExternalAgentConfigImportItemTypeFailure {
item_type: ExternalAgentConfigMigrationItemType::Plugins,
error_type: Some("plugin_install_failed".to_string()),
sub_error_type: Some("failed_to_copy_plugin_file".to_string()),
failure_stage: "plugin_import".to_string(),
message: "install failed".to_string(),
cwd: Some(PathBuf::from("/workspace/project")),