joolfe / postman-to-openapi

🛸 Convert postman collection to OpenAPI
MIT License
577 stars 99 forks source link

fix: missing `code` would result in an `undefined` response status #255

Closed erunion closed 1 year ago

erunion commented 1 year ago

If code is missing in a Postman-documented response example then an OpenAPI definition response entry will be generated with an invalid HTTP Status code of undefined. As code isn't required by the Postman spec[^1] I'm opting to default to an OAS-compliant status code of default rather than 200 (or something else).

[^1]: There's no required declaration for code anywhere in the v2.1 or v2.0 schemas.

joolfe commented 1 year ago

Hi @erunion,

First of all thanks so much for the PR, but your code is generating a not valid OPEN API result, you can test on https://editor.swagger.io/

Screenshot 2023-04-16 at 12 48 54

I know that Postman didn´t require the code but before thinking in a solution let's review the problem:

Does makes sense to have response examples without a response status code?

Even if it is possible (postman schema didnñt say that code is required), In my opinion you donñt want to have responses without status code in your collection, and even if you have it in your postman collection i will say that you donñt want to document this in your Open API. The missing code for me seems more like an error in somewhere (the place where you are generating the postman collection) than a problem in the translation between Postman and Open API.

Saying that if you read the HTTP specs Status Codes](https://www.rfc-editor.org/rfc/rfc9110.html#name-status-codes) clearly say "The status code of a response is a three-digit integer code that describes the result of the request" so the default values seem to not be a valid value. and also add "A client that receives a response with an invalid status code SHOULD process the response as if it had a 5xx (Server Error) status code." so for me the default status code would be a 500 as the missing of a status code should be process as this following HTTP spec, but again, i will never do this in the library as seems clearly an error in the source that is the postman collection.

Best Regards.

Dashron commented 1 year ago

@erunion isn't using default as an http status code, it's a part of the oas spec that acts as a wildcard for all status codes.

I was able to generate examples with no status codes and save then export them successfully from the postman mac app so I don't think we should be placing blame with the postman collection generation. Users can do it, the spec allows it, so it would be nice to have some level of support (use default, drop the example, throw an error, w/e) for what appears to be intended behavior.

kanadgupta commented 1 year ago

your code is generating a not valid OPEN API result

That's actually a bug with this library, not @erunion's pull request. This can be easily fixed with a couple additional lines, something like this:

if (description === undefined) {
  description = 'default'
}

If you run postman-to-openapi@3.0.1 against this Postman collection example, it produces a result with even more errors:

CleanShot 2023-04-17 at 12 18 06@2x

Note how default is a valid value according to the above error message. Here's what the OpenAPI Specification says about the default value in the responses object:

The documentation of responses other than the ones declared for specific HTTP response codes. Use this field to cover undeclared responses.

@joolfe as you can see, default is the most appropriate value here. This library is currently producing invalid OpenAPI output without this change for a very common use case that we see in Postman collections. We've already made this fix in our upstream fork but I encourage you to consider incorporating this change. Thanks!