Skip to content

Provide better error messages for oneOf schema validation failures #60

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

Closed
tedepstein opened this issue Jan 7, 2016 · 7 comments
Closed

Comments

@tedepstein
Copy link
Collaborator

This Swagger Spec has a "body" parameter that is missing the required "schema" property:


---
swagger: "2.0"
info:
  description: "Tax Blaster"
  version: "1.0.0"
  title: "TaxBlaster"
host: "taxblaster.com"
basePath: "/api"
tags:
- name: "TaxFilingObject"
  description: "An individual Tax Filing record, accessed by its ID"
schemes:
- "http"
parameters:
  idParam:
    name: "id"
    in: "path"
    description: "filingID of the requested TaxFiling"
    required: true
    type: "string"
paths:
  /taxFilings/{id}:
    put:
      tags:
      - "TaxFilingObject"
      description: ""
      operationId: "getTaxFiling"
      consumes:
      - "application/json"
      produces:
      - "application/json"
      parameters:
      - name: "MyPayload"
        in: "body"
        description: "Here's something" 
        required: true
      responses:
        200:
          description: ""
          schema:
            $ref: "#/definitions/TaxFilingObject"
        404:
          description: ""
definitions:
  TaxFilingObject:
    type: "object"
    description: "An individual Tax Filing record, accessed by its ID"
    properties:
      filingID:
        type: "string"
      jurisdiction:
        type: "string"

I would hope for an error message like missing required property: 'schema'. But what it shows is a long, repetitive list of error messages on the parameter object:

Multiple markers at this line
- value body is not allowed, value should be one of "header"
- object has missing required properties "$ref"
- object has properties "description", "in", "name", "required" which are not allowed
- value body is not allowed, value should be one of "query"
- instance failed to match exactly one schema (matched 0 out of 2)
- instance failed to match exactly one schema (matched 0 out of 4)
- object has missing required properties "schema"
- value body is not allowed, value should be one of "path"
- value body is not allowed, value should be one of "formData"
- object has missing required properties "type"

If it's reasonably easy to provide the user with a simple, helpful error message in cases like this, we should. If it's going to require a lot of work, we can defer it.

@tedepstein tedepstein added this to the Release 1.0 milestone Jan 7, 2016
@ghillairet
Copy link
Member

The json-validator returns an extensive report for each validation error, those reports are not easily convertible to marker errors.

In the current implementation, the report is flatten to collect all errors, that's why the list of markers is so long.

I'm not sure how to fix this without to implement a more advanced report processing tool that would identify the error that is relevant to the user's input or intend.

For example the report returned for the error in the above document.

