-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Need a way to mark response as error #236
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
Comments
There's no real need to mark responses as error. By standard, 2XX responses are 'ok', 4XX responses are errors due to the client request and 5XX responses are errors due to server issues. |
I'll mark is as a proposal for the next version and we'll see how people respond to the suggestion. |
I could see the following scenario: As much as I hate to encourage the use of 200-only APIs, here's a potential solution (and one that perhaps makes 200:
description: Expected response to a valid request
schema:
$ref: '#/definitions/Pets'
default:
error: true # Note addition of `error`
description: unexpected error
schema:
$ref: '#/definitions/Error' |
As an API author/consumer, I am not sure that I need extra information to indicate if a response is an error or not. The I'm not saying we shouldn't do it but adding an extra |
Not sure this is something we should support because "these things exist". Haven't seen many people ask for it, and it feels like that's one of the few things about REST APIs that people commonly agree on. Even if it's easy to support, don't know if we should. However, if that |
Agreed, if it had to be only for the |
I'd agree @webron that it's probably an issue that's trending down in public APIs. However, I have concern that as internal APIs are being built more and more (often with much lower standards/education), this could be a bigger issue. |
Just thought I'd chime in here. Since this is a in vNext (read breaking changes allowed) I think this is a great opportunity to describe these things as we'd like the spec to handle them ideally and not try to jam these things in existing constructs. IMHO, as a spec we'd be better served guiding the spec in a direction such that it drives the consumers, implementors and tool vendors into the "pit of success". I'd rather make it hard to do things sub-optimally. Littering the spec with these exception cases, just to be everything for everybody will make the job for spec implementers that much harder. Also will make evolution of the spec harder. Having said that, It seems like this could benefit by using the oneOf construct. 200:
description: Expected response to a valid request
schema:
oneOf: [
{ $ref: '#/definitions/Pets' },
{ $ref: '#/definitions/Error' }
], |
@dilipkrish That's just something I don't get. Your example shows a 200 response sending either Pets or Error - but how does that help the consumer? How would they know which is the successful response case and which is the error (ignoring the fact that we both have some level of knowledge in English of course). That definitely doesn't help automatic processing, whether it's for validation or code generation. I'd rather not support the construct at all then support it like that. As a side note, the |
May be I wasnt being clear. I'm NOT for doing this the suggested way. My example was in response to this use case 👇🏻, and wasn't particularly fond of the "default" approach 😋
My example is no different than the one with the "default" from the clients perspective; but it introduces a new construct that implementers need to care about just for the response object alone. IMO as a spec there needs to be standard patterns when we're introducing new constructs. It doesn't have to use oneOf, but it needs to be a consistent construct. My comment was to reiterate that when we're making decisions about adding new constructs to parts of the spec let's be
Ironic that I suggested the support for this one-off use case with oneOf 😜 |
Heh, fair enough, it was my misunderstanding. Generally, I believe we have a similar view re your last comment. |
Link to meta #566 |
I'd argue that the right way to handle this is really to use a media type for error responses - |
My suggested solution from 2020 got some upvotes, and was the first comment since 2016. It does not require a spec change. We're working on a new version over in the OAI/sig-moonwalk repository, so larger philosophical concerns around error modeling should join the discussion there. Also, if there is truly interest in an error flag field of some sort, I'd suggest trying an extension which could go in the registry. If it gets enough use it might get adopted. Given that no one else has commented since 2016 and there are several paths forward that do not involve an immediate spec change, I'm going to close this. |
This may be similar to #166. Currently there is no way to indicate whether a ResponseObject is an error or not. In the pet store example a "default" response is an error, however there is no way for the tooling to understand whether the model being returned is an error or not unless the assumption is made that all "default" responses are errors (which is not always correct). Please consider adding something like "isError" property to a ResponseObject.
The text was updated successfully, but these errors were encountered: