Should "default" spec responses throw an error?

[dudasaus]

Our spec uses "default" responses for errors per the swagger docs.

The example:

  /templates:
    get:
      operationId: listTemplates
      summary: Retrieve a list of Templates
      description: Returns an array of Template objects
      parameters:
        - in: query
          name: view
          required: false
          schema:
            $ref: '#/components/schemas/TemplateView'
      responses:
        '200':
          description: Successful retrieval of templates
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Template'
        default:
          description: Server error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Error'

With release 0.43 which began to include default response types, the generated response type include the default response type:

export type ListTemplatesResponse = Array<Template> | Error;

Which means I now need to type check my response instead of catching an error.

I'm not sure if my use case is the same way everyone uses the default response in the spec. My question is:

Should default response types from the spec be included in the generated TS response type? Or should it throw an error and be treated like response codes 400+? Or, maybe a config option to choose the behavior?

This is related to #500

[mrlubos]

Yes, this is a bit tricky since default response doesn't have to describe errors. In fact, before I made that change, default response was assumed to be status 200 which was also incorrect. Perhaps a simple solution for now would be to check whether 200 is defined and if so, assume default response describes errors.

[seriouslag]

Personally I feel all 2XX status codes responses should be included in the response unless there is only a default then whatever is in the default. All 400,500 status codes should throw by default.

If someone wants to handle 400/500 errors without catching then a config option to handle those would be good. I would like to see that option only when using the serviceResponse='response' as having the code is almost required to know which object we are dealing with.

When serviceResponse='body' and the type is every response type it is difficult to determine which type we are working with, IMO.

[dudasaus]

It's hard to predict everyone's use case for default, which leads me to believe a config option might be the best way to support the most users.

Checking for a 2xx status code, and if so assuming default describes errors would certainly cover my case.

[schehlmj]

I also would prefer to have an error thrown-- especially since in strict TypeScript instead of when having a success that I am guaranteed to have my success response type back, I have to first check if it is not undefined. Also, I still have to check for errors for communication errors, etc. I have a lot of code with the older openapi from earlier versions that I can switch to using the treeshaking style without too much work, but this way of handling errors would cause a lot of extra work to handle this... Or I change something like this:

const fooResponse  = await oldClient.oldController.getFoo();

to

const {data: fooResponse, error} = await getFoo();
if (error) throw error;
if (fooResponse === undefined) throw Error('Server error: received undefined response data');

it's not the worst situation, but I'd rather not have to add those 2 lines.

Of course, likely if the fetch-client did throw errors, then it would wrap the api error type in something like a RequestError type, and so, I would have to make changes there:

catch (error) {
   if (error instanceof RequestError) {
       console.log(error.request);
       console.log(error.response);
       handleError(error.error);
   } 
}