oxidecomputer / oxidecomputer/progenitor
could handle 300-level responses better
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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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