Skip to content

Commit

Permalink
Provide a more detailed message in constraint error details field (#3543
Browse files Browse the repository at this point in the history
)

Currently, constraint violation errors include a nicely-formatted
message that is intended to be displayed to application end user, and
so don't include any extra technical details, such as the type on which
the constraint is defined.  This makes life harder on developers, so
include a more elaborate message in the `details` field:

    edgedb> insert City2 { name := 'sesh' };
    edgedb error: ConstraintViolationError: name violates exclusivity constraint
      Detail: value of property 'name' of object type 'default::City2' violates exclusivity constraint

It would also be nice to include the constraint definition and the
offending value, but that requires a more elaborate fix.

Fixes: #3522
  • Loading branch information
elprans committed Feb 23, 2022
1 parent 936258b commit d9550ae
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
11 changes: 9 additions & 2 deletions edb/schema/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,10 @@ def get_subject(self, schema: s_schema.Schema) -> ConsistencySubject:
self.get_field_value(schema, 'subject'),
)

def format_error_message(
def format_error(
self,
schema: s_schema.Schema,
) -> str:
errmsg = self.get_errmessage(schema)
subject = self.get_subject(schema)
titleattr = subject.get_annotation(schema, sn.QualName('std', 'title'))

Expand All @@ -208,6 +207,14 @@ def format_error_message(
else:
subjtitle = titleattr

return self.format_error_message(schema, subjtitle)

def format_error_message(
self,
schema: s_schema.Schema,
subjtitle: str,
) -> str:
errmsg = self.get_errmessage(schema)
args = self.get_args(schema)
if args:
args_ql: List[qlast.Base] = [
Expand Down
8 changes: 6 additions & 2 deletions edb/server/compiler/errormech.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,12 @@ def _interpret_constraint_errors(code, schema, err_details, hint):

constraint = schema.get_by_id(constraint_id)

return errors.ConstraintViolationError(
constraint.format_error_message(schema))
msg = constraint.format_error(schema)
subject = constraint.get_subject(schema)
vname = subject.get_verbosename(schema, with_parent=True)
subjtitle = f"value of {vname}"
details = constraint.format_error_message(schema, subjtitle)
return errors.ConstraintViolationError(msg, details=details)
elif error_type == 'newconstraint':
# If we're here, it means that we already validated that
# schema_name, table_name and column_name all exist.
Expand Down

0 comments on commit d9550ae

Please sign in to comment.