Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: propagate exceptions from slow_validate #581

Merged
merged 3 commits into from
Nov 25, 2019

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Nov 23, 2019

If an exception other than TraitError occurs in a slow_validate call, it should be propagated rather than swallowed. Propagation is the usual behaviour, but it doesn't happen for code that goes through the validate_trait_complex code path. This PR fixes that.

The particular case that motivated this PR: given a Trait definition:

class A(HasTraits):
    foo = Either(None, Range(0, 100))

if the range validation check fails due to a bad __index__ method on the target object, for example, we would expect to see the exception from that method. Instead, it's currently swallowed.

@codecov-io
Copy link

codecov-io commented Nov 23, 2019

Codecov Report

Merging #581 into master will increase coverage by 0.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #581     +/-   ##
=========================================
+ Coverage   65.85%   66.15%   +0.3%     
=========================================
  Files          44       44             
  Lines        6900     6900             
  Branches     1404     1404             
=========================================
+ Hits         4544     4565     +21     
+ Misses       1939     1922     -17     
+ Partials      417      413      -4
Impacted Files Coverage Δ
traits/trait_types.py 64.48% <0%> (+0.88%) ⬆️
traits/etsconfig/etsconfig.py 63.58% <0%> (+6.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3833ac8...aeeb318. Read the comment docs.

@mdickinson
Copy link
Member Author

Note: while this change could be regarded as a bugfix, it's got quite high potential for causing accidental breakage in real projects (even though any code it breaks is arguably itself buggy), so it wouldn't be appropriate to put this in a bugfix release. But I think it's okay for 6.0.

@mdickinson mdickinson merged commit aa89d3f into master Nov 25, 2019
@mdickinson mdickinson deleted the fix/propagate-exceptions-in-slow-validate branch November 25, 2019 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants