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

* Treat connection upgrade (HTTP 101) as normal success.

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)

* Per ahl PR feedback: Forego supporting the erroneous OpenAPI format

(and manually update sample OpenAPI JSON for Nexus and Propolis accordingly)

* Old, incorrect schema for WebSocket endpoints is explicitly an error

* Further feedback: Support the previous format after all, and test that it continues to work

* clean up

---------

Co-authored-by: lif <>
Co-authored-by: Adam H. Leventhal <ahl@oxide.computer>

Author: lifning

progenitor-impl/src/method.rs

@@ -443,7 +443,13 @@ impl Generator {
                     },
                 ))
                 .map(|v: Result<(OperationResponseStatus, &Response)>| {
-                    let (status_code, response) = v?;
+                    let (mut status_code, response) = v?;
+
+                    // A previous version of dropshot websockets failed to
+                    // properly report responses; clean this up here.
+                    if dropshot_websocket && status_code == OperationResponseStatus::Default {
+                        status_code = OperationResponseStatus::Code(101);
+                    }
 
                     // We categorize responses as "typed" based on the
                     // "application/json" content type, "upgrade" if it's a
@@ -472,7 +478,7 @@ impl Generator {
                         };
 
                         OperationResponseKind::Type(typ)
-                    } else if dropshot_websocket {
+                    } else if status_code == OperationResponseStatus::Code(101) {
                         OperationResponseKind::Upgrade
                     } else if response.content.first().is_some() {
                         OperationResponseKind::Raw
@@ -484,6 +490,7 @@ impl Generator {
                     if matches!(
                         status_code,
                         OperationResponseStatus::Default
+                            | OperationResponseStatus::Code(101)
                             | OperationResponseStatus::Code(200..=299)
                             | OperationResponseStatus::Range(2)
                     ) {
@@ -516,15 +523,6 @@ impl Generator {
             });
         }
 
-        // Must accept HTTP 101 Switching Protocols
-        if dropshot_websocket {
-            responses.push(OperationResponse {
-                status_code: OperationResponseStatus::Code(101),
-                typ: OperationResponseKind::Upgrade,
-                description: None,
-            })
-        }
-
         let dropshot_paginated = self.dropshot_pagination_data(operation, &params, &responses);
 
         if dropshot_websocket && dropshot_paginated.is_some() {
@@ -533,6 +531,17 @@ impl Generator {
                 operation_id
             )));
         }
