Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@hey-api/client-fetch: Error is unknown when using default #691

Closed
mlankamp opened this issue Jun 18, 2024 · 5 comments · Fixed by #744
Closed

@hey-api/client-fetch: Error is unknown when using default #691

mlankamp opened this issue Jun 18, 2024 · 5 comments · Fixed by #744
Labels
bug 🔥 Something isn't working

Comments

@mlankamp
Copy link
Contributor

Description

The I use a non-200 response code, the Error object is created with a specific type. But if I use response code default the response is created as unknown

OpenAPI specification (optional)

openapi: 3.0.1
info:
  title: Testing
  version: v1
paths:
  '/specific/{id}':
    post:
      tags:
        - Testing
      operationId: SpecficError
      parameters:
        - name: id
          in: path
          required: true
          schema:
            type: string
            format: uuid
      responses:
        '204':
          description: No Content
        '500':
          description: Unexpected error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ProblemDetails'
  '/default/{id}':
    post:
      tags:
        - Testing
      operationId: DefaultError
      parameters:
        - name: id
          in: path
          required: true
          schema:
            type: string
            format: uuid
      responses:
        '204':
          description: No Content
        default:
          description: Unexpected error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ProblemDetails'

components:
  schemas:
    ProblemDetails:
      type: object
      properties:
        type:
          type: string
          nullable: true
        title:
          type: string
          nullable: true
        status:
          type: integer
          format: int32
          nullable: true
        detail:
          type: string
          nullable: true
        instance:
          type: string
          nullable: true
      additionalProperties: { }

Configuration

import { defineConfig } from '@hey-api/openapi-ts';

export default defineConfig({
  input: 'source/swagger.yaml',
  output: {
    path: 'output',
    format: false,
    lint: false
  } ,
  client: '@hey-api/client-fetch',
  schemas: false,
  types: {
    enums: 'javascript'
  },
  services: {
    asClass: true,
  }
});

System information (optional)

Example repo: https://github.com/mlankamp/openapi

@mlankamp mlankamp added the bug 🔥 Something isn't working label Jun 18, 2024
@mlankamp
Copy link
Contributor Author

mlankamp commented Jun 18, 2024

I think the problem is caused by

const errorResults = getErrorResponses(operation.errors);

The causes the error to be filtered twice, because the operation.errors is already assigned with operation.errors = getErrorResponses(operationResponses);

Changing line 403 to : const errorResults = operation.errors; fixes the problem.

@mrlubos
Copy link
Contributor

mrlubos commented Jun 19, 2024

cc @gergan

@gergan
Copy link
Contributor

gergan commented Jun 20, 2024

Yes I think @mlankamp is correct -> operation.errors are set in operation.ts and they are already filtered through getErrorResponses so no need to filter them once more. It seems that removing the function call is the correct thing to do. Albeit my question is if the logic for the default in

export const inferDefaultResponse = (
is correct. If I understand the specification correctly https://github.com/OAI/OpenAPI-Specification/blob/main/versions/2.0.md#fixed-fields-9 or https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#fixed-fields-14 it seems that default covers all the responses, which are not declared. But inferDefaultResponse tries to identify if there are declared success responses and in this way decides if the default is only for error responses...

@sbking
Copy link

sbking commented Jul 3, 2024

I'm running into this as well - unfortunately it causes issues with discriminating the data type:

const { data, error } = await foo();
if (error) throw error;
data; // TypeScript still thinks this is `Response | undefined` because it can't discriminate the type of `data` based on the `unknown` error type.

@mrlubos
Copy link
Contributor

mrlubos commented Jul 3, 2024

Sorry for the delay all, try with the next version once it's released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🔥 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants