fluree / db

Fluree database library
https://fluree.github.io/db/
Other
337 stars 21 forks source link

SHACL validation error messages #565

Closed dpetran closed 5 months ago

dpetran commented 1 year ago

Our SHACL error messages have several deficiencies:

dpetran commented 1 year ago

We may want to consider returning validation reports, which do contain a lot of useful information necessary to solve the problem.

And if not, at least some subset of that information.

aaj3f commented 10 months ago

Generally speaking, I think one way to approach this ticket is to look at each test we have for SHACl in db and deliberately violate it. Almost always the error response is less than helpful (e.g. doesn't identify the node in violation, doesn't identify the property in violation, doesn't identify the value supplied on a property, doesn't clarify in human-readable terms the kind of rule being violated)

A few examples though:

When using sh:minCount: 1 to specify a property is required on a target, if an entity is transacted without that required property:

{
    "error": "db/shacl-validation",
    "message": "SHACL PropertyShape exception - sh:minCount of 1 higher than actual count of 0."
}

the error message would be more helpful if it specified any/all of the following:

When using sh:maxCount to specify single or limited cardinality on a target, if an entity is transacted with more than the allowed cardinality:

{
    "error": "db/shacl-validation",
    "message": "SHACL PropertyShape exception - sh:maxCount of 1 lower than actual count of 2."
}

the error message would be more helpful if it specified any/all of the following:

When using sh:datatype to specify the datatype on a property, if an entity is transacted with the wrong datatype:

{
    "error": "db/shacl-required",
    "message": "Required data type 1 does not match provided data type: 2"
}

the error message would be more helpful if it specified any/all of the following:

When using sh:closed to restrict a target to a close set of properties, if an entity is transacted with extraneous properties:

{
    "error": "db/shacl-validation",
    "message": "SHACL shape is closed, extra properties not allowed: [211106232532994]"
}

the error message would be more helpful if it specified any/all of the following:

dpetran commented 10 months ago

Here are some thoughts I have about implementation that may be helpful:

All of our validation is initiated through a call to shacl/validate-target, which in turn processes all of the validation results and throws the validation exception via shacl/throw-shacl-exception.

Right now our validators return error messages, but they don't have the context to create useful error messages. If instead of returning error messages they returned data about the constraint and value that failed validation, they could be composed into a data structure that shacl/throw-shacl-exception can translate into an actually useful error message, translating all relevant sids into iris, providing shape context of expected and actual values, and formatting everything with the user's context.

jdorety commented 10 months ago
{
    "error": "db/shacl-validation",
    "message": "SHACL shape is closed, extra properties not allowed: [200]"
}

I've been getting this one when trying to transact against a closed schema. Some identification of the offending property would be a huge help for debugging my SHACL node shapes.

jdorety commented 10 months ago

When attempting the same transaction on a new ledger, and changing sh:closed to false for each node shape, I got a much more esoteric error when attempting the same tx as before

{
    "error": "db/shacl-validation",
    "message": "SHACL PropertyShape exception - sh:datatype: every datatype must be 7; sh:datatype: every datatype must be 7."
}

I'll throw up a link to a gist to I'm not filling up this space with JSON objects

cap10morgan commented 10 months ago

Note to self: current progress on this is in a git stash named WIP improving SHACL error messages

aaj3f commented 8 months ago

@cap10morgan -- I think you mentioned somewhere that this ticket was on pause for a particular reason as it awaited something else, but just wanted to check if it was, in fact, blocked by something else. @jakep36 has some folks working on SHACL-related Nexus UI items this sprint, and it would be ideal if they could do those dev items (and if the features could go live) w/ the improved SHACL error responses.

I wouldn't be surprised if, given the number of SHACL errors that would ideally return IRIs for affected invalid facts, you were waiting on the new SID PR to continue, but again, just wanted to check

cap10morgan commented 8 months ago

Some work @zonotope is doing is either going to complete this task or necessitate starting over on it (hopefully with a smaller overall workload) anyway. I can assign him if he thinks that's appropriate or move this back to the backlog w/ a blocker linked. One of those is probably a better indicator of the current status.