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

[Part 1] Improved error handling with beautiful trace and exit code in cli, input_engine and cvedb #798

Merged
merged 10 commits into from Jul 17, 2020

Conversation

Niraj-Kamdar
Copy link
Contributor

@Niraj-Kamdar Niraj-Kamdar commented Jul 12, 2020

Features added:

  • colorful trace
  • custom exit code
  • Four different error handling mode:
    1. TruncTrace (default)
      tructrace
    2. FullTrace (while logging level debug)
      WindowsTerminal_wId5SU5O2V
    3. NoTrace (while quite mode)
      WindowsTerminal_o1lKMeMB0b
    4. Ignore
      It will ignore exception and is for internal use only.

Fixes #787 and #788.

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2020

Codecov Report

Merging #798 into master will increase coverage by 5.06%.
The diff coverage is 78.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #798      +/-   ##
==========================================
+ Coverage   82.40%   87.47%   +5.06%     
==========================================
  Files         147      148       +1     
  Lines        2523     2594      +71     
  Branches      297      305       +8     
==========================================
+ Hits         2079     2269     +190     
+ Misses        371      254     -117     
+ Partials       73       71       -2     
Flag Coverage Δ
#longtests 87.47% <78.90%> (+5.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/csv2cve.py 0.00% <0.00%> (ø)
cve_bin_tool/cvedb.py 85.89% <27.27%> (+30.34%) ⬆️
cve_bin_tool/error_handler.py 81.63% <81.63%> (ø)
cve_bin_tool/cli.py 84.48% <83.33%> (+6.70%) ⬆️
cve_bin_tool/input_engine.py 93.44% <100.00%> (+0.22%) ⬆️
test/test_cli.py 100.00% <100.00%> (+17.27%) ⬆️
test/test_input_engine.py 100.00% <100.00%> (ø)
cve_bin_tool/version_scanner.py 85.71% <0.00%> (+1.78%) ⬆️
cve_bin_tool/extractor.py 61.06% <0.00%> (+3.05%) ⬆️
... and 6 more

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 6335920...b825724. Read the comment docs.

fix extractor problems.
Copy link
Contributor Author

@Niraj-Kamdar Niraj-Kamdar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed modernize exception handling of almost every module except OutputEngine since @SinghHrmn is working on it. I have talked with him and he will incorporate changes in his future PR.

Comment on lines 1 to 5
import sys
from enum import Enum

from rich.console import Console
from rich.traceback import Traceback
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created new module for exception handling.

Comment on lines +10 to +16
class InsufficientArgs(Exception):
""" Insufficient command line arguments"""


class InvalidCsvError(Exception):
""" Given File is an Invalid CSV """

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved all exception to one place in error_handler so that new contributor can easily see which exit code is used and always give unique exit code.

Comment on lines +73 to +77
def excepthook(exc_type, exc_val, exc_tb):
trace = Traceback.from_exception(exc_type, exc_val, exc_tb)
CONSOLE.print(trace)
if ERROR_CODES.get(exc_type):
sys.exit(ERROR_CODES[exc_type])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom excepthook with colorized traceback.

Comment on lines +81 to +91
class ErrorHandler:
"""
@summary: Error handler context manager.
usecases:
Different modes that can ignore error, print full trace, truncated trace and no trace.
Log messages if logger specified.
@param: mode (ErrorMode): Can take any valid ErrorMode as an arg and change output according to that.
@param: logger: logs error message specified while raising Exception if logger is passed
while class initialization.
Ex: raise ValueError("file required") will log 'file required'.
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New ErrorHandler context manager. It supports 4 types of mode Ignore (ignore error), NoTrace (trace won't print), TruncTrace(truncated trace for the context only (default)), FullTrace (full trace default when debugging.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While using ErrorHandler context manager if error get throws it sets exit_code to error_code of that error. This will be helpful in Ignore mode to inspect if exception get throw.

Comment on lines 119 to 133
ERROR_CODES = {
SystemExit: -2,
FileNotFoundError: -3,
InvalidCsvError: -4,
InvalidJsonError: -4,
MissingFieldsError: -5,
InsufficientArgs: -6,
EmptyCache: -7,
CVEDataForYearNotInCache: -8,
CVEDataForCurlVersionNotInCache: -9,
AttemptedToWriteOutsideCachedir: -10,
SHAMismatch: -11,
ExtractionFailed: -12,
UnknownArchiveType: 13,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionary mapping of exception with the error code.

Comment on lines +172 to +178
if 0 < LOGGER.level <= 10:
error_mode = ErrorMode.FullTrace
elif LOGGER.level >= 50:
error_mode = ErrorMode.NoTrace
else:
error_mode = ErrorMode.TruncTrace

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set error mode according to logging level. It will not print exception trace in quite mode (Currently we are printing that)

Comment on lines -124 to +108
raise AttemptedToWriteOutsideCachedir(filepath)
with ErrorHandler(mode=self.error_mode, logger=self.LOGGER):
raise AttemptedToWriteOutsideCachedir(filepath)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always use ErrorHandler context manager while raising exception.

cve_bin_tool/extractor.py Outdated Show resolved Hide resolved
cve_bin_tool/extractor.py Outdated Show resolved Hide resolved
Comment on lines +62 to +64
with self.assertRaises(SystemExit) as e:
main(["cve-bin-tool"])
self.assertEqual(e.exception.args[0], -6)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modernize tests

Separate PR for extractor
@Niraj-Kamdar Niraj-Kamdar changed the title Improved error handling with beautiful trace and exit code. [Part 1] Improved error handling with beautiful trace and exit code. Jul 13, 2020
@Niraj-Kamdar Niraj-Kamdar changed the title [Part 1] Improved error handling with beautiful trace and exit code. [Part 1] Improved error handling with beautiful trace and exit code in cli, input_engine and cvedb Jul 13, 2020
cve_bin_tool/error_handler.py Outdated Show resolved Hide resolved
Copy link
Member

@pdxjohnny pdxjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Terri's comment

Co-authored-by: Terri Oda <terri@toybox.ca>
@pdxjohnny pdxjohnny merged commit 2304b4b into intel:master Jul 17, 2020
@Niraj-Kamdar Niraj-Kamdar deleted the error branch July 18, 2020 06:07
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.

Improve consistency in returning error codes in cli.py and the csv2cve.py stub
4 participants