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

Invalid schema causes "should have required property '$ref'" error #403

Closed
AnthonyPorthouse opened this issue Jul 23, 2019 · 24 comments · Fixed by #1071
Closed

Invalid schema causes "should have required property '$ref'" error #403

AnthonyPorthouse opened this issue Jul 23, 2019 · 24 comments · Fixed by #1071
Assignees
Labels
cs/reported p/urgent t/bug Something isn't working

Comments

@AnthonyPorthouse
Copy link
Contributor

AnthonyPorthouse commented Jul 23, 2019

Describe the bug
When an OpenAPI v3 Response Object contains a schema which has invalid schemas, an error is thrown saying: should have a required $ref property.

This is probably coming from better-ajv-errors, which is deduping errors which are saying "should be a valid sub schema, or should be a $ref" and its just showing the $ref bit.

To Reproduce

  1. Given this OpenAPI document
openapi: 3.0.1
info:
  title: Example $ref error
  version: 1.0.0
paths:
  /user:
    get:
      operationId: getUser
      responses:
        "200":
          description: An Example
          content:
            application/json:
              schema:
                type: object
                properties:
                  user_id: 12345
  1. Run this CLI command spectral lint openapi.yml

  2. See error

Expected behavior

Show a more appropriate validation error. In this instance, other validators return this error:

Structural error at paths./user.get.responses.200.content.application/json.schema.properties.user_id should be object

Environment:

  • Library version: 4.0.1
  • OS: Mac OS 10.14.5
@AnthonyPorthouse AnthonyPorthouse added the t/bug Something isn't working label Jul 23, 2019
@mburtless
Copy link

Adding exact error code, so this can be found by other folks that run into this issue. For the schema above, error code would likely be something like:

error  oas3-schema                     /paths//user/get/responses/200 should have required property '$ref'

I see this error even if I have ref inside the schema object.

@tbarn
Copy link
Contributor

tbarn commented Jul 29, 2019

@AnthonyPorthouse I need to double check if that's a valid OAS document. I changed the last part to this:

openapi: 3.0.1
info:
  title: Example $ref error
  version: 1.0.0
paths:
  /user:
    get:
      operationId: getUser
      responses:
        "200":
          description: An Example
          content:
            application/json:
              schema:
                type: object
                properties:
                  user_id:
                    type: integer

And it passed. If you are still wanting the example there, I would normally do:

openapi: 3.0.1
info:
  title: Example $ref error
  version: 1.0.0
paths:
  /user:
    get:
      operationId: getUser
      responses:
        "200":
          description: An Example
          content:
            application/json:
              schema:
                type: object
                properties:
                  user_id:
                    type: integer
                    example: 12345

@tbarn
Copy link
Contributor

tbarn commented Jul 29, 2019

The error message is definitely misleading, but I think the initial document is not valid either way.

@AnthonyPorthouse
Copy link
Contributor Author

@tbarn perhaps the issue then is that it is eating errors saying it should be a $ref instead of why it's invalid?

Agreed my initial PoC spec was rushed as it was towards the end of my work day and as you've pointed out is in fact invalid.

Would you prefer I open a new issue describing these issues more correctly, or should we simply go with this issue?

@tbarn
Copy link
Contributor

tbarn commented Aug 2, 2019

@AnthonyPorthouse We can totally stick to this issue, but could you edit it so it accurate?

@philsturgeon philsturgeon changed the title OpenAPI 3 Response Object validation fails with none required properties Invalid schema causes "should have required property '$ref'" error Aug 14, 2019
@philsturgeon
Copy link
Contributor

Got an easily repeatable one:

image

openapi: '3.0.0'
info:
  version: 1.0.0
  title: Foo
  description: Why you talking bout ref
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    get:
      summary: List all pets
      operationId: listPets
      parameters:
        - name: limit
          in: query
          description: 
          required: false
          schema:
            type: integer
            format: int32
      responses:
        '200':
          description: A paged array of pets

Why is it talking about line 7?

@P0lip do you have any ideas on this one?

@P0lip
Copy link
Contributor

P0lip commented Aug 27, 2019

@philsturgeon I'm quite sure, it talks about line 8, but the lines displayed in the sandbox are not correct. They should be shifted by 1, i.e. info should be reported for line 2 and same for operation issues.

