Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

structured error codes should be in propolis-client #775

Open
hawkw opened this issue Oct 1, 2024 · 0 comments
Open

structured error codes should be in propolis-client #775

hawkw opened this issue Oct 1, 2024 · 0 comments
Assignees
Labels
api Related to the API. enhancement New feature or request. server Related specifically to the Propolis server API and its VM management functions.

Comments

@hawkw
Copy link
Member

hawkw commented Oct 1, 2024

Currently, propolis-server's API returns structured error codes for certain error conditions:

/// Error codes used to populate the `error_code` field of Dropshot API responses.
#[derive(
Clone, Copy, Debug, Deserialize, PartialEq, Eq, Serialize, JsonSchema,
)]
pub enum ErrorCode {
/// This `propolis-server` process has not received an `InstanceEnsure`
/// request yet.
NoInstance,
/// This `propolis-server` process has already received an `InstanceEnsure`
/// request with a different ID.
AlreadyInitialized,
/// Cannot update a running server.
AlreadyRunning,
/// Instance creation failed
CreateFailed,
}
impl fmt::Display for ErrorCode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}
impl std::str::FromStr for ErrorCode {
type Err = &'static str;
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.trim() {
s if s.eq_ignore_ascii_case("NoInstance") => Ok(Self::NoInstance),
s if s.eq_ignore_ascii_case("AlreadyInitialized") => {
Ok(ErrorCode::AlreadyInitialized)
}
s if s.eq_ignore_ascii_case("AlreadyRunning") => {
Ok(ErrorCode::AlreadyRunning)
}
s if s.eq_ignore_ascii_case("CreateFailed") => {
Ok(ErrorCode::CreateFailed)
}
_ => Err("unknown error code, expected one of: \
'NoInstance', 'AlreadyInitialized', 'AlreadyRunning', \
'CreateFailed'"),
}
}
}

These are parsed by sled-agent and used to detect particular error conditions, such as the propolis-server believing that an instance-ensure request has never been received. At present, the enum representing these error codes is defined in the propolis-api-types crate, which requires that sled-agent depend on it. The propolis-api-types dependency is not otherwise needed by sled-agent. It would be much nicer if sled-agent could get the definition of the error code values from its propolis-client dependency, instead. Ideally, these would be part of the OpenAPI document and generated by Progenitor when building the client, as discussed in oxidecomputer/omicron#6726 (comment)

Ideally, we would be able to define the error codes returned by these APIs as part of the Dropshot server and have them be included in the OpenAPI spec without requiring us to patch the document to include them. See oxidecomputer/dropshot#39 and oxidecomputer/dropshot#41 for related issues.

@hawkw hawkw added api Related to the API. enhancement New feature or request. server Related specifically to the Propolis server API and its VM management functions. labels Oct 1, 2024
@hawkw hawkw self-assigned this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to the API. enhancement New feature or request. server Related specifically to the Propolis server API and its VM management functions.
Projects
None yet
Development

No branches or pull requests

1 participant