{
  "level": "error",
  "schema": {
    "loadingURI": "http://swagger.io/v2/schema.json#",
    "pointer": "/definitions/parametersList/items"
  },
  "instance": {
    "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
  },
  "domain": "validation",
  "keyword": "oneOf",
  "message": "instance failed to match exactly one schema (matched 0 out of 2)",
  "matched": 0,
  "nrSchemas": 2,
  "reports": {
    "/definitions/parametersList/items/oneOf/0": [
      {
        "level": "error",
        "schema": {
          "loadingURI": "http://swagger.io/v2/schema.json#",
          "pointer": "/definitions/parameter"
        },
        "instance": {
          "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
        },
        "domain": "validation",
        "keyword": "oneOf",
        "message": "instance failed to match exactly one schema (matched 0 out of 2)",
        "matched": 0,
        "nrSchemas": 2,
        "reports": {
          "/definitions/parameter/oneOf/0": [
            {
              "level": "error",
              "schema": {
                "loadingURI": "http://swagger.io/v2/schema.json#",
                "pointer": "/definitions/bodyParameter"
              },
              "instance": {
                "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
              },
              "domain": "validation",
              "keyword": "required",
              "message": "object has missing required properties ([\"schema\"])",
              "required": [
                "in",
                "name",
                "schema"
              ],
              "missing": [
                "schema"
              ]
            }
          ],
          "/definitions/parameter/oneOf/1": [
            {
              "level": "error",
              "schema": {
                "loadingURI": "http://swagger.io/v2/schema.json#",
                "pointer": "/definitions/nonBodyParameter"
              },
              "instance": {
                "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
              },
              "domain": "validation",
              "keyword": "oneOf",
              "message": "instance failed to match exactly one schema (matched 0 out of 4)",
              "matched": 0,
              "nrSchemas": 4,
              "reports": {
                "/definitions/nonBodyParameter/oneOf/0": [
                  {
                    "level": "error",
                    "schema": {
                      "loadingURI": "http://swagger.io/v2/schema.json#",
                      "pointer": "/definitions/headerParameterSubSchema/properties/in"
                    },
                    "instance": {
                      "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0/in"
                    },
                    "domain": "validation",
                    "keyword": "enum",
                    "message": "instance value (\"body\") not found in enum (possible values: [\"header\"])",
                    "value": "body",
                    "enum": [
                      "header"
                    ]
                  }
                ],
                "/definitions/nonBodyParameter/oneOf/1": [
                  {
                    "level": "error",
                    "schema": {
                      "loadingURI": "http://swagger.io/v2/schema.json#",
                      "pointer": "/definitions/formDataParameterSubSchema/properties/in"
                    },
                    "instance": {
                      "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0/in"
                    },
                    "domain": "validation",
                    "keyword": "enum",
                    "message": "instance value (\"body\") not found in enum (possible values: [\"formData\"])",
                    "value": "body",
                    "enum": [
                      "formData"
                    ]
                  }
                ],
                "/definitions/nonBodyParameter/oneOf/2": [
                  {
                    "level": "error",
                    "schema": {
                      "loadingURI": "http://swagger.io/v2/schema.json#",
                      "pointer": "/definitions/queryParameterSubSchema/properties/in"
                    },
                    "instance": {
                      "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0/in"
                    },
                    "domain": "validation",
                    "keyword": "enum",
                    "message": "instance value (\"body\") not found in enum (possible values: [\"query\"])",
                    "value": "body",
                    "enum": [
                      "query"
                    ]
                  }
                ],
                "/definitions/nonBodyParameter/oneOf/3": [
                  {
                    "level": "error",
                    "schema": {
                      "loadingURI": "http://swagger.io/v2/schema.json#",
                      "pointer": "/definitions/pathParameterSubSchema/properties/in"
                    },
                    "instance": {
                      "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0/in"
                    },
                    "domain": "validation",
                    "keyword": "enum",
                    "message": "instance value (\"body\") not found in enum (possible values: [\"path\"])",
                    "value": "body",
                    "enum": [
                      "path"
                    ]
                  }
                ]
              }
            },
            {
              "level": "error",
              "schema": {
                "loadingURI": "http://swagger.io/v2/schema.json#",
                "pointer": "/definitions/nonBodyParameter"
              },
              "instance": {
                "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
              },
              "domain": "validation",
              "keyword": "required",
              "message": "object has missing required properties ([\"type\"])",
              "required": [
                "in",
                "name",
                "type"
              ],
              "missing": [
                "type"
              ]
            }
          ]
        }
      }
    ],
    "/definitions/parametersList/items/oneOf/1": [
      {
        "level": "error",
        "schema": {
          "loadingURI": "http://swagger.io/v2/schema.json#",
          "pointer": "/definitions/jsonReference"
        },
        "instance": {
          "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
        },
        "domain": "validation",
        "keyword": "additionalProperties",
        "message": "object instance has properties which are not allowed by the schema: [\"description\",\"in\",\"name\",\"required\"]",
        "unwanted": [
          "description",
          "in",
          "name",
          "required"
        ]
      },
      {
        "level": "error",
        "schema": {
          "loadingURI": "http://swagger.io/v2/schema.json#",
          "pointer": "/definitions/jsonReference"
        },
        "instance": {
          "pointer": "/paths/~1taxFilings~1{id}/put/parameters/0"
        },
        "domain": "validation",
        "keyword": "required",
        "message": "object has missing required properties ([\"$ref\"])",
        "required": [
          "$ref"
        ],
        "missing": [
          "$ref"
        ]
      }
    ]
  }
}

@tedepstein
Copy link
Collaborator Author

tedepstein commented Jan 11, 2016

@ghillairet , @tfesenko , I had made some comments about this in #9. Essentially, my suggestion is to list the schemas individually, like this:

Failed to match exactly one schema:
  schema 1 :
    - value of type integer is not allowed, value should be of type string
  schema 2:
    - object has properties "description" which are not allowed
    - object has missing required properties "$ref"

There are a couple of challenges here:

First, the individual candidate schemas within a oneOf group may not be named; and it's difficult to explain the problem to the user when it involves an anonymous schema. If a candidate schema is defined in the definitions section, we can use its key. For inline, anonymous schemas we could (a) move them to the definitions section, and assign a name there; or (b) put extended properties into the schema, with names or brief descriptions on each schema, so we can reference it in a more meaningful way. These changes would have to go into our private version of the schema.

Second, the schemas can be recursive. So we can A = oneOf (B, C), and C= oneOf(D, E). To report that, it's logically something like this:

Failed to match A because it's neither B nor C
  Failed to match B because
    missing required properties 'foo' and 'bar'
    contains property 'blip' which is not allowed.
  Failed to match C because it's neither D nor E
    Failed to match D because
      missing required property 'bling'
    Failed to match E because
      missing required properties 'blang'
      contains property 'boing' which is not allowed.

In an earlier comment, I suggested that we should not expand beyond the first level, but now I think this is the only way to provide useful output.

A possible optimization: detect cases where the instance data is clearly "trying" to conform to one candidate schema, and only report errors related to that schema. We could do this by using certain properties as discriminators. For example, in a parameter, we could use the in property as a discriminator, and only show validation errors related to that candidate schema. We would hide error messages related to body parameters.

We might want to encode those rules into the schema itself (e.g. using extended properties in the schema to specify criteria for each candidate schema in a oneOf group). Or it might make more sense to maintain this logic in code, separate from the schema.

There could still be cases where there is no logical discriminator, where the discriminator value(s) are not provided, or not legal. In those cases, the changes above (schema names + recursive error messages) would still be useful.

For the moment, this is the best I can suggest. And because it's likely to be a significant amount of work, I think we should put it on the backlog.

@tedepstein
Copy link
Collaborator Author

@ghillairet , please read my comment above. I'd like to know if you think the combination of named schemas + recursive error messages is feasible. How much time do you think that would take?

To clarify, I am only asking about named schemas + recursive error messages, not including the discriminator optimization. That would be a separate scope.

@tedepstein tedepstein changed the title Body parameter without schema shows a long list of error messages Provide better error messages for oneOf schema validation failures Jan 13, 2016
@ghillairet
Copy link
Member

What is doable now, is to create an error message like this one for validation errors that do match several schema. It stills need some formatting.

screen shot 2016-01-21 at 14 56 26

@tedepstein
Copy link
Collaborator Author

@ghillairet, what you're showing in the screenshot is already a significant improvement. Yes, formatting improvements (normalizing the indent levels, especially) would help. Thanks.

tfesenko added a commit that referenced this issue Jan 21, 2016
[#60] Provide better error messages for `oneOf` schema validation failures
@tfesenko
Copy link
Member

Pull request merged, please address the comments from PR and I will close the issue.
Related issues: #86 and #87.

@tedepstein
Copy link
Collaborator Author

Opened new issue #111 to track the code review follow-ups. Closing this now.

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

No branches or pull requests

3 participants