Explicitly include HTTP statuses in OpenAPI document for WebSockets#1554
Explicitly include HTTP statuses in OpenAPI document for WebSockets#1554
Conversation
rather than wildcard default, which made generated clients not expect any error responses from the server at all. (#1548)
for WebSocket channels with explicit StatusCodes, and allowing descriptions in freeform responses
| /// [WebsocketUpgrade::handle]'s return type. | ||
| /// The `#[endpoint]` handler must return the value returned by | ||
| /// [WebsocketUpgrade::handle]. (This is done for you by `#[channel]`.) |
There was a problem hiding this comment.
I don't understand the situation in which a user would care about this type.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Do we have any uses that don't do (or shouldn't do) pre-upgrade error reporting?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
What happens e.g. in the CLI if I try to view the serial console of a instance that doesn't exist.
2 participants
Explicitly include HTTP statuses in OpenAPI document for WebSockets, rather than wildcard default, which made generated clients not expect any error responses from the server at all.
Also fixes
descriptionbeing unconditionally set to empty string in endpoints without a JSON schema (such as WebSocket endpoints).oxidecomputer/progenitor#1320 should be applied prior to shipping this change.
This includes an API change, albeit with only minor impact: The pub type of
dropshot::WebsocketEndpointResultchanges from beingResult<Response<Body>, HttpError>, to instead beResult<SwitchingToWebsocket, HttpError>.The two places in our projects where this type appears would not be intrinsically broken by this change, as their only usage of it is covered by the return type of our
WebsocketUpgrade::handlechanging accordingly:https://github.com/oxidecomputer/omicron/blob/339f7e4dca594f1009d885a0d9605f0df508b442/gateway/src/http_entrypoints.rs#L429-L460
https://github.com/oxidecomputer/sush/blob/5c6b64e898a906cad24618a3a82cfc4eba4908d3/server/src/server.rs#L120-L143
(issue #1548)