oxidecomputer / oxidecomputer/progenitor

could handle 300-level responses better

Open
#451 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
1k
Forks
136
Avg merge
8h 36m
Merged PRs (30d)
14

Description

I have an API endpoint that returns a 303 redirect on success with an empty body. The response contains a header that I want to be able to get at. I'm not sure this is intended to be supported with Progenitor. However, the current failure mode is:

    Unexpected Response: Response { url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Ipv4(127.0.0.1)), port: Some(12220), path: "//login/test-suite-silo/local", query: None, fragment: None }, status: 303, headers: {"location": "/", "set-cookie": "session=cdb10d5430e901b00050ff41480ad0afdc6869ee; Path=/; HttpOnly; SameSite=Lax; Max-Age=28800", "x-request-id": "6a543c5a-bc48-43ac-8cb2-b0352d08b550", "content-length": "0", "date": "Tue, 02 May 2023 00:00:53 GMT"} }

Here's the spec for this endpoint:

    "/login/{silo_name}/local": {
      "post": {
        "tags": [
          "login"
        ],
        "summary": "Authenticate a user via username and password",
        "operationId": "login_local",
        "parameters": [
          {
            "in": "path",
            "name": "silo_name",
            "required": true,
            "schema": {
              "$ref": "#/components/schemas/Name"
            }
          }
        ],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/UsernamePasswordCredentials"
              }
            }
          },
          "required": true
        },
        "responses": {
          "303": {
            "description": "redirect (see other)",
            "headers": {
              "location": {
                "description": "HTTP \"Location\" header",
                "style": "simple",
                "required": true,
                "schema": {
                  "type": "string"
                }
              }
            }
          },
          "4XX": {
            "$ref": "#/components/responses/Error"
          },
          "5XX": {
            "$ref": "#/components/responses/Error"
          }
        }
      }
    },

which shows that it has a 303 response.

It might be fair to say that 300-level responses aren't supported, especially since reqwest follows 300s by default, so in principle Progenitor wouldn't usually see them. In this case, I configured my reqwest client not to follow the redirect so that I could get the header I want back. If that's not supported though then I think we should at least produce a clearer error (e.g., 300-level response codes are not supported). We may even want to fail to generate a client for a spec with a 300-level response? Or emit a warning and skip that endpoint altogether?

@ahl noticed we don't handle 300-level responses here:
https://github.com/oxidecomputer/oxide.rs/blob/d007f734e8f379d8a174d45c18aabd67ec24eef5/sdk/src/generated_sdk.rs#L20053-L20062

Gory details (which I hope aren't necessary, but if you want to repro this): https://github.com/davepacheco/oxide-client-test

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with sdk/src/generated_sdk.rs around the linked response-handling code and inspect how Progenitor treats non-2xx responses. Reproduce the 303 case using the linked oxide-client-test details or an equivalent reqwest client configuration. The issue is resolved when 300-level responses are either supported for accessing headers or rejected with a clear, tested error, according to a decided behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.