voxpupuli / json-schema

Ruby JSON Schema Validator
MIT License
1.52k stars 242 forks source link

More useful error message than "TypeError: no implicit conversion of String into Integer" in case of invalid schema #380

Open bobbybobby opened 7 years ago

bobbybobby commented 7 years ago

I ran into a problem when validating against a schema that was valid JSON but wrong semantically. It took me a while to find where the mistake was, because the validator doesn't troubleshot for semantical errors in the schema. I know it's easier say than done, but I post at least in the hope that it will help if someone run into the same issue. By the way, if anyone knows an external tool that allow to proofread JSON schemas, let me know.

Reproduction of the issue :

schema = { 
  "type" => "object", 
  "properties" => { 
    "a" => [nil, "string"] 
  } 
}

JSON::Validator.validate(schema, { "a" => "foo"})

Result :

TypeError: no implicit conversion of String into Integer
    from /tmp/json-schema/lib/json-schema/schema.rb:13:in `[]'
    from /tmp/json-schema/lib/json-schema/schema.rb:13:in `initialize'
    from /tmp/json-schema/lib/json-schema/attributes/properties.rb:31:in `new'
    from /tmp/json-schema/lib/json-schema/attributes/properties.rb:31:in `block in validate'
    from /tmp/json-schema/lib/json-schema/attributes/properties.rb:14:in `each'
    from /tmp/json-schema/lib/json-schema/attributes/properties.rb:14:in `validate'
    from /tmp/json-schema/lib/json-schema/schema/validator.rb:25:in `block in validate'
    from /tmp/json-schema/lib/json-schema/schema/validator.rb:23:in `each'
    from /tmp/json-schema/lib/json-schema/schema/validator.rb:23:in `validate'
    from /tmp/json-schema/lib/json-schema/schema.rb:33:in `validate'
    from /tmp/json-schema/lib/json-schema/validator.rb:113:in `validate'
    from /tmp/json-schema/lib/json-schema/validator.rb:254:in `validate!'
    from /tmp/json-schema/lib/json-schema/validator.rb:238:in `validate'
    from (irb):2
    from /home/quentin/.rbenv/versions/2.3.0/bin/irb:11:in `<main>'

The mistake (obvious in this example, but in a real life big schema with no hints, not so much), is that schema should have been :

schema = { 
  "type" => "object", 
  "properties" => { 
    "a" => {"type" => [nil, "string"] }
  } 
}
iainbeeston commented 7 years ago

Hey, thanks for reporting the issue. We do in fact have an option to validate that the schema is valid, before using it to validate the data. If you run

JSON::Validator.validate(schema, { "a" => "foo"}, validate_schema: true)

You should get a more helpful error that in fact your schema is invalid.

However, there is a good point here. While the gem explicitly raises validation errors using the JSON::Schema::ValidationError class, sometimes other, unexpected errors occur during validation. We could catch these errors, and (on the assumption that these are probably errors in the schema) re-raising them as a JSON::Schema::SchemaError. This might be more helpful for users of the gem. I'll have to think about the potential side-effects of that

RST-J commented 7 years ago

As a start, I think introducing a covariant exception should not lead to problems with existing handlers. Whatever error class they are catching, they will also catch our new subtype of it. But there is a potential semantic change: If we introduce our new schema error as a subtype of an assumed type typehierarchy A < B and we also assume a block of code with two exception handlers for A and B. A realistic scenario would be if we assign StandardError to A and JSON::Schema::SchemaError as B. Then legacy code would handle some unexpected error with A for now. But after we wrap these and raise them as JSON::Schema::SchemaError it would then handle them with B. I don't expect that to be a big issue, but that would be a side effect.