Giter Club home page Giter Club logo

Comments (16)

aeneasr avatar aeneasr commented on May 15, 2024 1

For anyone looking, I went ahead and implemented those changes in a fork: ory#1

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

It would also be beneficial to return context on the error type - for example missing, type_mismatch, and so on.

from jsonschema.

handrews avatar handrews commented on May 15, 2024

While we leave the exact content up to implementations, the latest draft adds a great deal of recommendations for context. This could easily be applied to earlier drafts.

We may further recommend error content depending on feedback.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

It's great to see recommendations on error responses. Every library solves this differently atm and it's hard to replace one with the other without impacting components downstream.

from jsonschema.

santhosh-tekuri avatar santhosh-tekuri commented on May 15, 2024

At the moment, the context returned by validation errors is very small. For example, a "required" error does not include the pointer to the required field itself, but to it's parent. However, sometimes it's required to know the exact path to present a suitable human readable error.

can you give me an example. When required field is missing, SchemaPtr points to the required field in the schema as json-pointer.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

Yes, so assuming you have a schema along the lines of

{
  "$id": "https://example.com/config.schema.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "required": ["foo"],
  "properties": {
    "foo": {
      "type": "string"
    }
  }
}

and an input of {} I would like to have to show use the error information to show the user, for example, what field exactly is causing this:

foo: This field can not be empty.

or in other languages (e.g. German):

foo: Das Feld darf nicht leer sein.