+        if dropshot_websocket
+            && responses
+                .iter()
+                .find(|r| r.status_code == OperationResponseStatus::Code(101))
+                .is_none()
+        {
+            return Err(Error::InvalidExtension(format!(
+                "websocket endpoint {:?} must include an explicit 101 response code",
+                operation_id
+            )));
+        }
 
         Ok(OperationMethod {
             operation_id: sanitize(operation_id, Case::Snake),

progenitor-impl/tests/output/src/nexus_builder.rs

@@ -26332,6 +26332,26 @@ impl Client {
         builder::InstanceSerialConsoleStream::new(self)
     }
 
+    ///Connect to an instance's serial console
+    ///
+    ///Use `GET /v1/instances/{instance}/serial-console/stream` instead
+    ///
+    ///Sends a `GET` request to
+    /// `/organizations/{organization_name}/projects/{project_name}/instances/
+    /// {instance_name}/serial-console/stream_v2`
+    ///
+    ///```ignore
+    /// let response = client.instance_serial_console_stream_v2()
+    ///    .organization_name(organization_name)
+    ///    .project_name(project_name)
+    ///    .instance_name(instance_name)
+    ///    .send()
+    ///    .await;
+    /// ```
+    pub fn instance_serial_console_stream_v2(&self) -> builder::InstanceSerialConsoleStreamV2<'_> {
+        builder::InstanceSerialConsoleStreamV2::new(self)
+    }
+
     ///Boot an instance
     ///
     ///Use `POST /v1/instances/{instance}/start` instead
@@ -35228,9 +35248,7 @@ pub mod builder {
         ///Sends a `GET` request to
         /// `/organizations/{organization_name}/projects/{project_name}/
         /// instances/{instance_name}/serial-console/stream`
-        pub async fn send(
-            self,
-        ) -> Result<ResponseValue<reqwest::Upgraded>, Error<reqwest::Upgraded>> {
+        pub async fn send(self) -> Result<ResponseValue<reqwest::Upgraded>, Error<()>> {
             let Self {
                 client,
                 organization_name,
@@ -35277,7 +35295,118 @@ pub mod builder {
             let response = result?;
             match response.status().as_u16() {
                 101u16 => ResponseValue::upgrade(response).await,
-                200..=299 => ResponseValue::upgrade(response).await,
+                _ => Err(Error::UnexpectedResponse(response)),
+            }
+        }
+    }
+
+    ///Builder for [`Client::instance_serial_console_stream_v2`]
+    ///
+    ///[`Client::instance_serial_console_stream_v2`]: super::Client::instance_serial_console_stream_v2
+    #[derive(Debug, Clone)]
+    pub struct InstanceSerialConsoleStreamV2<'a> {
+        client: &'a super::Client,
+        organization_name: Result<types::Name, String>,
+        project_name: Result<types::Name, String>,
+        instance_name: Result<types::Name, String>,
+    }
+
+    impl<'a> InstanceSerialConsoleStreamV2<'a> {
+        pub fn new(client: &'a super::Client) -> Self {
+            Self {
+                client: client,
+                organization_name: Err("organization_name was not initialized".to_string()),
+                project_name: Err("project_name was not initialized".to_string()),
+                instance_name: Err("instance_name was not initialized".to_string()),
+            }
+        }
+
+        pub fn organization_name<V>(mut self, value: V) -> Self
+        where
+            V: std::convert::TryInto<types::Name>,
+        {
+            self.organization_name = value
+                .try_into()
+                .map_err(|_| "conversion to `Name` for organization_name failed".to_string());
+            self
+        }
+
+        pub fn project_name<V>(mut self, value: V) -> Self
+        where
+            V: std::convert::TryInto<types::Name>,
+        {
+            self.project_name = value
+                .try_into()
+                .map_err(|_| "conversion to `Name` for project_name failed".to_string());
+            self
+        }
+
+        pub fn instance_name<V>(mut self, value: V) -> Self
+        where
+            V: std::convert::TryInto<types::Name>,
+        {
+            self.instance_name = value
+                .try_into()
+                .map_err(|_| "conversion to `Name` for instance_name failed".to_string());
+            self
+        }
+
+        ///Sends a `GET` request to
+        /// `/organizations/{organization_name}/projects/{project_name}/
+        /// instances/{instance_name}/serial-console/stream_v2`
+        pub async fn send(self) -> Result<ResponseValue<reqwest::Upgraded>, Error<types::Error>> {
+            let Self {
+                client,
+                organization_name,
+                project_name,
+                instance_name,
+            } = self;
+            let organization_name = organization_name.map_err(Error::InvalidRequest)?;
+            let project_name = project_name.map_err(Error::InvalidRequest)?;
+            let instance_name = instance_name.map_err(Error::InvalidRequest)?;
+            let url = format!(
+                "{}/organizations/{}/projects/{}/instances/{}/serial-console/stream_v2",
+                client.baseurl,
+                encode_path(&organization_name.to_string()),
+                encode_path(&project_name.to_string()),
+                encode_path(&instance_name.to_string()),
+            );
+            let mut header_map = ::reqwest::header::HeaderMap::with_capacity(1usize);
+            header_map.append(
+                ::reqwest::header::HeaderName::from_static("api-version"),
+                ::reqwest::header::HeaderValue::from_static(super::Client::api_version()),
+            );
+            #[allow(unused_mut)]
+            let mut request = client
+                .client
+                .get(url)
+                .headers(header_map)
+                .header(::reqwest::header::CONNECTION, "Upgrade")
+                .header(::reqwest::header::UPGRADE, "websocket")
+                .header(::reqwest::header::SEC_WEBSOCKET_VERSION, "13")
+                .header(
+                    ::reqwest::header::SEC_WEBSOCKET_KEY,
+                    ::base64::Engine::encode(
+                        &::base64::engine::general_purpose::STANDARD,
+                        ::rand::random::<[u8; 16]>(),
+                    ),
+                )
+                .build()?;
+            let info = OperationInfo {
+                operation_id: "instance_serial_console_stream_v2",
+            };
+            client.pre(&mut request, &info).await?;
+            let result = client.exec(request, &info).await;
+            client.post(&result, &info).await?;
+            let response = result?;
+            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)),
             }
         }
@@ -47944,9 +48073,7 @@ pub mod builder {
 
         ///Sends a `GET` request to
         /// `/v1/instances/{instance}/serial-console/stream`
-        pub async fn send(
-            self,
-        ) -> Result<ResponseValue<reqwest::Upgraded>, Error<reqwest::Upgraded>> {
+        pub async fn send(self) -> Result<ResponseValue<reqwest::Upgraded>, Error<types::Error>> {
             let Self {
                 client,
                 instance,
@@ -47996,7 +48123,12 @@ pub mod builder {
             let response = result?;
             match response.status().as_u16() {
                 101u16 => ResponseValue::upgrade(response).await,
-                200..=299 => 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)),
             }
         }