For forbid_extra_keys raise custom ForbiddenExtraKeyError

[raabf]

When forbid_extra_keys is actived, I would like to be able to try-except that specific Problem (here ForbiddenExtraKeyError) instead of any Exception.

The unit tests also gain a little more percition through that change.

[codecov-commenter]

Codecov Report

Merging #225 (5e136f5) into main (5675952) will increase coverage by 0.38%.
The diff coverage is 100.00%.

❗ Current head 5e136f5 differs from pull request most recent head b20eab5. Consider uploading reports for the commit b20eab5 to get more accurate results

@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   97.94%   98.33%   +0.38%     
==========================================
  Files          16       16              
  Lines         779      719      -60     
==========================================
- Hits          763      707      -56     
+ Misses         16       12       -4     

Impacted Files	Coverage Δ
src/cattr/errors.py	100.00% <100.00%> (ø)
src/cattr/dispatch.py	100.00% <0.00%> (ø)
src/cattr/preconf/json.py	100.00% <0.00%> (ø)
src/cattr/preconf/ujson.py	100.00% <0.00%> (ø)
src/cattr/disambiguators.py	100.00% <0.00%> (ø)
src/cattr/preconf/orjson.py	100.00% <0.00%> (ø)
src/cattr/_compat.py	94.77% <0.00%> (+0.54%)	⬆️
src/cattr/converters.py	99.04% <0.00%> (+0.64%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[Tinche]

I think this is a good idea but it should wait until after the extended validation branch is merged, which is blocked by other things temporarily.

[raabf]

Sure, order the MR as it fits best. 😄

[Tinche]

Hello, I'm preparing a new release and let's get this merged!

So please rebase, and I would ask for a change: add a field to the exception (extra_keys) that carries the list of extra keys. This will make it easier to introspect these exceptions programmatically.

[raabf]

add a field to the exception (extra_keys) that carries the list of extra keys. This will make it easier to introspect these exceptions programmatically.

Yea that is nice – I have added this and a reference to class to which the extra keys should be used (i.e. to which class constructor the extra keys were passed). I also extended unit test to check that attributes.

However, there is a small problem, since ForbiddenExtraKeysError can conflict with ClassValidationError. I assumed that an ForbiddenExtraKeysError means that the Class Validation failed. That means if ClassValidationError is used, I add ForbiddenExtraKeysError to the errors list which is then passed to the ClassValidationError. This is more or less anyway implicitly done, because if ForbiddenExtraKeysError in a nested class is raised it will be catched anyway by its upper class because you have added except Exception as e: to every attribute. See this commit where I have implemented this behaviour fd5a9b8 . I hope this is what you have intended.

[raabf]

So that it is easy to understand this is an example generated structure function (unknows_fields are added to errors list):

def structure_A(o, _, *, __cl=__cl, __c_cve=__c_cve, __c_structure_c=__c_structure_c, __c_type_c=__c_type_c, __c_structure_a=__c_structure_a):
  res = {}
  errors = []
  try:
    res['c'] = __c_structure_c(o['c'], __c_type_c)
  except Exception as e:
    e.__note__ = 'Structuring class test_forbid_extra_keys_nested_override.<locals>.A @ attribute c'
    errors.append(e)
  if 'a' in o:
    try:
      res['a'] = __c_structure_a(o['a'])
    except Exception as e:
      e.__note__ = 'Structuring class test_forbid_extra_keys_nested_override.<locals>.A @ attribute a'
      errors.append(e)
  unknown_fields = set(o.keys()) - __c_a
  if unknown_fields:
    errors.append(__c_feke('', __cl, unknown_fields)) 
  if errors: raise __c_cve('While structuring A', errors, __cl)
  try:
    return __cl(
      **res,
    )
  except Exception as exc: raise __c_cve('While structuring A', [exc], __cl)

[Tinche]

Yep, that looks great. Thanks a lot!