Skip to content
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
16 changes: 11 additions & 5 deletions dropshot/src/api_description.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2025 Oxide Computer Company
// Copyright 2026 Oxide Computer Company

//! Describes the endpoints and handler functions in your API

Expand Down Expand Up @@ -996,10 +996,16 @@ impl<Context: ServerContext> ApiDescription<Context> {
},
);
openapiv3::Response {
// TODO: perhaps we should require even free-form
// responses to have a description since it's required
// by OpenAPI.
description: "".to_string(),
description: if let Some(description) =
&endpoint.response.description
{
description.clone()
} else {
// TODO: perhaps we should require even free-form
// responses to have a description since it's required
// by OpenAPI.
"".to_string()
},
content,
..Default::default()
}
Expand Down
52 changes: 40 additions & 12 deletions dropshot/src/websocket.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
// Copyright 2023 Oxide Computer Company
// Copyright 2026 Oxide Computer Company

//! Implements websocket upgrades as an Extractor for use in API route handler
//! parameters to indicate that the given endpoint is meant to be upgraded to
//! a websocket.
//!
//! This exposes a raw upgraded HTTP connection to a user-provided async future,
//! which will be spawned to handle the incoming connection.

use crate::api_description::ExtensionMode;
use crate::api_description::{ApiSchemaGenerator, ExtensionMode};
use crate::body::Body;
use crate::handler::HttpHandlerResult;
use crate::{
ApiEndpointBodyContentType, ExclusiveExtractor, ExtractorMetadata,
HttpError, RequestContext, ServerContext,
ApiEndpointBodyContentType, ApiEndpointResponse, ExclusiveExtractor,
ExtractorMetadata, HttpError, HttpResponse, RequestContext, ServerContext,
};
use async_trait::async_trait;
use base64::Engine;
Expand Down Expand Up @@ -45,7 +47,39 @@ pub type WebsocketChannelResult =
/// [WebsocketUpgrade::handle]'s return type.
/// The `#[endpoint]` handler must return the value returned by
/// [WebsocketUpgrade::handle]. (This is done for you by `#[channel]`.)
Comment on lines 47 to 49
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the situation in which a user would care about this type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One such case is described here:
https://github.com/oxidecomputer/omicron/blob/0d9b2d7ef83ac3a081972c7f4f6492440bff413f/gateway-api/src/lib.rs#L194-L218
Essentially, it's serving as an escape hatch for anyone who wishes to do pre-upgrade error reporting through HTTP (as opposed to upgrading and then reporting the error condition via a WebSocket Close frame).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any uses that don't do (or shouldn't do) pre-upgrade error reporting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The serial-console endpoints of propolis and nexus do any error reporting in-band after the upgrade (e.g. for loss of connection with the instance), and propolis's internal API endpoint for instance migrate also does its error reporting post-upgrade, starting from the migration protocol handshake. That's not to say they have a particular reason to avoid reporting errors via HTTP, but they don't do so currently. (Of course, as we encountered, lower levels of the stack may still report an Invalid Request before getting that far)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens e.g. in the CLI if I try to view the serial console of a instance that doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omicron sends the instance lookup error through the WebSocket as a CloseCode reason (nexus/src/external_api/http_entrypoints.rs#L2932-L2954); the client prints the close code (thouart/src/lib.rs#L264), and returns the reason in an Err to the CLI's invocation (cli/src/cmd_instance.rs#L137)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we do that rather than returning an http response prior to the upgrade?

pub type WebsocketEndpointResult = Result<Response<Body>, HttpError>;
pub type WebsocketEndpointResult = Result<SwitchingToWebsocket, HttpError>;

pub struct SwitchingToWebsocket {
accept_key: String,
}

impl HttpResponse for SwitchingToWebsocket {
fn to_result(self) -> HttpHandlerResult {
Response::builder()
.status(StatusCode::SWITCHING_PROTOCOLS)
.header(header::CONNECTION, "Upgrade")
.header(header::UPGRADE, "websocket")
.header(header::SEC_WEBSOCKET_ACCEPT, self.accept_key)
.body(Body::empty())
.map_err(Into::into)
}
fn response_metadata() -> ApiEndpointResponse {
const UPGRADE_DESCRIPTION: &str =
"Negotiating protocol upgrade from HTTP/1.1 to WebSocket";
ApiEndpointResponse {
schema: Some(ApiSchemaGenerator::Static {
schema: Box::new(schemars::schema::Schema::Bool(false)),
dependencies: Default::default(),
}),
headers: vec![],
success: Some(StatusCode::SWITCHING_PROTOCOLS),
description: Some(UPGRADE_DESCRIPTION.to_string()),
}
}
fn status_code(&self) -> StatusCode {
StatusCode::SWITCHING_PROTOCOLS
}
}

/// The upgraded connection passed as the last argument to the websocket
/// handler function. [`WebsocketConnection::into_inner`] can be used to
Expand Down Expand Up @@ -257,13 +291,7 @@ impl WebsocketUpgrade {
}
}
});
Response::builder()
.status(StatusCode::SWITCHING_PROTOCOLS)
.header(header::CONNECTION, "Upgrade")
.header(header::UPGRADE, "websocket")
.header(header::SEC_WEBSOCKET_ACCEPT, accept_key)
.body(Body::empty())
.map_err(Into::into)
Ok(SwitchingToWebsocket { accept_key })
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_channel17.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ note: required by a bound in `need_shared_extractor`
| ------------------- required by a bound in this function
= note: this error originates in the attribute macro `channel` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {two_websocket_channels_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {two_websocket_channels_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_channel17.rs:16:1
|
12 | / #[channel {
Expand All @@ -49,7 +49,7 @@ error[E0277]: the trait bound `fn(RequestContext<()>, WebsocketConnection, Webso
16 | async fn two_websocket_channels(
| ^^^^^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {two_websocket_channels_adapter}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {two_websocket_channels_adapter}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_channel18.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ note: required by a bound in `validate_websocket_connection_type`
26 | _query: Query<Stuff>,
| ^^^^^ required by this bound in `validate_websocket_connection_type`

error[E0277]: the trait bound `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {websocket_channel_not_last_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {websocket_channel_not_last_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_channel18.rs:23:1
|
19 | / #[channel {
Expand All @@ -88,7 +88,7 @@ error[E0277]: the trait bound `fn(RequestContext<()>, WebsocketConnection, Webso
23 | async fn websocket_channel_not_last(
| ^^^^^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {websocket_channel_not_last_adapter}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {websocket_channel_not_last_adapter}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_channel19.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ note: required by a bound in `need_shared_extractor`
| ------ required by a bound in this function
= note: this error originates in the attribute macro `channel` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `fn(RequestContext<()>, std::string::String, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {non_extractor_as_last_argument_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(RequestContext<()>, std::string::String, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {non_extractor_as_last_argument_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_channel19.rs:23:1
|
19 | / #[channel {
Expand All @@ -49,7 +49,7 @@ error[E0277]: the trait bound `fn(RequestContext<()>, std::string::String, Webso
23 | async fn non_extractor_as_last_argument(
| ^^^^^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, std::string::String, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {non_extractor_as_last_argument_adapter}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, std::string::String, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {non_extractor_as_last_argument_adapter}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_channel4.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ note: required by a bound in `dropshot::Query`
| ^^^^^^^^^^^^^^^^ required by this bound in `Query`
= note: this error originates in the attribute macro `channel` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {bad_channel_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {bad_channel_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_channel4.rs:20:1
|
16 | / #[channel {
Expand All @@ -196,7 +196,7 @@ error[E0277]: the trait bound `fn(RequestContext<()>, dropshot::Query<QueryParam
20 | async fn bad_channel(
| ^^^^^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {bad_channel_adapter}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {bad_channel_adapter}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_channel5.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ note: required by a bound in `dropshot::Query`
| ^^^^^^^^^^^^^^^^ required by this bound in `Query`
= note: this error originates in the attribute macro `channel` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {bad_channel_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {bad_channel_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_channel5.rs:22:1
|
18 | / #[channel {
Expand All @@ -107,7 +107,7 @@ error[E0277]: the trait bound `fn(RequestContext<()>, dropshot::Query<QueryParam
22 | async fn bad_channel(
| ^^^^^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {bad_channel_adapter}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(RequestContext<()>, dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {bad_channel_adapter}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_channel9.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ help: the trait `RequestContextArgument` is implemented for `RequestContext<T>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: this error originates in the attribute macro `channel` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `fn(dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {bad_channel_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {bad_channel_adapter}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_channel9.rs:23:1
|
19 | / #[channel {
Expand All @@ -37,7 +37,7 @@ error[E0277]: the trait bound `fn(dropshot::Query<QueryParams>, WebsocketUpgrade
23 | async fn bad_channel(
| ^^^^^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {bad_channel_adapter}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(dropshot::Query<QueryParams>, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {bad_channel_adapter}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_trait_channel17.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ note: required by a bound in `ApiEndpoint::<StubContext>::new_for_types`
| FuncParams: RequestExtractor + 'static,
| ^^^^^^^^^^^^^^^^ required by this bound in `ApiEndpoint::<StubContext>::new_for_types`

error[E0277]: the trait bound `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {two_websocket_channels_adapter::<ServerImpl>}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {two_websocket_channels_adapter::<ServerImpl>}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_trait_channel17.rs:15:5
|
10 | #[dropshot::api_description]
Expand All @@ -48,7 +48,7 @@ error[E0277]: the trait bound `fn(dropshot::RequestContext<<ServerImpl as MyServ
19 | | async fn two_websocket_channels(
| |_________^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {two_websocket_channels_adapter::<ServerImpl>}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {two_websocket_channels_adapter::<ServerImpl>}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
4 changes: 2 additions & 2 deletions dropshot/tests/fail/bad_trait_channel18.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ note: required by a bound in `ApiEndpoint::<StubContext>::new_for_types`
| FuncParams: RequestExtractor + 'static,
| ^^^^^^^^^^^^^^^^ required by this bound in `ApiEndpoint::<StubContext>::new_for_types`

error[E0277]: the trait bound `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {websocket_channel_not_last_adapter::<ServerImpl>}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
error[E0277]: the trait bound `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {websocket_channel_not_last_adapter::<ServerImpl>}: dropshot::handler::HttpHandlerFunc<_, _, _>` is not satisfied
--> tests/fail/bad_trait_channel18.rs:22:5
|
17 | #[dropshot::api_description]
Expand All @@ -87,7 +87,7 @@ error[E0277]: the trait bound `fn(dropshot::RequestContext<<ServerImpl as MyServ
26 | | async fn websocket_channel_not_last(
| |_________^ unsatisfied trait bound
|
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<http::response::Response<Body>, HttpError>> {websocket_channel_not_last_adapter::<ServerImpl>}`
= help: the trait `dropshot::handler::HttpHandlerFunc<_, _, _>` is not implemented for fn item `fn(dropshot::RequestContext<<ServerImpl as MyServer>::Context>, dropshot::WebsocketConnection, WebsocketUpgrade) -> impl Future<Output = Result<dropshot::websocket::SwitchingToWebsocket, HttpError>> {websocket_channel_not_last_adapter::<ServerImpl>}`
note: required by a bound in `ApiEndpoint::<Context>::new`
--> src/api_description.rs
|
Expand Down
Loading