Add better error handling

[PhillSimonds]

Environment

• Python version: 3.7.9
• schema-enforcer version: 0.1.1

Proposed Functionality

Schema enforcer currently calls sys.exit(1) after an error occurs, rather than raising an exception which describes the failure case then allowing that exception to bubble up, quitting script execution if it's not handled. This feature request proposes adding custom exceptions (or non-custom exceptions, pending use case) to throw when an issue occurs instead of calling sys.exit().

Use Case

in schema enforcer's SchemaManager class, a test_dir file is verified for existence before schema enforcer iterates over test cases to verify that when data is checked for schema adherence, the validation yields an expected result. The following code block is in use in the function that does this.

if not os.path.exists(test_dir):
    error(f"Tried to search {test_dir} for {test_type} data, but the path does not exist.")
    sys.exit(1)

When this block of code is hit, the following message is generated

ERROR | Tried to search /<some_path>/schema/tests/syslog_servers/valid for valid data, but the path does not exist.

The ERROR message is printed in red, and the output is easy to understand for the user.

This feature proposes rewriting this code block, and others like it, so that a design pattern akin the the following would be used instead

if not os.path.exists(test_dir):
    raise TestDirNotFoundException(f"Tried to search {test_dir} for {test_type} data, but the path does not exist.")

I've not played around with how to suppress the stack trace to give an inteligable message instead of the long variant that can be difficult for users to understand, but think we should attempt to do so in some way/form.

[glennmatthews]

The general pattern I'd go with would be to have a base Exception class (SchemaEnforcerException or similar) that all case-specific exceptions could inherit from, and then your main CLI function(s) in cli.py would each have a structure like:

try:
    # do all of the schema-enforcer function calls that we're already doing
except SchemaEnforcerException as exc:
    error(str(exc))
    sys.exit(1)

The main benefit of this approach is that it makes it easier in future to use schema-enforcer as a library as part of some other program (without needing to go through the CLI) and the consumer of this library can decide for itself what it wants to do if these various errors are encountered (which may or may not be a sys.exit()).

[PhillSimonds]

Brilliant, thank you for the input! I'll work something up :).

[jvanderaa]

@PhillSimonds would updating of the error message format also be included in this or do I need a new issue for it?

Having just numbers such as intf:0:property is confusing if this happens somewhere further down. I spent a bit of time checking out line number 54 that was showing an error instead of going to the 54th instance within the list. This does make sense, but most CI tools that this would be considered inclusion to that would be a line number. So lots of attention was spent trying to figure out why line 54 was erroring, not the 54th list item. Perhaps a intf:[54]:property type thing would indicate to me to check the 54th item since it is within Python list notiation.

[PhillSimonds]

@jvanderaa I think we'll need a new issue for this as the string representing the error wouldn't change per this issue.