interagent / committee

A collection of Rack middleware to support JSON Schema.
MIT License
882 stars 135 forks source link

Error message of Committee::ValidationError looks odd #293

Closed nicolasiensen closed 3 years ago

nicolasiensen commented 3 years ago

I'm using Committee::Middleware::RequestValidation to validate incoming requests for a Sinatra app.

When the body of the request doesn't match the schema defined in the OpenAPI documentation, Committee::ValidationError generates a message that looks odd and hard to read:

"#/paths/~1foo~1{foo_id}~1bar/post/requestBody/content/application~1json/schema missing required parameters: baz"

The OpenAPI documentation looks like this:

---
openapi: 3.0.3
paths:
  "/foo/{foo_id}/bar":
    post:
      parameters:
      - name: foo_id
        in: path
        required: true
        schema:
          type: string
      requestBody:
        content:
          application/json:
            schema:
              type: object
              required:
              - baz
              properties:
                baz:
                  type: string
                  format: date-time

Any idea why the message looks like that? I was expecting a simpler message like the one documented in the project's readme ("Require params: recipient.").

I'm using committee (4.2.1).

I really appreciate any help you can provide.

ota42y commented 3 years ago

For example, if parameters with the same name are in different hierarchies, we should display exactly which parameter caused the error.

{
  "ship": {
    "name": "xxx",
    "speed: 1.4,
    "captain": {
      "name": "Hook"
    }
  } 
}

(i.e. When the user post this json without 'name', we should show which 'name' is missing. )

When you use 'committee' and OpenAPI 3, we show the exact path in error message.

If you want to show other message, please use 'raise' option. When 'raise=true', committee raise error so you can catch it and change other message. (It is better to be able to replace the error message if necessary 🤔 )

nicolasiensen commented 3 years ago

Looking from that perspective, it makes sense to include the JSON path in the error message. Thanks for the response, @ota42y.

As a side note, perhaps we should change the README to document the error responses as they actually look like?

ota42y commented 3 years ago

Yes, I think we should change README.

nicolasiensen commented 3 years ago

299