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

switch block-style to line-style comments #770

Merged
merged 1 commit into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions api_identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

// Copyright 2020 Oxide Computer Company
/*!
* This macro is a helper to generate an accessor for the identity of any
* `api::Object`.
*/
//! This macro is a helper to generate an accessor for the identity of any
//! `api::Object`.

extern crate proc_macro;

Expand All @@ -15,10 +13,8 @@ use quote::quote;
use syn::Fields;
use syn::ItemStruct;

/**
* Generates an "identity()" accessor for any `api::Object` having an `identity`
* field.
*/
/// Generates an "identity()" accessor for any `api::Object` having an `identity`
/// field.
#[proc_macro_derive(ObjectIdentity)]
pub fn object_identity(
item: proc_macro::TokenStream,
Expand Down
144 changes: 60 additions & 84 deletions common/src/api/external/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

/*!
* Error handling facilities for the Oxide control plane
*
* For HTTP-level error handling, see Dropshot.
*/
//! Error handling facilities for the Oxide control plane
//!
//! For HTTP-level error handling, see Dropshot.

use crate::api::external::Name;
use crate::api::external::ResourceType;
Expand All @@ -15,64 +13,58 @@ use serde::Deserialize;
use serde::Serialize;
use uuid::Uuid;

/**
* An error that can be generated within a control plane component
*
* These may be generated while handling a client request or as part of
* background operation. When generated as part of an HTTP request, an
* `Error` will be converted into an HTTP error as one of the last steps in
* processing the request. This allows most of the system to remain agnostic to
* the transport with which the system communicates with clients.
*
* General best practices for error design apply here. Where possible, we want
* to reuse existing variants rather than inventing new ones to distinguish
* cases that no programmatic consumer needs to distinguish.
*/
/// An error that can be generated within a control plane component
///
/// These may be generated while handling a client request or as part of
/// background operation. When generated as part of an HTTP request, an
/// `Error` will be converted into an HTTP error as one of the last steps in
/// processing the request. This allows most of the system to remain agnostic to
/// the transport with which the system communicates with clients.
///
/// General best practices for error design apply here. Where possible, we want
/// to reuse existing variants rather than inventing new ones to distinguish
/// cases that no programmatic consumer needs to distinguish.
#[derive(Debug, Deserialize, thiserror::Error, PartialEq, Serialize)]
pub enum Error {
/** An object needed as part of this operation was not found. */
/// An object needed as part of this operation was not found.
#[error("Object (of type {lookup_type:?}) not found: {type_name}")]
ObjectNotFound { type_name: ResourceType, lookup_type: LookupType },
/** An object already exists with the specified name or identifier. */
/// An object already exists with the specified name or identifier.
#[error("Object (of type {type_name:?}) already exists: {object_name}")]
ObjectAlreadyExists { type_name: ResourceType, object_name: String },
/**
* The request was well-formed, but the operation cannot be completed given
* the current state of the system.
*/
/// The request was well-formed, but the operation cannot be completed given
/// the current state of the system.
#[error("Invalid Request: {message}")]
InvalidRequest { message: String },
/**
* Authentication credentials were required but either missing or invalid.
* The HTTP status code is called "Unauthorized", but it's more accurate to
* call it "Unauthenticated".
*/
/// Authentication credentials were required but either missing or invalid.
/// The HTTP status code is called "Unauthorized", but it's more accurate to
/// call it "Unauthenticated".
#[error("Missing or invalid credentials")]
Unauthenticated { internal_message: String },
/** The specified input field is not valid. */
/// The specified input field is not valid.
#[error("Invalid Value: {label}, {message}")]
InvalidValue { label: String, message: String },
/** The request is not authorized to perform the requested operation. */
/// The request is not authorized to perform the requested operation.
#[error("Forbidden")]
Forbidden,

/** The system encountered an unhandled operational error. */
/// The system encountered an unhandled operational error.
#[error("Internal Error: {internal_message}")]
InternalError { internal_message: String },
/** The system (or part of it) is unavailable. */
/// The system (or part of it) is unavailable.
#[error("Service Unavailable: {internal_message}")]
ServiceUnavailable { internal_message: String },
/** Method Not Allowed */
/// Method Not Allowed
#[error("Method Not Allowed: {internal_message}")]
MethodNotAllowed { internal_message: String },
}

/** Indicates how an object was looked up (for an `ObjectNotFound` error) */
/// Indicates how an object was looked up (for an `ObjectNotFound` error)
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
pub enum LookupType {
/** a specific name was requested */
/// a specific name was requested
ByName(String),
/** a specific id was requested */
/// a specific id was requested
ById(Uuid),
}

Expand All @@ -97,10 +89,8 @@ impl From<&Name> for LookupType {
}

impl Error {
/**
* Returns whether the error is likely transient and could reasonably be
* retried
*/
/// Returns whether the error is likely transient and could reasonably be
/// retried
pub fn retryable(&self) -> bool {
match self {
Error::ServiceUnavailable { .. } => true,
Expand All @@ -116,67 +106,55 @@ impl Error {
}
}

/**
* Generates an [`Error::ObjectNotFound`] error for a lookup by object
* name.
*/
/// Generates an [`Error::ObjectNotFound`] error for a lookup by object
/// name.
pub fn not_found_by_name(type_name: ResourceType, name: &Name) -> Error {
LookupType::from(name).into_not_found(type_name)
}

/**
* Generates an [`Error::ObjectNotFound`] error for a lookup by object id.
*/
/// Generates an [`Error::ObjectNotFound`] error for a lookup by object id.
pub fn not_found_by_id(type_name: ResourceType, id: &Uuid) -> Error {
LookupType::ById(*id).into_not_found(type_name)
}

/**
* Generates an [`Error::InternalError`] error with the specific message
*
* InternalError should be used for operational conditions that should not
* happen but that we cannot reasonably handle at runtime (e.g.,
* deserializing a value from the database, or finding two records for
* something that is supposed to be unique).
*/
/// Generates an [`Error::InternalError`] error with the specific message
///
/// InternalError should be used for operational conditions that should not
/// happen but that we cannot reasonably handle at runtime (e.g.,
/// deserializing a value from the database, or finding two records for
/// something that is supposed to be unique).
pub fn internal_error(internal_message: &str) -> Error {
Error::InternalError { internal_message: internal_message.to_owned() }
}

/**
* Generates an [`Error::InvalidRequest`] error with the specific message
*
* This should be used for failures due possibly to invalid client input
* or malformed requests.
*/
/// Generates an [`Error::InvalidRequest`] error with the specific message
///
/// This should be used for failures due possibly to invalid client input
/// or malformed requests.
pub fn invalid_request(message: &str) -> Error {
Error::InvalidRequest { message: message.to_owned() }
}

/**
* Generates an [`Error::ServiceUnavailable`] error with the specific
* message
*
* This should be used for transient failures where the caller might be
* expected to retry. Logic errors or other problems indicating that a
* retry would not work should probably be an InternalError (if it's a
* server problem) or InvalidRequest (if it's a client problem) instead.
*/
/// Generates an [`Error::ServiceUnavailable`] error with the specific
/// message
///
/// This should be used for transient failures where the caller might be
/// expected to retry. Logic errors or other problems indicating that a
/// retry would not work should probably be an InternalError (if it's a
/// server problem) or InvalidRequest (if it's a client problem) instead.
pub fn unavail(message: &str) -> Error {
Error::ServiceUnavailable { internal_message: message.to_owned() }
}
}

impl From<Error> for HttpError {
/**
* Converts an `Error` error into an `HttpError`. This defines how
* errors that are represented internally using `Error` are ultimately
* exposed to clients over HTTP.
*/
/// Converts an `Error` error into an `HttpError`. This defines how
/// errors that are represented internally using `Error` are ultimately
/// exposed to clients over HTTP.
fn from(error: Error) -> HttpError {
match error {
Error::ObjectNotFound { type_name: t, lookup_type: lt } => {
/* TODO-cleanup is there a better way to express this? */
// TODO-cleanup is there a better way to express this?
let (lookup_field, lookup_value) = match lt {
LookupType::ByName(name) => ("name", name),
LookupType::ById(id) => ("id", id.to_string()),
Expand Down Expand Up @@ -318,11 +296,9 @@ impl<T: ClientError> From<progenitor::progenitor_client::Error<T>> for Error {
}
}

/**
* Like [`assert!`], except that instead of panicking, this function returns an
* `Err(Error::InternalError)` with an appropriate message if the given
* condition is not true.
*/
/// Like [`assert!`], except that instead of panicking, this function returns an
/// `Err(Error::InternalError)` with an appropriate message if the given
/// condition is not true.
#[macro_export]
macro_rules! bail_unless {
($cond:expr $(,)?) => {
Expand All @@ -343,7 +319,7 @@ mod test {
#[test]
fn test_bail_unless() {
#![allow(clippy::eq_op)]
/* Success cases */
// Success cases
let no_bail = || {
bail_unless!(1 + 1 == 2, "wrong answer: {}", 3);
Ok(())
Expand All @@ -355,7 +331,7 @@ mod test {
assert_eq!(Ok(()), no_bail());
assert_eq!(Ok(()), no_bail_label_args());

/* Failure cases */
// Failure cases
let do_bail = || {
bail_unless!(1 + 1 == 3);
Ok(())
Expand Down
Loading