That said, I still don't really know why it complains about $ref 🤔
I'll try to investigate it this week.

@P0lip P0lip self-assigned this Aug 28, 2019
@P0lip
Copy link
Contributor

P0lip commented Aug 28, 2019

Okay, I nailed it down.
In the example Phil submitted description is empty. It's not an empty string, though, and therefore AJV cannot match it, since description property is required. The fix would be to add an empty string as follows

- name: limit
  in: query
  description: ""
  required: false
  schema:
    type: integer
    format: int32

There is really nothing we can do about it :/
oas2 and oas3 schemas are very complex beasts, so the errors reported by AJV tend to be vague.
Hopefully, we introduce a more specified oas2/oas3 schema validation that's not based only on AJV.

@philsturgeon @AnthonyPorthouse @marbemac

@AnthonyPorthouse
Copy link
Contributor Author

AnthonyPorthouse commented Aug 28, 2019 via email

@mburtless
Copy link

+1 I've run into the same issue when using externally referenced JSON schema (same use case for it on our end).

@philsturgeon
Copy link
Contributor

@AnthonyPorthouse are you mixing OpenAPI and proper-JSON Schema without transpiling/downgrading/etc your external schemas from JSON Schema to OpenAPI Schema Objects? They are not as interchangeable as you might think.

https://blog.apisyouwonthate.com/openapi-and-json-schema-divergence-part-1-1daf6678d86e

@jrieko
Copy link

jrieko commented Dec 11, 2019

I'm likewise encountering this. As @philsturgeon says, the cause is something non-oas3-compliant somewhere in the external schema referenced by the response object, but the issue with Spectral is that cause and location isn't communicated by error oas3-schema /paths//users/get/responses/200 should have required property '$ref'.

I can work around this by

  1. finding this issue to translate the Spectral error message to 'one or more of your schema is invalid oas3 schema'
  2. manually review each schema and compare with the oas3 spec until I find the cause
  3. spectral lint ..., repeat

Spectral is still providing value by telling me there's an error early on, but this work-around takes a lot of time.

On the other hand, an easier work-around is purportedly around the corner-ish (emphasis on the -ish), pending OAS-JSON schema convergence as Phil reported. Spectral does individually validate the referenced external schema if included in the input file-path, just as JSON Schema. So theoretically, after that OAS revision drops, when Spectral lints external schema files marked as JSON Schema Draft 2019/09, they're implicitly valid OAS schema.

@moltar
Copy link

moltar commented Dec 20, 2019

This also happens for path params:

  '/modules/{module_id}/sections':
    parameters:
      - type: string
        name: module_id
        in: path
        required: true
        schema:
          type: string
        description: Module ID
    get:
/paths//modules/{module_id}/sections/parameters/0 should have required property '$ref'

@jrieko
Copy link

jrieko commented Dec 23, 2019

Yep and, in this case, the actual cause of the error is this is invalid oas, specifically parameter objects have no field type, but Spectral isn't communicating that at all.

@huksley
Copy link

huksley commented Jan 14, 2020

When using external schemas I get oas3-schema error, however, there is nothing wrong with specification.

> [email protected] lint-spec /Users/user/src/maas-tsp-api
> node utils/adopt-schemas.js && cross-env NODE_OPTIONS=--max_old_space_size=4096 spectral lint specs/booking.yml

OpenAPI 3.x detected

/Users/user/src/maas-tsp-api/specs/booking.yml
 319:11  error  oas3-schema  /paths//bookings/options/get/parameters/4 should have required property '$ref'

✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints)

I took a look at parameters, they are all have defined type via schema/$ref

If I remove the mode parameter, which declared like this (and references this):

        - name: mode
          description: 'Transfer mode'
          in: query
          required: false
          schema:
            $ref: ../schemas/core/components/travel-mode.json

it starts to complain with this:

/Users/user/src/maas-tsp-api/specs/booking.yml
 337:15  error  oas3-schema  /paths//bookings/options/get/responses/200 should have required property '$ref'

which is again looks fine to me 🤔

      responses:
        '200':
          x-summary: Array of options
          description: |
            Available transport options matching the given query parameters. If no transport options are available; an empty array is returned.
          content:
            application/json:
              schema:
                $ref: ../schemas/tsp/booking-options-list/response.json
              examples:
                Taxi:
                  summary: Taxi
                  externalValue: '../examples/taxi/booking-options-response.json'