This is currently not possible, because:

  1. We do not know what type of error occurred (only implicitly by looking at SchemaPtr which would be #/required here or, if nested, something like #/foo/bar/required)
  2. We know what the parent of the property is that has thrown the error, but not the property itself. Here InstancePtr is #, making it impossible to e.g. point the user directly to the field. This is problematic when rendering HTML Forms where we would like to associate the error message with the Form Field itself.
  3. We lack additional context. In cases where, for example, the error is something like expected $1 items but got $2 or expected string got object we would, at the moment, need to parse the error message in order to be able to translate/transform (think JSON, and translation into other languages like German) it, for example.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

As pointed out above, a verbose error would be along the lines of:

// schema
{
  "$id": "https://example.com/polygon",
  "$schema": "https://json-schema.org/draft/2019-09/schema",
  "type": "object",
  "properties": {
    "validProp": true,
  },
  "additionalProperties": false
}

// instance
{
  "validProp": 5,
  "disallowedProp": "value"
}

// result
{
  "valid": false,
  "keywordLocation": "#",
  "instanceLocation": "#",
  "errors": [
    {
      "valid": true,
      "keywordLocation": "#/type",
      "instanceLocation": "#"
    },
    {
      "valid": true,
      "keywordLocation": "#/properties",
      "instanceLocation": "#"
    },
    {
      "valid": false,
      "keywordLocation": "#/additionalProperties",
      "instanceLocation": "#",
      "errors": [
        {
          "valid": false,
          "keywordLocation": "#/additionalProperties",
          "instanceLocation": "#/disallowedProp",
          "error": "Additional property 'disallowedProp' found but was invalid."
        }
      ]
    }
  ]
}

This actually isn't very far away from the current error model. Maybe adding just a few more "ValidationError.Causes" would already help?

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

I was thinking about this a bit more (sorry for the multi-responses). In fact, you're right, most of the context is there. We just have to add one or two more errors to the "causes" list. For example, the required error gives the following result:

I[#] S[#/required] missing properties: "foo"

instead, it would be helpful if it gave this one instead:

I[#] S[#/required] missing properties: "foo"
  I[#/foo] S[#/required] is required

This would already provide sufficient context. On top of that, instead of having InstancePtr and SchemaPtr a simple string, it could be it's own type:

type Pointer struct {
  JSONPointer string // #/path/to/wherever
  Value interface{} // This would be the value of the "resolved" JSONPointer
}

That way, I could, for example, get minItems by doing fmt.Sprintf("%s", ValidationError.SchemaPtr.Value) or the number of items with len(ValidationError.InstancePtr.Value.([]interface{}))

from jsonschema.

santhosh-tekuri avatar santhosh-tekuri commented on May 15, 2024
  1. We do not know what type of error occurred (only implicitly by looking at SchemaPtr which would be #/required here or, if nested, something like #/foo/bar/required)

based on last path component in SchemaPtr you can know type of error. if SchemaPtr is #/required you know that required constraint validation failed.

  1. We know what the parent of the property is that has thrown the error, but not the property itself. Here InstancePtr is #, making it impossible to e.g. point the user directly to the field. This is problematic when rendering HTML Forms where we would like to associate the error message with the Form Field itself.

in case of required the InstancePtr cannot point to field foo as in #/foo because foo property does not exist. if you want actually property name that is missing we can set SchemaPtrto#/required/0' then from schema you can get the property. currently property name is embedded in the error message.

  1. We lack additional context. In cases where, for example, the error is something like expected $1 items but got $2 or expected string got object we would, at the moment, need to parse the error message in order to be able to translate/transform (think JSON, and translation into other languages like German) it, for example.

18n was never on the list. the error messages are for humans. the errors allow only programmatic access to schema and instance location where error occurred. think of golang compilation errors.
they give package/file/line and error message. you don't expect all information why it failed is not programatically accessible.

mainly error message are for human to understand why it failed. the schemaPtr and instancePtr can help editors like sublime to locate error in editor and show error mark.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

based on last path component in SchemaPtr you can know type of error. if SchemaPtr is #/required you know that required constraint validation failed.

Point being that I don't have to retrieve the pointer manually. This is more about developer experience.

in case of required the InstancePtr cannot point to field foo as in #/foo because foo property does not exist. if you want actually property name that is missing we can set SchemaPtrto#/required/0' then from schema you can get the property. currently property name is embedded in the error message.

That makes sense. See my comment after the next quote.

18n was never on the list. the error messages are for humans. the errors allow only programmatic access to schema and instance location where error occurred. think of golang compilation errors.
they give package/file/line and error message. you don't expect all information why it failed is not programatically accessible.

I understand that. My point is that while this is out of this libraries domain, it should still be possible for consumer. Right now, it's not. To, for example, implement form validation for required fields I would have to:

  • split SchemaPtr and use the last element of the path to understand if it's caused by required
  • write a regex to parse the error message to match "missing properties: %s" (e.g. missing properties: (.+)), split the result with , to get the field that's missing.
  • trust that this library doesn't modify the error message in any way (typically merged as patches because while it is a breaking API change - since I have no other way of knowing error details - it probably won't be viewed as such as it's an implicit contract).

I hope you see my point. This would - by the way - be fairly easy to solve by adding additional types such as ValidationErrorRequired that carry additional context such as:

type ValidationErrorRequired struct {
  MissingPtr string
  *ValidationError
}

I hope you understand my point.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

Just checking in if you've thought about this. If you don't see this as an issue or think it's out of the scope of this library that's fine as we'll just work with a fork then.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

I've started working on our fork, you can check it out to see what we're planning to address these issues: https://github.com/ory/jsonschema/pull/1/files

from jsonschema.

santhosh-tekuri avatar santhosh-tekuri commented on May 15, 2024

this will be incompatible change.

the latest draft has specification for output format. also need to thought about implementing this. so I kept it aside.

I checked your code. the code gets bloated for each type of error. I prefer to use simple map[string]interface{} as Context in Validation in your pull commit.

will have a look at this, in latest draft implementation to avoid breaking changes multiple times.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

the latest draft has specification for output format. also need to thought about implementing this. so I kept it aside.

Fair point! We'll go with our fork in the meanwhile and I'm happy to help once you decide to start work on draft8!

I checked your code. the code gets bloated for each type of error. I prefer to use simple map[string]interface{} as Context in Validation in your pull commit.

I think that's a fair point, the design style is up to you! Personally, I prefer static types as a developer consuming Go libraries. While map[string]interface keeps your codebase clean, it pollutes my codebase, because I have to assert types and handle type checks.

When designing my Go code (hydra, fosite, ladon, ...) I always try to design things in such a way that consuming them is less work - even if writing them is a bit more. But hey - please take this just as input - I'm not here to tell you how to design your code, just share my experiences with maintaining tons of Go code and listening to people complain about it on GitHub ;)

from jsonschema.

santhosh-tekuri avatar santhosh-tekuri commented on May 15, 2024

now the library implements the output format as specified in draft.

from jsonschema.

aeneasr avatar aeneasr commented on May 15, 2024

Awesome!!

from jsonschema.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.