Cannot use different type for HTTP 404.

[citizen-stig]

I want to use progenitor, but generated client uses same response type for HTTP 404 as it has for HTTP 200.

Here is the most simple reproducible spec:

openapi: "3.0.2"
info:
  title: Demo That Triggers It
  version: 0.1.0
  description: "Here you go"
  contact:
    name: Demo
servers:
  - url: http://localhost:12346/
paths:
  /items/{key}:
    get:
      summary: get_state_element
      operationId: get_state_element
      tags: [ "hi-there" ]
      parameters:
        - name: key
          in: path
          required: true
          schema:
            type: string
      responses:
        200:
          $ref: "#/components/responses/StateMapElementResponse2"
        404:
          "$ref": "#/components/responses/NotFound"
components:
  parameters:
    rollup_height:
      name: rollup_height
      in: query
      description: The height of the rollup to query. If not provided, the rollup head is used.
      required: false
      schema:
        type: integer
        minimum: 0
  responses:
    StateMapElementResponse2:
      description: Response containing the value of a `StateMap` element.
      content:
        application/json:
          schema:
            type: object
            properties:
              data:
                type: object
                properties:
                  key:
                    $ref: "#/components/schemas/AnyJsonValue"
                  value:
                    $ref: "#/components/schemas/AnyJsonValue"
                required:
                  - key
                  - value
              meta:
                $ref: "#/components/schemas/AnyJsonValue"
            required:
              - data
    NotFound:
      description: The requested resource was not found.
      content:
        application/json:
          schema:
            type: object
            properties:
              errors:
                type: array
                items:
                  $ref: "#/components/schemas/Error"
              meta:
                $ref: "#/components/schemas/AnyJsonValue"
            required:
              - errors
  schemas:
    AnyJsonValue:
      description: "Any JSON type, including null."
      nullable: true
      oneOf:
        - type: string
          title: "String"
        - type: number
          title: "Number"
        - type: boolean
          title: "Boolean"
        - type: array
          title: "Array"
          items:
            x-stainless-any: true
        - type: object
          title: "Object"
          additionalProperties: true
    Error:
      type: object
      properties:
        status:
          type: integer
          format: int32
          description: HTTP status code related to this error
        title:
          type: string
        details:
          $ref: "#/components/schemas/AnyJsonValue"
      required:
        - status
        - title
        - details

Here is part of the codegen.rs:

impl Client {
    pub async fn get_state_element<'a>(
        &'a self,
        key: &'a str,
    ) -> Result<
        ResponseValue<types::GetStateElementResponse>,
        Error<types::GetStateElementResponse>,
    > { todo!("omitted"); }
}    

I am struggling to find what causes the problem, that Error variant is the same as success.

[ahl]

That is messed up! This is, effectively, the result of a name conflict when making up names. (Note that naming being hard is issue oxidecomputer/typify#1 for a reason.)

Because the types for StateMapElementResponse2 and NotFound are complex, we need to create a type with a name, and the name we choose for both is GetStateElementResponse. Because the logic to compare our internal state is janky, we erroneously deduplicate these:

https://github.com/oxidecomputer/typify/blob/main/typify-impl/src/lib.rs#L834-L846

This is lousy! There's work ongoing to fix this, but it's not going to be done imminently (candidly that part is a side project for me since it's not particularly relevant to our use of progenitor/typify at Oxide). See oxidecomputer/typify#579 and oxidecomputer/typify#195.

There's an easy workaround that, I hope, might be viable for you:

@@ -40,38 +40,42 @@
       content:
         application/json:
           schema:
-            type: object
-            properties:
-              data:
-                type: object
-                properties:
-                  key:
-                    $ref: "#/components/schemas/AnyJsonValue"
-                  value:
-                    $ref: "#/components/schemas/AnyJsonValue"
-                required:
-                  - key
-                  - value
-              meta:
-                $ref: "#/components/schemas/AnyJsonValue"
-            required:
-              - data
+            $ref: "#/components/schemas/StateMapElementResponse2"
     NotFound:
       description: The requested resource was not found.
       content:
         application/json:
           schema:
-            type: object
-            properties:
-              errors:
-                type: array
-                items:
-                  $ref: "#/components/schemas/Error"
-              meta:
-                $ref: "#/components/schemas/AnyJsonValue"
-            required:
-              - errors
+            $ref: "#/components/schemas/NotFound"
   schemas:
+    StateMapElementResponse2:
+      type: object
+      properties:
+        data:
+          type: object
+          properties:
+            key:
+              $ref: "#/components/schemas/AnyJsonValue"
+            value:
+              $ref: "#/components/schemas/AnyJsonValue"
+          required:
+            - key
+            - value
+        meta:
+          $ref: "#/components/schemas/AnyJsonValue"
+      required:
+        - data
+    NotFound:
+      type: object
+      properties:
+        errors:
+          type: array
+          items:
+            $ref: "#/components/schemas/Error"
+        meta:
+          $ref: "#/components/schemas/AnyJsonValue"
+      required:
+        - errors
     AnyJsonValue:
       description: "Any JSON type, including null."
       nullable: true

In short, name the types; this produces this:

impl Client {
    ///get_state_element
    ///         
    ///Sends a `GET` request to `/items/{key}`
    pub async fn get_state_element<'a>(
        &'a self,
        key: &'a str,
    ) -> Result<ResponseValue<types::StateMapElementResponse2>, Error<types::NotFound>> {

There's potentially some work we could do before the more significant restructuring to help here. For example, we could start to do a little name uniquification by checking to see if multiple responses are going to (or might) need generated named; in such a case, we could come up with unique names:

https://github.com/oxidecomputer/progenitor/blob/main/progenitor-impl/src/method.rs#L478-L509

While naming uniqueness is a general (and generally annoying) problem. It might make sense to have a specific solution here since we have local context about how we might distinguish names. For good or ill, we've already resolved the $ref to the response component (though one could imagine holding that around in some sort of "context" stack) so that semantic name is not easy to recover. However, we do have context such as the response code. It would be feasible to have type names such as GetStateElementSuccess / GetStateElementResponse and GetStateElementError or GetStateElement200 and GetStateElement404.

Please let us know if the workaround suggested suffices or if some local name awareness would be helpful.

[neysofu]

Is this a duplicate of #344?