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

use of __example observations #722

Closed
equalspeterd opened this issue Oct 21, 2019 · 7 comments
Closed

use of __example observations #722

equalspeterd opened this issue Oct 21, 2019 · 7 comments
Labels

Comments

@equalspeterd
Copy link

Not sure if this is a bug or a feature request, so I will start here with a question/observation, and convert as appropriate.

selecting response examples
when using ?__example=foo request parameters to select response examples, prism assumes that only a response object body is requested (as evidenced by the response error), and not examples defined elsewhere in the specification (e.g. headers):

POST /bar?__example=myHeaderExample

{
    "type": "https://stoplight.io/prism/errors#NOT_FOUND",
    "title": "The server cannot find the requested content",
    "status": 404,
    "detail": "Response for contentType: application/json and exampleKey: myHeaderExample does not exist."
}

despite having defined:

Location:
      description: the URI of a newly created resource
      required: true
      schema:
        type: string
        format: uri
        pattern: https://example.com/.*
      examples:
        myHeaderExample:
          value: 'https://example.com/foo'

(no application/json example named myHeaderExample was defined in the response content).

I totally understand the challenge here, as openAPI does no properly define a response object with an examples property. Rather, it relies on explicit parameter examples and media-type examples (and I don't see that changing anytime soon). However, a documented profile in prism could resolve this deficiency, when both a parameter named example and a media-type named example shared the same name, each could be used in constructing a response. Food for thought.

The observation I had was why you choose to respond with a 404 if a named example is not found. That seems odd (to me at least). The resource was found, just not a suitable representation (the sought-after example). My €0.02.

@XVincentX
Copy link
Contributor

Hey @equalspeterd

Sorry for the delay in the response!

when using ?__example=foo request parameters to select response examples, prism assumes that only a response object body is requested (as evidenced by the response error), and not examples defined elsewhere in the specification (e.g. headers):

This is correct, currently we only support body examples. Headers example is something we might want to support as well.

However, a documented profile in prism could resolve this deficiency

Can you clarify what's a documented profile? 🤔

The observation I had was why you choose to respond with a 404 if a named example is not found.

This is a good point. Theoretically Prism has:

  • Found the resource (path matched)
  • Found the responses definition
  • NOT found the named example for that content Type

And the spec says that a 404 is when The server has not found anything matching the Request-URI — which is not the case here.

I guess we could 415 (Unsupported Media Type) — is that what you were thinking about?

@equalspeterd
Copy link
Author

However, a documented profile in prism could resolve this deficiency

Can you clarify what's a documented profile? 🤔

Just a carefully crafted description in the prism README or such. Nothing special. If written in a manner that could be directly fork-lifted into the openAPI spec, that would go a long way towards moving the ball to fix the root cause (header & body named examples).

I guess we could 415 (Unsupported Media Type) — is that what you were thinking about?

415 or 422 (which you use today for other unserializable representation errors) I think would work.

I appreciate you guys working on this. HT 🎩 to the team.

@XVincentX
Copy link
Contributor

@philsturgeon Thoughts?

@philsturgeon
Copy link
Contributor

Yep Picking the example is for the Media Type examples, which do not have headers inside them. Headers are defined on the response object.

image

Then inside there is content, which is the Media Type Object:

image

This has an examples property which is capable of having multiple examples. This is also the only place in OpenAPI where an examples keyword exists in the specification. It is not listed as one of their differences from JSON Schema in the Schema Object, so I think the "In a parameter" example for examples is incorrect:

image

I just asked on the OpenAPI slack whilst writing this and yep @webron says it's a mistake!

That looks like a mistake. Only example is supported inside schema.

So! I think your description document here is invalid, and this is not a mistake in Prism. I'll send a PR to the OpenAPI spec to fix up that invalid example now. Thanks for noticing this!

@XVincentX
Copy link
Contributor

@philsturgeon What about the 404 status code?

@equalspeterd
Copy link
Author

@philsturgeon thanks for circling back with openAPI. I will take my proposal for supporting header/body paired response examples directly to them. I do think the 404 error prism emit's for missing examples somewhat odd. I would think the fall-back would be to assemble a generic response as if no examples were defined at all.

@philsturgeon
Copy link
Contributor

@equalspeterd no problem! I get that 404 is not the most obvious thing, we agonised for ages over what to do. There are two schools of thought:

  1. Always try to give the user something useful
  2. Be very clear about when the user is requesting something that doesnt exist

We picked 2 because if you are making bad requests we would like you to know about that, so you can fix your request.

As for specifically 404, it could have been absolutely anything. It could have been a 500 because Prism as a server is unsure what to do, it could have been a 409 because the data source is unable to take this request in its current state, we could probably make a case for anything, but in the end we decided to push on and 404 is fine.

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

No branches or pull requests

3 participants