mercurius-js / validation

Adds configurable validation support to Mercurius.
MIT License
29 stars 6 forks source link

Validation failures from constraints result in a 500 status code rather than a 400 status code #51

Closed anthonyroach closed 2 years ago

anthonyroach commented 2 years ago

Validation errors mean the client sent something invalid, so they should result in a 400 status code but instead they result in a 500 status code.

Library versions:

Example:

HTTP/1.1 500 Internal Server Error
cache-control: no-cache
content-type: application/json; charset=utf-8
content-length: 605
Date: Wed, 29 Jun 2022 19:36:33 GMT
Connection: close

{
  "errors": [
    {
      "message": "Failed Validation on arguments for field 'Query.uploadFile'",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "uploadFile"
      ],
      "extensions": {
        "name": "ValidationError",
        "code": "MER_VALIDATION_ERR_FAILED_VALIDATION",
        "details": [
          {
            "instancePath": "/key/name",
            "schemaPath": "https://mercurius.dev/validation/FileKey/properties/name/pattern",
            "keyword": "pattern",
            "params": {
              "pattern": "^[^/]+$"
            },
            "message": "must match pattern \"^[^/]+$\"",
            "schema": "^[^/]+$",
            "parentSchema": {
              "pattern": "^[^/]+$",
              "type": "string",
              "$id": "https://mercurius.dev/validation/FileKey/name"
            },
            "data": "foo/icon.png"
          }
        ]
      }
    }
  ],
  "data": null
}
mcollina commented 2 years ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

anthonyroach commented 2 years ago

Certainly!

anthonyroach commented 2 years ago

@jonnydgreen thanks for the quick turn around on the PR merge and 3.0.0 release. I verified the fix for this issue in my project and it works as expected:

HTTP/1.1 400 Bad Request
cache-control: no-cache
content-type: application/json; charset=utf-8
content-length: 562
Date: Tue, 05 Jul 2022 15:02:52 GMT
Connection: close
jonnydgreen commented 2 years ago

fantastic, thanks for verifying and no problem at all :)