Skip to content

Treat connection upgrade (HTTP 101) as normal success.#1320

Open
lifning wants to merge 3 commits intomainfrom
switching-protocols-fix
Open

Treat connection upgrade (HTTP 101) as normal success.#1320
lifning wants to merge 3 commits intomainfrom
switching-protocols-fix

Conversation

@lifning
Copy link
Contributor

@lifning lifning commented Feb 27, 2026

This fixes a bug wherein client code would be generated for a 2XX-range response on WebSocket endpoints, which in turn would unnecessarily trip an assert!(response_types.len() <= 1) in extract_responses when explicit status codes are provided in the OpenAPI document (oxidecomputer/dropshot#1554). This also fixes a bug in such a scenario wherein all modeled response codes, including HTTP errors, would be set to OperationResponseKind::Upgrade.

Prior to this change, progenitor (incorrectly) outputs:

/// Sends a `GET` request to
/// `/v1/instances/{instance}/serial-console/stream`
pub async fn send(self) -> Result<ResponseValue<reqwest::Upgraded>, Error<reqwest::Upgraded>> {
    // [snip...]
    match response.status().as_u16() {
        101u16 => ResponseValue::upgrade(response).await,
        200..=299 => ResponseValue::upgrade(response).await,
        _ => Err(Error::UnexpectedResponse(response)),
    }
}

With this change plus the explicitly-modeled error codes ( oxidecomputer/dropshot#1554 ), progenitor outputs:

/// Sends a `GET` request to
/// `/v1/instances/{instance}/serial-console/stream`
pub async fn send(self) -> Result<ResponseValue<reqwest::Upgraded>, Error<()>> {
    // [snip...]
    match response.status().as_u16() {
        101u16 => ResponseValue::upgrade(response).await,
        400u16..=499u16 => Err(Error::ErrorResponse(
            ResponseValue::from_response(response).await?,
        )),
        500u16..=599u16 => Err(Error::ErrorResponse(
            ResponseValue::from_response(response).await?,
        )),
        _ => Err(Error::UnexpectedResponse(response)),
    }
}

And in turn, the CLI, which previously would only close with Connection lost. on errors prior to the server's accepting the protocol change, with this version of progenitor would now instead output, for example:

$ oxide instance serial console --project proj --instance inst
Connection closed: Error                                                                                                                                                                                                                                                       
Server error: Object (of type ByName("foo")) not found: instance

This fixes a bug wherein client code would be generated for a 2XX-range
response on WebSocket endpoints, which in turn would unnecessarily trip
an `assert!(response_types.len() <= 1)` in `extract_responses` when
explicit status codes are provided in the OpenAPI document.
(This also fixes a bug in such a scenario wherein *all* responses,
including HTTP errors, would be set to Upgrade type)

(As in dropshot#1548)
@lifning lifning force-pushed the switching-protocols-fix branch from e0c9453 to 326b6f1 Compare March 3, 2026 05:32
(and manually update sample OpenAPI JSON for Nexus and Propolis accordingly)
@lifning lifning force-pushed the switching-protocols-fix branch from 326b6f1 to 2fd36dd Compare March 3, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants