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

Allow for multiple response schemas for each response code #270

Closed
aroemen opened this issue Feb 9, 2015 · 81 comments
Closed

Allow for multiple response schemas for each response code #270

aroemen opened this issue Feb 9, 2015 · 81 comments

Comments

@aroemen
Copy link

aroemen commented Feb 9, 2015

Example

  responses:
    200:
      description: An array of events
      schema:
        type: array
        items:
          $ref: '#/definitions/Event'
    200:
      description: |
        An array of flattened events. This is the response
        object when singleEvents is True.
      schema:
        type: array
        items:
          $ref: '#/definitions/EventFlat'
    default:
      description: Unexpected error
      schema:
        $ref: '#/definitions/Error'
@webron
Copy link
Member

webron commented Feb 9, 2015

I doubt we'll support that any time soon. One of the leading principles we have behind Swagger is API determinism. By saying that a single response may return various object types, that pretty much breaks that principle.

@theganyo
Copy link

theganyo commented Feb 9, 2015

Standing on idealism on this point can, ironically, lead to less determinism: Because Swagger doesn't allow multiple return values, one has to code APIs that need this capability to return generic responses.

@webron
Copy link
Member

webron commented Feb 9, 2015

Generic responses don't necessarily mean less determinism (though they can, depending on the level of abstraction). That said, I could argue it may imply a faulty API design in general, in many case ;)

@theganyo
Copy link

theganyo commented Feb 9, 2015

You could. :) However, as I'm sure you're aware, that's a rigid and isolating argument that excludes Swagger from being able to support many APIs (and API styles) that exist in the real world.

@fehguy
Copy link
Contributor

fehguy commented Feb 9, 2015

I do think it's worth exploring how it could be supported in the future. But I will add that, the goal of swagger is not to describe every single API case out there. Some will be left behind, for the sake of efficiency in the ones that we can support with a full toolchain.

@webron
Copy link
Member

webron commented Feb 9, 2015

It's a matter of finding the balance between requirements, support, elegance and much more. The spec has evolved over the years and obviously supports today much more than it had initially.

My main concern with breaking away from the determinism principle is that it'll lead to changes all across the spec, and may increase its complexity substantially. This is not just an issue for the spec itself but also for the support tools. If we do it, we need to do it carefully and take it all under consideration.

@sebderenty
Copy link

Just to support this request.
In my API, the "get user" endpoint (GET /users/{user_id}) returns the User object if the requested user is the current user and it returns a smallest "PublicUser" object otherwise.
The format of the URL seems semantically correct for the two types of object returned.

@webron
Copy link
Member

webron commented Mar 27, 2015

@sebderenty - But how would you "document" these two use cases? How can you generate both client and server code that can behave this way from a Swagger definition?

@dannylevenson-ps
Copy link

I have a similar problem/concern. We recently updated the version of swagger-tools we are using which moved us from version 1.2 of the Swagger schema to version 2.0. This caused my API spec to no longer validate correctly. I have a URL path that looks like this:

GET /quote/{citvId}/group/{groupId}

Since this is a front end for a third party service, the error information I am able to provide is quite different for an invalid/unrecognized citvId value versus an invalid groupId value. In either case, I need to return a 400 status code. I originally had my responses defined like this:

"responses": {
    "200": {
        "description": "Successful response",
        "schema": {
            "$ref": "#/definitions/GroupDetailed"
        }
    },
    "400": {
        "description": "Unrecognized quote or group ID",
        "schema": {
            "oneOf": [
                {
                    "$ref": "#/definitions/UnrecognizedQuoteId"
                },
                {
                    "$ref": "#/definitions/UnrecognizedGroupId"
                }
            ]
        }
    }

and in the definitions:

"UnrecognizedQuoteId": {
    "required": [
        "error"
    ],
    "properties": {
        "error": {
            "type": "object",
            "required": [
                "httpStatus",
                "dateTime",
                "enservioStatusCode",
                "enservioMessage"
            ],
            "properties": {
                "httpStatus": {
                    "type": "integer",
                    "description": "This will be 400 for this type of error"
                },
                "dateTime": {
                    "type": "string",
                    "description": "ISO format timestamp for request"
                },
                "serviceStatusCode": {
                    "type": "string",
                    "description": "Numeric status code returned by third party service (e.g. 1106)"
                },
                "serviceMessage": {
                    "type": "string",
                    "description": "Message from third party service"
                }
            }
        }
    }
},
"UnrecognizedGroupId": {
    "required": [
        "error"
    ],
    "properties": {
        "error": {
            "type": "object",
            "required": [
                "citvId",
                "httpStatus",
                "dateTime",
                "validation"
            ],
            "properties": {
                "citvId": {
                    "type": "string",
                    "description": "The citvId that was specified in the request."
                },
                "httpStatus": {
                    "type": "integer",
                    "description": "This will be 400 for validation errors"
                },
                "dateTime": {
                    "type": "string",
                    "description": "ISO format timestamp for request"
                },
                "validation": {
                    "type": "string",
                    "description": "Message indicating the unrecognized group ID"
                }
            }
        }
    }
},

Since responses no longer allow oneOf for the schema, I have had to collapse these two errors into a single error like this:

"responses": {
    "200": {
        "description": "Successful response",
        "schema": {
            "$ref": "#/definitions/GroupDetailed"
        }
    },
    "400": {
        "description": "Unrecognized citvId or groupId",
        "schema": {
            "$ref": "#/definitions/UnrecognizedQuoteOrGroupId"
        }
    }
}

with the definitions collapsed like this:

"UnrecognizedQuoteOrGroupId": {
    "required": [
        "error"
    ],
    "properties": {
        "error": {
            "type": "object",
            "required": [
                "httpStatus",
                "dateTime",
            ],
            "properties": {
                "citvId": {
                    "type": "string",
                    "description": "The citvId that was specified in the request in the case of a valid citvId but igroupId."
                },
                "httpStatus": {
                    "type": "integer",
                    "description": "This will be 400 for this type of error"
                },
                "dateTime": {
                    "type": "string",
                    "description": "ISO format timestamp for request"
                },
                "serviceStatusCode": {
                    "type": "string",
                    "description": "Numeric status code returned by third party service (e.g. 1106) for an invalid citvId"
                },
                "serviceMessage": {
                    "type": "string",
                    "description": "Message from third party service for an invalid citvId which translates to their risk ID"
                },
                "validation": {
                    "type": "string",
                    "description": "Message indicating the unrecognized groupId in the case of a valid citvId but igroupId"
                }
            }
        }
    }
},

My API is behaving in a completely deterministic manner. If the citvId is invalid, one type of error information is available. Alternatively, if the groupId is invalid, slightly different type of error information is available. In order to provide the various information in a single response definition, I have had to introduce ambiguity into my definitions by removing the uncommon properties from the required list and explaining when certain properties will be available in the response. In my opinion, this is less deterministic and more confusing than when oneOf was available to allow for multiple schemas for the 400 response.

I believe that restricting the responses to a single schema per status code is overly rigid and restrictive.

Please do not respond by simply saying, "I could argue it may imply a faulty API design in general" as you did to a previous post. If you feel that this API design is faulty, please explain the correct way to handle a situation like my case.

@webron
Copy link
Member

webron commented Apr 24, 2015

@dannylevenson-ps - thank you for the elaborate use case, much appreciated.

To get it out of the way oneOf was never supported. It wasn't supported in 1.2 nor in 2.0. There may have been a bug in the JSON Schema which allowed for it in swagger-tools, but that does not mean it was supported. The source of truth is the spec itself and it's clear oneOf is not part of it.

You're right about the API behaving in a deterministic manner, but it behaves in a deterministic manner to the producer (i.e. the server). At the end of the day, the API (and its documentation) aim to serve the consumer (i.e. the client), and from the client's point of view, it's a non-deterministic behavior. The client has no way of knowing that citvId or groupId are invalid. Had it known that, I imagine it wouldn't have tried to invoke the API call in the first place - so it has no way of knowing what error to expect. In fact, by using oneOf, you're saying "you'll be getting this OR that" which is... non-deterministic.

If your errors had a shared property that has a unique value per error, that could have acted as a discriminator and you could have used inheritance to document it. Unfortunately, httpStatus in your example, while shared, has the same value for both errors.

I do have some ideas how to deal with situations that are not fully deterministic, but that's more of a general solution and not specific here. Unfortunately, it's far from being baked but I'll get around to it as well.

FWIW, the comment "I could argue it may imply a faulty API design in general" was a bit out of frustration and not directed at anyone in particular. I don't think it's our job here to comment on one API paradigm or the other. Just like anyone else, I have my opinions and can share them if asked, but telling people how to design their APIs is not within the scope of our work. As @fehguy mentioned, we're never going to cover all use cases, but we definitely want to expand the spec carefully to accommodate for more.

@dannylevenson-ps
Copy link

@webron - you said, "The client has no way of knowing what citvId or groupId are invalid. Had it known that, I imagine it wouldn't have tried to invoke the API call in the first place - so it has no way of knowing what error to expect."

In a perfect world, yes, the client would invoke this API if and only if it knew that both the citvId and groupId were valid. And, if that were true, my server code would not have to do parameter validation on the invocation for either parameter. However, in order to be robust, my server code must validate the multiple inputs for this call and the error responses need to be different depending on which value is invalid. I do not agree that this is non-deterministic from the client's point of view. If the client feeds the API an invalid citvId it gets one type of error response. If the client feeds the API an invalid groupId it gets a different type of error response. The only way it would be non-deterministic from the client's perspective is if the client could get either this or that given the same erroneous input for two different calls.

If the client is using the API properly, it should always have valid values for both, but it is unrealistic to state that a client will only ever invoke the API call with valid values for all input parameters.

@webron
Copy link
Member

webron commented Apr 24, 2015

It's deterministic if you combine the error with the cause (as you mention above). Since there's no way to link the error to the cause, it's becoming an either/or situation. You know the link, but if, for example, there's an auto-generated client, who would it know how to handle it?

@mclate
Copy link

mclate commented Jun 22, 2015

Also would vote for adding multiple responses with the same code. The best example would be any 400 (Bad request) error which can be caused by many reasons. Take in example validation errors: missing fields, extra fields passed or simply wrong data provided - these all are different errors grouped under the same status code where exact error is returned in response playload, which may vary for each situation. This is still considered as deterministic behaviour as it is uniquely defined in response playload.

@webron
Copy link
Member

webron commented Jun 22, 2015

You can still document it, assuming you follow a common structure to all the errors (as you probably should).

@dannylevenson-ps
Copy link

I'm getting angry all over again. I agree with Jack and and disagree with Ron. Even if you use a common structure for all errors, there are going to be cases where more or different information is available based on the erroneous input. All bad requests need to return a 400; all bad requests do not have the same error response data.

@mclate
Copy link

mclate commented Jun 22, 2015

@webron it's true (to some extend), but as @dannylevenson-ps just mentioned reponse data can vary. And the purpose of api documentation is to show user what errors he can get. So how would one document different error responses (let's assume same data structure for simplification)?

@webron
Copy link
Member

webron commented Jun 22, 2015

Don't see why you need to get angry. We're allowed to disagree.

I'm not saying they have to return the same response data, I'm saying the should have a common response structure.

There's no problem documenting something like:

{
  "code": 0,
  "details": ...
}

Where the details can be a string or an object or whatever you want with the specific information for that error. The code would act as the discriminator between the different error types. That would allow consumers to process such errors more easily, if they know the available codes in general, or even know to expect a basic common structure at all. Does it force a design? Absolutely. Does it cover the requirements? I believe so.

@mclate
Copy link

mclate commented Jun 22, 2015

Okay, this makes sense. However, this doesn't look useful to me when i try to describe response from my route as i still can define only one object for given response code. Assume i do have unified errors where code is a discriminator. The issue i see here is that i can have 10-30 errors for different cases while given route can return only couple of specific ones. And another route will raise different subset of errors. How would one document this?
What i'm struggling to see is list of errors in the request description as we have list of status codes.

Wonder if it would make more sense to have separate section named errors and allow each route to have it's own list of returned errors taken from errors sections (in the same way as you have responses now).

@webron
Copy link
Member

webron commented Jun 22, 2015

You raise a valid point here. It's not that you can't define those groups right now, but you'd require to duplicate your definitions. It would be interesting to try and find a way to control it more easily, but I'd suggest opening a different issue for it, and going into more details, with an elaborate example. That would allow us to dive into the use case and see if/how we can resolve it better in the future.