Reproduce

Before using spec, it is bundled into the JSON single file from YAML spec and schemas/ folder.

git clone https://github.com/maasglobal/maas-tsp-api/
cd maas-tsp-ap
npm install
npm run lint-spec

@m-denton
Copy link

So what is the final take on this one? Is Stoplight working on a solution or should we refer to the link Phil posted above?

@philsturgeon
Copy link
Contributor

@m-denton this is being worked on, it has a very high priority, but so do a few other things. We'll get to it. In the meantime just keep in mind that it means "something nearby is invalid" and do you best to fix it by eye. Then soon, after an update, it'll just start being awesome again and you won't have to eyeball it anymore. :D

@m-denton
Copy link

Sounds great, thank you @philsturgeon. I believe we found our issue and that it isn't as we originally thought. Seems to be a transient error, somehow related to circular references. An example can be seen here: #870 (comment)

@canberkol
Copy link

canberkol commented Jan 27, 2020

For me it was a UI (studio) based error. I was using UI to generate "model" specifications but Studio was creating false yaml.

For example for a "model" with a "nullable" property the UI outputs something like this:

properties:
  id:
    type:
     - string
     - null

However OAS3 specification tells that it should be like below:

properties:
  id:
    type: string
    nullable: true

After I fixed this on the "code" view. The errors disappeared. So my assumption is that if you trust the output of Studio for OAS3 you may fall into such issues and the linter can get confused. It is not a big issue but the message is misleading for sure.

I hope this info will help you to fix your issues.

Cheers!

@philsturgeon
Copy link
Contributor

Thank you for sharing the example. You are right that this will cause the validator library we use to vomit up the $ref error, and we are working on switching out to a more useful validator. AJV is not the best for error messages sadly. You either get duplicates, or you get this misleading one.

All that said, the conclusion that the GUI cannot be trusted is not entirely accurate. Allowing multiple types in OpenAPI is a known issue, but you would not believe how many people do this intentionally and expect it to work. Most people are not aware of the JSON Schema !== OpenAPI Divergence, see JSON Schema allowing multiple types, then because people do it so much various tools have decided to add support for it... 🤕

We're working on getting "multiple flavour" support into Stoplight to support multiple drafts of JSON Schema and OpenAPI v2/v3/v3.1 more accurately, but this is the biggest issue you're likely to notice using the GUI. https://github.com/stoplightio/studio/issues/190 This one will be fixed, but just be aware of it and keep on using the GUI to your hearts content.

@philsturgeon
Copy link
Contributor

We've not forgotten about this issue. We are solving problems a few levels down, with supporting multiple JSON Schema and OpenAPI dialects and vocabularies, which will get us to reevaluate some of the dependencies we've built features like schema-based validation on top of. If we're continuing to use schema-based validation it'll likely be switching to DJV, and we need to work on some functionality there to make it support better errors. Otherwise maybe Sway, but we'll have to see what happens as this work progresses.

For now invalid schema messages are a bit funky and I'm sorry about that. We'll get there.

@P0lip
Copy link
Contributor

P0lip commented Apr 7, 2020

#1071 should address most of the issues that have been reported here.

@philsturgeon
Copy link
Contributor

@AnthonyPorthouse hey, any chance you could pull this down and give it a go?

@AnthonyPorthouse
Copy link
Contributor Author

@philsturgeon I can confirm this definitely works with the example I initially posted. Output is as follows:

OpenAPI 3.x detected

/Users/anthony/spectral-test/openapi.yaml
  1:1   warning  oas3-api-servers       OpenAPI `servers` must be present and non-empty array.
  1:1   warning  openapi-tags           OpenAPI object should have non-empty `tags` array.
  2:6   warning  info-contact           Info object should contain `contact` object.
  2:6   warning  info-description       OpenAPI object info `description` must be present and non-empty string.
  7:9   warning  operation-description  Operation `description` must be present and non-empty string.
  7:9   warning  operation-tags         Operation should have non-empty `tags` array.
 17:28    error  oas3-schema            `user_id` property type should be object.

✖ 7 problems (1 error, 6 warnings, 0 infos, 0 hints)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cs/reported p/urgent t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.