stk icon indicating copy to clipboard operation
stk copied to clipboard

Add MutationPreconditionException

Open lukasturcani opened this issue 4 years ago • 1 comments

I re-opened #250 for this. Basically, I do not like using None to indicate an expected failure in the Mutator.mutate() method because it loses a lot of information. For example, it is not clear why the failure happened, because there is no message and no way to distinguish different types of failure - they all return None. This API is also dangerous because I think it can be used by people to suppress errors which should not be suppressed. I.e by just returning None when there is any error, in order to suppress it and prevent the EA from crashing.

The solution to this is fairly obvious. Mutator.mutate() is allowed to raise specific types of exceptions, such as MutationPreconditionViolation, which the EA, which does the actual calling of Mutator.mutate(), can then expect and handle in a reasonable way. This system should be extensible, because we can introduce exceptions for other types of errors which we want to EA to handle. This system is very conservative about the types exceptions we expect the EA to handle, it does not introduce a general silencing mechanism. Also, because the allowed exception types are specific, like MutationPreconditionViolation, this is unlikely to get abused as a general error handling mechanism, for example by wrapping any error into a MutationPreconditionViolation. I think it would be obvious to any reasonable person that this would be bad form.

@andrewtarzia could you review this for me please? Also @stevenbennett96 I think this may be interesting to you.

lukasturcani avatar Jan 01 '21 23:01 lukasturcani

This pull request introduces 1 alert when merging 213e7de1f696cfe92d2e5f72b3e172b9c1b453d6 into b01482650952ec96ed4e6beb2f98d8de17ad41df - view on LGTM.com

new alerts:

  • 1 for 'import *' may pollute namespace

lgtm-com[bot] avatar Jan 02 '21 00:01 lgtm-com[bot]