@gjuchault
Copy link

I'd add that in my API, I have multiple errors that can be thrown.

For example :
404 : document not found
400 : invalid model data (validation)
400 : token invalid (missing "bearer" or base64)
401 : token expired
401 : token invalid (JWT can't parse)
500 : could not write to disk (rethinkdb)
500 : unknown errors

etc.

@tommyjcarpenter
Copy link

I vote for this as well. I opened an SO Post too: http://stackoverflow.com/questions/36576447/swagger-specify-two-responses-with-same-code-based-on-optional-parameter

@Ciantic
Copy link

Ciantic commented May 5, 2016

Highly needed, I can't generate accurate TypeScript definition for error results because of this. TypeScript allowes union types and are perfect to encapsulate differing error cases e.g.

interface Error {
    error: string
}

interface ValidationError extends Error {
    error: "ValidationError"
    data: {
        fields: {[k: string]: string[]}
    }
}

interface GenericError extends Error {
    error: "GenericError"
}

var swaggerResult: ValidationError | GenericError; // Unable to generate this!

Note: above example uses string literal types, e.g. "ValidationError", which are not common in other languages.

Edit Btw, I find this arguing silly. Union types have existed for eons they are really useful. Before this is solved the API's are replaced with GraphQL which does have union types among others.

@rgilling
Copy link

+1 for me - I keep hitting this issue with Swagger. I have a general search service that accepts a parameter "ns". The ns determines which search is executed, and returns a result that may have different fields depending on the search configuration. I definitely don't want a service per search, as the general idea is to give flexibility by having a generic service - and I definitely don't want the reason for needing a service per search to be "the documentation tool doesn't support it" that would be disappointing. Documentation shouldn't get in the way of productivity. At the moment I can't find even a hack to document different successful search responses.

@webron
Copy link
Member

webron commented May 19, 2016

Moving forward, please suggest ways to represent it in the spec.

It looks like the goal is to have dependency between parameter values and the accepted responses. I can think of ways to do that (pending #574), but that would work for simple cases. Not sure how to support more complicated dependencies and keeping some level of elegance to the spec.

@rgilling
Copy link

For my case, I'd be happy without needing to associate the parameter with accepted responses. I can use the description of the responses to indicate to the reader which parameters apply if that's necessary. Simply removing the constraint of having only a single response definition per http response code would be fine for my case.

Bear in mind I'm using Swagger only for documentation, I'm not using any of the code-gen tools. The only things I use are hand crafted JSON, Swagger-UI to display it, and Swagger-tools to validate the JSON.

@webron
Copy link
Member

webron commented May 19, 2016

@rgilling - thanks. I agree that for human-only documentation, the solution is very simple. However, given the growing number of tools out there that rely on compliance with the spec and applying logic based on it, we can't ignore them.

@tommyjcarpenter
Copy link

Yes, I +1d here also for the documentation case. I gave up on Swagger Codegen because I don't understand how it evolves: http://stackoverflow.com/questions/36133273/update-flask-code-with-new-code-generated-by-swagger

@DavidBiesack
Copy link

In Open API 2.0, we use #tag in the paths to allow separate definitions. A hack yes, but it works. See my comment on #146 #146 (comment) . I like it better that query parameters. Swagger UI renders them as different operations (useful) but shows the anchor tags. We have a local patch to hide them, as well as strip them on the actual API call or curl command. Again, this is only a workaround for 2.0; 3.0 addresses this better.

@brandur
Copy link

brandur commented Sep 21, 2016

It looks like this has already been debated to death, but I wanted to weigh in here as well: I'm trying to retroactively build an OpenAPI specification for our API (Stripe), and this issue is the last blocker to getting one that passes validation. I'd reflexively written response schemas using oneOf to point to different responses and only later discovered that although they're valid JSON schema, they're not valid OpenAPI JSON schema.

My use cases are largely around endpoints that I refer to as "polymorphic". They group together a number of different resource types that are related and share a number of fields, but whose concrete implementations are different. An example might be to have an endpoint that lists payment sources, each of which could be a credit card, bank account, or other type.

I'll probably end up implementing one of the workarounds mentioned above (i.e. generating a schema for an artificial resource that has just the fields that are common to all types that could come back from an endpoint), but it would be nice to have a real solution so we could do better validation.

It seems that a number of arguments against allowing this revolve around "correctness" in that endpoint responses should always be totally deterministic. I'd suggest that although this is a somewhat noble goal, the OpenAPI community might be better served with a specification that's flexible enough to represent all types of APIs that might be found in the real world.

@darrelmiller
Copy link
Member

@brandur The current plan is for oneOf to be supported in OpenAPI 3.0 #741

@brandur
Copy link

brandur commented Sep 21, 2016

@brandur The current plan is for oneOf to be supported in OpenAPI 3.0 #741

Excellent news! :) Thanks for the quick response.

@dkuida
Copy link

dkuida commented Oct 2, 2016

👍

@webron
Copy link
Member

webron commented Mar 3, 2017

Going over the tickets now, and it was definitely a nice read. I'm sure I managed to piss off some people with some of my replies, but I'm very happy to have had this discussion and it helped us include a few additional features in 3.0.

Currently, the two relevant features that were added are:

  • support for anyOf and oneOf in the Schema Object.
  • support for multiple media type definitions for responses (see @darrelmiller's comments).

This covers a lot of what was requested here, but not necessarily all. However, I'm going to close the ticket for now and ask people to evaluate the existing options that were introduced to 3.0.0-RC0. If there's a strong need for more, I'd suggest opening a separate ticket for that so we can have a more focused discussion.

@jakesylvestre
Copy link

For anyone looking for this in 2018/after: https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

@ghost
Copy link

ghost commented Nov 15, 2019

It's a matter of finding the balance between requirements, support, elegance and much more. The spec has evolved over the years and obviously supports today much more than it had initially.

My main concern with breaking away from the determinism principle is that it'll lead to changes all across the spec, and may increase its complexity substantially. This is not just an issue for the spec itself but also for the support tools. If we do it, we need to do it carefully and take it all under consideration.

The rules and standards are created to solve practical problem in better way. The need of the hour is to have specs to be flexible so that 1 API spec would be able to satisfy multiple response based on different client.

  • E.g. Profile API has 50 elements.
  • Client 1 will be provided by 5 elements
  • Client 2 will be provided by 10
    -Client 3 may get all 50

Going by current spec client 1 will see all the 50 elements as part of spec which we don't want them to see.

sreeshas pushed a commit to vmware/vmware-openapi-generator that referenced this issue Jan 27, 2020
An API response that returned an 'opaque' had caused some issues as that
is not a response type understood by the Swagger v2 spec. This now
responds with an "object" type in place of "opaque".

It isn't entirely accurate as the opaque type could return any type, but
the Swagger v2+ spec does not allow this. See
[here](OAI/OpenAPI-Specification#270)
for more information on this. The ability to have multiple response
types for a single response code has been added as a feature in
OpenAPI v3+, but this currently uses the Swagger v2 specification.

Signed-off-by: J.R. Garcia <[email protected]>
@haohetao
Copy link

I have same problem.
The @OA annotation is case sensitivity,linux also case sensitivity,but windows case insensitivity,so must ensure capitalized is right.

@douglasg14b
Copy link

I have an endpoint that can return 200 or 206, this is not allowed for the specification...?

@DavidBiesack
Copy link

@douglasg14b yes, you can already define different operation responses, one per response code (or pattern such as '4XX' ):

responses:
    200:
      description: ...
      schema: ...
    206:
      description: ...
      schema: ...

Were you asking something subtly different?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests