mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Preserve Windows runtime registration failure diagnostics (#53426)
## Why Managed runtime package registration errors lacked the terminal status, separate async and deployment error codes, and deployment activity ID needed to diagnose failures. ## What changed Include these diagnostics in registration errors and distinguish failures in `GetResults`, the `ExtendedErrorCode` getter, and deployment itself. Preserve the original completion error; failures to read diagnostic fields do not turn successful registration into failure. Keep free-form Windows error text out of diagnostic summaries. ## Testing Add unit tests for distinct async and deployment errors, failed or canceled result retrieval, and diagnostic getter failures, including preservation of native error codes and omission of free-form diagnostic text. GitOrigin-RevId: 7f3a46741304ef04f5c3e2aa575bcfc4392dff62
This commit is contained in:
@@ -39,6 +39,7 @@ use crate::package_lifecycle::with_owner_impersonation;
|
||||
|
||||
mod known_folder;
|
||||
mod metadata;
|
||||
mod registration_result;
|
||||
mod removal;
|
||||
|
||||
pub(crate) use metadata::remove as remove_metadata;
|
||||
@@ -168,7 +169,7 @@ pub(crate) fn provision(
|
||||
// As with provisioning, retain the request's resources until Windows finishes.
|
||||
// A client timeout or an unreadable status cannot safely release the profile.
|
||||
let mut status_error_logged = false;
|
||||
loop {
|
||||
let status = loop {
|
||||
if shutdown.load(Ordering::Acquire) {
|
||||
// The typed error bypasses all response conversion and cleanup. Keep the
|
||||
// in-flight operation and its profile alive until this own-process service exits.
|
||||
@@ -177,8 +178,12 @@ pub(crate) fn provision(
|
||||
}
|
||||
match operation.Status() {
|
||||
Ok(AsyncStatus::Started) => {}
|
||||
Ok(AsyncStatus::Completed | AsyncStatus::Canceled | AsyncStatus::Error) => {
|
||||
break;
|
||||
Ok(
|
||||
status @ (AsyncStatus::Completed
|
||||
| AsyncStatus::Canceled
|
||||
| AsyncStatus::Error),
|
||||
) => {
|
||||
break status;
|
||||
}
|
||||
status => {
|
||||
if !status_error_logged {
|
||||
@@ -193,12 +198,8 @@ pub(crate) fn provision(
|
||||
}
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
}
|
||||
operation
|
||||
.GetResults()
|
||||
.and_then(|result| result.ExtendedErrorCode())
|
||||
.and_then(windows::core::HRESULT::ok)
|
||||
.context("finish managed runtime package registration")?;
|
||||
};
|
||||
registration_result::RegistrationResult::read(&operation, status).check()?;
|
||||
ensure!(
|
||||
registered_packages(&profile.user_sid, &family)
|
||||
.context("verify managed runtime package registration")?
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
//! Retains the distinct native registration failure codes and deployment activity ID.
|
||||
//! Diagnostic getters never replace the completion error or turn success into failure.
|
||||
//! Free-form Windows ErrorText stays in Windows deployment diagnostics, not this summary.
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use windows::Foundation::AsyncStatus;
|
||||
use windows::Foundation::IAsyncOperationWithProgress;
|
||||
use windows::Management::Deployment::DeploymentProgress;
|
||||
use windows::Management::Deployment::DeploymentResult;
|
||||
use windows::core::GUID;
|
||||
use windows::core::HRESULT;
|
||||
|
||||
pub(super) struct RegistrationResult {
|
||||
status: AsyncStatus,
|
||||
async_error: windows::core::Result<HRESULT>,
|
||||
// Keep GetResults failure separate from failure to read ExtendedErrorCode.
|
||||
result: windows::core::Result<DeploymentDetails>,
|
||||
}
|
||||
|
||||
struct DeploymentDetails {
|
||||
extended_error: windows::core::Result<HRESULT>,
|
||||
activity_id: windows::core::Result<GUID>,
|
||||
}
|
||||
|
||||
impl RegistrationResult {
|
||||
/// The caller must first observe a terminal status while retaining the profile/token.
|
||||
pub(super) fn read(
|
||||
operation: &IAsyncOperationWithProgress<DeploymentResult, DeploymentProgress>,
|
||||
status: AsyncStatus,
|
||||
) -> Self {
|
||||
Self {
|
||||
status,
|
||||
async_error: operation.ErrorCode(),
|
||||
result: operation.GetResults().map(|result| DeploymentDetails {
|
||||
extended_error: result.ExtendedErrorCode(),
|
||||
activity_id: result.ActivityId(),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn check(self) -> Result<()> {
|
||||
let Self {
|
||||
status,
|
||||
async_error,
|
||||
result,
|
||||
} = self;
|
||||
let async_error = match async_error {
|
||||
Ok(code) => format!("{code:?}"),
|
||||
Err(error) => format!("unavailable({:?})", error.code()),
|
||||
};
|
||||
let context = format!(
|
||||
"finish managed runtime package registration: status={} async_error={async_error}",
|
||||
status.0,
|
||||
);
|
||||
let details = result.map_err(|error| {
|
||||
let summary = format!("{context} phase=GetResults get_results={:?}", error.code());
|
||||
anyhow::Error::new(error).context(summary)
|
||||
})?;
|
||||
let activity_id = match details.activity_id {
|
||||
Ok(id) => format!("{id:?}"),
|
||||
Err(error) => format!("unavailable({:?})", error.code()),
|
||||
};
|
||||
let context = format!("{context} get_results=0x00000000 activity_id={activity_id}");
|
||||
let extended_error = details.extended_error.map_err(|error| {
|
||||
let summary = format!(
|
||||
"{context} phase=ExtendedErrorCode getter_error={:?}",
|
||||
error.code()
|
||||
);
|
||||
anyhow::Error::new(error).context(summary)
|
||||
})?;
|
||||
extended_error.ok().with_context(|| {
|
||||
format!("{context} phase=deployment extended_error={extended_error:?}")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "registration_result_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,93 @@
|
||||
//! Completion diagnostics preserve native errors without depending on account mutation.
|
||||
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
const ACCESS_DENIED: HRESULT = HRESULT(0x80070005_u32 as i32);
|
||||
const CANCELED: HRESULT = HRESULT(0x800704C7_u32 as i32);
|
||||
const ASYNC_FAILURE: HRESULT = HRESULT(0x80073D19_u32 as i32);
|
||||
const DEPLOYMENT_FAILURE: HRESULT = HRESULT(0x80073D28_u32 as i32);
|
||||
|
||||
#[test]
|
||||
fn retains_different_async_and_deployment_errors_with_activity() {
|
||||
let activity_id = GUID::from_u128(0x1b60fffa_59b3_0002_d8d5_611bb359dd01);
|
||||
let error = RegistrationResult {
|
||||
status: AsyncStatus::Error,
|
||||
async_error: Ok(ASYNC_FAILURE),
|
||||
result: Ok(DeploymentDetails {
|
||||
extended_error: Ok(DEPLOYMENT_FAILURE),
|
||||
activity_id: Ok(activity_id),
|
||||
}),
|
||||
}
|
||||
.check()
|
||||
.unwrap_err();
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
format!(
|
||||
"finish managed runtime package registration: status=3 async_error={ASYNC_FAILURE:?} get_results=0x00000000 activity_id={activity_id:?} phase=deployment extended_error={DEPLOYMENT_FAILURE:?}"
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
error.downcast_ref::<windows::core::Error>().unwrap().code(),
|
||||
DEPLOYMENT_FAILURE
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_results_errors_keep_original_hresult_and_phase() {
|
||||
for (status, code) in [
|
||||
(AsyncStatus::Error, ACCESS_DENIED),
|
||||
(AsyncStatus::Canceled, CANCELED),
|
||||
] {
|
||||
let error = RegistrationResult {
|
||||
status,
|
||||
async_error: Ok(code),
|
||||
result: Err(windows::core::Error::from_hresult(code)),
|
||||
}
|
||||
.check()
|
||||
.unwrap_err();
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
format!(
|
||||
"finish managed runtime package registration: status={} async_error={code:?} phase=GetResults get_results={code:?}",
|
||||
status.0
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
error.downcast_ref::<windows::core::Error>().unwrap().code(),
|
||||
code
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diagnostic_getter_failures_do_not_mask_completion_or_leak_text() {
|
||||
let private_error = || windows::core::Error::new(ACCESS_DENIED, "private deployment path");
|
||||
for code in [HRESULT(0), CANCELED] {
|
||||
let result = RegistrationResult {
|
||||
status: AsyncStatus::Completed,
|
||||
async_error: Err(private_error()),
|
||||
result: Ok(DeploymentDetails {
|
||||
extended_error: code.ok().map(|()| code),
|
||||
activity_id: Err(private_error()),
|
||||
}),
|
||||
}
|
||||
.check();
|
||||
if code.is_err() {
|
||||
let error = result.unwrap_err();
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
format!(
|
||||
"finish managed runtime package registration: status=1 async_error=unavailable({ACCESS_DENIED:?}) get_results=0x00000000 activity_id=unavailable({ACCESS_DENIED:?}) phase=ExtendedErrorCode getter_error={CANCELED:?}"
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
error.downcast_ref::<windows::core::Error>().unwrap().code(),
|
||||
CANCELED
|
||||
);
|
||||
assert!(!format!("{error:#}").contains("private deployment path"));
|
||||
} else {
|
||||
assert_eq!(result.unwrap(), ());
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user