Skip to content

Commit 3478876

Browse files
pmaiTimmRuppert
andauthored
Improved error handling (#26)
* Add smarter error handling in checker bundles * Add more informative errors for input file errors * Add more test cases for error handling Fixes #24 Signed-off-by: Pierre R. Mai <pmai@pmsf.de> Signed-off-by: Timm Ruppert <timm.ruppert@persival.de> Co-authored-by: Timm Ruppert <timm.ruppert@persival.de>
1 parent 0134871 commit 3478876

File tree

3 files changed

+203
-145
lines changed

3 files changed

+203
-145
lines changed

qc_ositrace/checks/deserialization/deserialization_checker.py

Lines changed: 69 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ def run_checks(config: Configuration, result: Result) -> None:
2222
expected_type_name = config.get_config_param("osiType") or "SensorView"
2323
expected_type = OSITrace.map_message_type(expected_type_name)
2424

25-
trace = OSITrace(
26-
config.get_config_param("InputFile"), config.get_config_param("osiType")
27-
)
28-
2925
result.register_checker(
3026
checker_bundle_name=constants.BUNDLE_NAME,
3127
checker_id=deserialization_constants.CHECKER_ID,
@@ -69,51 +65,73 @@ def run_checks(config: Configuration, result: Result) -> None:
6965
rule_full_name="deserialization.expected_version",
7066
)
7167

72-
logging.info("Executing deserialization.expected_type check")
73-
logging.info("Executing deserialization.version_is_set check")
74-
logging.info("Executing deserialization.expected_version check")
75-
76-
for message in trace:
77-
if type(message) is not expected_type:
78-
issue_id = result.register_issue(
79-
checker_bundle_name=constants.BUNDLE_NAME,
80-
checker_id=deserialization_constants.CHECKER_ID,
81-
description=f"Deserialized message is not of expected type {expected_type}.",
82-
level=IssueSeverity.ERROR,
83-
rule_uid=type_rule_uid,
84-
)
85-
if not message.HasField("version"):
86-
issue_id = result.register_issue(
87-
checker_bundle_name=constants.BUNDLE_NAME,
88-
checker_id=deserialization_constants.CHECKER_ID,
89-
description="Version field is not set in top-level message.",
90-
level=IssueSeverity.ERROR,
91-
rule_uid=version_rule_uid,
68+
try:
69+
try:
70+
trace = OSITrace(
71+
config.get_config_param("InputFile"), config.get_config_param("osiType")
9272
)
93-
elif (
94-
expected_version is not None
95-
and (
96-
int(message.version.version_major),
97-
int(message.version.version_minor),
98-
int(message.version.version_patch),
99-
)
100-
!= expected_version
101-
):
102-
issue_id = result.register_issue(
103-
checker_bundle_name=constants.BUNDLE_NAME,
104-
checker_id=deserialization_constants.CHECKER_ID,
105-
description=f"Version field value {message.version.version_major}.{message.version.version_minor}.{message.version.version_patch} is not the expected version {'.'.join([str(s) for s in expected_version])}.",
106-
level=IssueSeverity.ERROR,
107-
rule_uid=exp_version_rule_uid,
108-
)
109-
110-
logging.info(
111-
f"Issues found - {result.get_checker_issue_count(checker_bundle_name=constants.BUNDLE_NAME, checker_id=deserialization_constants.CHECKER_ID)}"
112-
)
113-
114-
# TODO: Add logic to deal with error or to skip it
115-
result.set_checker_status(
116-
checker_bundle_name=constants.BUNDLE_NAME,
117-
checker_id=deserialization_constants.CHECKER_ID,
118-
status=StatusType.COMPLETED,
119-
)
73+
except Exception as e:
74+
logging.error(f"Error reading input file: {e}")
75+
raise RuntimeError(f"Error reading input file: {e}") from e
76+
77+
logging.info("Executing deserialization.expected_type check")
78+
logging.info("Executing deserialization.version_is_set check")
79+
logging.info("Executing deserialization.expected_version check")
80+
81+
for message in trace:
82+
if type(message) is not expected_type:
83+
issue_id = result.register_issue(
84+
checker_bundle_name=constants.BUNDLE_NAME,
85+
checker_id=deserialization_constants.CHECKER_ID,
86+
description=f"Deserialized message is not of expected type {expected_type}.",
87+
level=IssueSeverity.ERROR,
88+
rule_uid=type_rule_uid,
89+
)
90+
if not message.HasField("version"):
91+
issue_id = result.register_issue(
92+
checker_bundle_name=constants.BUNDLE_NAME,
93+
checker_id=deserialization_constants.CHECKER_ID,
94+
description="Version field is not set in top-level message.",
95+
level=IssueSeverity.ERROR,
96+
rule_uid=version_rule_uid,
97+
)
98+
elif (
99+
expected_version is not None
100+
and (
101+
int(message.version.version_major),
102+
int(message.version.version_minor),
103+
int(message.version.version_patch),
104+
)
105+
!= expected_version
106+
):
107+
issue_id = result.register_issue(
108+
checker_bundle_name=constants.BUNDLE_NAME,
109+
checker_id=deserialization_constants.CHECKER_ID,
110+
description=f"Version field value {message.version.version_major}.{message.version.version_minor}.{message.version.version_patch} is not the expected version {'.'.join([str(s) for s in expected_version])}.",
111+
level=IssueSeverity.ERROR,
112+
rule_uid=exp_version_rule_uid,
113+
)
114+
115+
logging.info(
116+
f"Issues found - {result.get_checker_issue_count(checker_bundle_name=constants.BUNDLE_NAME, checker_id=deserialization_constants.CHECKER_ID)}"
117+
)
118+
119+
result.set_checker_status(
120+
checker_bundle_name=constants.BUNDLE_NAME,
121+
checker_id=deserialization_constants.CHECKER_ID,
122+
status=StatusType.COMPLETED,
123+
)
124+
125+
except Exception as e:
126+
logging.error(f"Error during deserialization checks: {e}")
127+
logging.exception(e)
128+
result.set_checker_status(
129+
checker_bundle_name=constants.BUNDLE_NAME,
130+
checker_id=deserialization_constants.CHECKER_ID,
131+
status=StatusType.ERROR,
132+
)
133+
result.add_checker_summary(
134+
checker_bundle_name=constants.BUNDLE_NAME,
135+
checker_id=deserialization_constants.CHECKER_ID,
136+
content=f"Error: {str(e)}.",
137+
)

qc_ositrace/checks/osirules/osirules_checker.py

Lines changed: 106 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -445,15 +445,6 @@ def run_checks(config: Configuration, result: Result) -> None:
445445
)
446446
expected_type_name = config.get_config_param("osiType")
447447

448-
trace = OSITrace(config.get_config_param("InputFile"), expected_type_name)
449-
450-
result.register_checker(
451-
checker_bundle_name=constants.BUNDLE_NAME,
452-
checker_id=osirules_constants.CHECKER_ID,
453-
description="Evaluates messages in the trace file against the OSI Rules of the given OSI version to guarantee they are in conformance with the standard OSI rules.",
454-
summary="Checker validating OSI Rules compliance of messages in a trace file",
455-
)
456-
457448
if expected_version is None:
458449
logging.info(
459450
f"No expected version, falling back to {'.'.join([str(s) for s in fallback_version])} rules"
@@ -468,102 +459,123 @@ def run_checks(config: Configuration, result: Result) -> None:
468459
impresources.files(rulesyml)
469460
/ f"osi_{'_'.join(map(str,expected_version or fallback_version))}.yml"
470461
)
471-
try:
472-
with rules_file.open("rt") as file:
473-
rules = yaml.safe_load(file)
474-
rules_version = expected_version or fallback_version
475-
logging.info(
476-
f"Read {'custom' if custom_rules_file else 'standard'} rules file for version {'.'.join([str(s) for s in rules_version])}"
477-
)
478462

479-
except FileNotFoundError:
480-
logging.info(
481-
f"No {'custom' if custom_rules_file else 'standard'} rules file for expected version {'.'.join([str(s) for s in expected_version])}, falling back to standard {'.'.join([str(s) for s in fallback_version])} rules"
482-
)
483-
fallback_rules_file = (
484-
impresources.files(rulesyml)
485-
/ f"osi_{'_'.join(map(str,fallback_version))}.yml"
486-
)
487-
with fallback_rules_file.open("rt") as file:
488-
rules = yaml.safe_load(file)
489-
rules_version = fallback_version
490-
logging.info(
491-
f"Read standard rules file for version {'.'.join([str(s) for s in rules_version])}"
492-
)
493-
494-
version_rule_uid = result.register_rule(
495-
checker_bundle_name=constants.BUNDLE_NAME,
496-
checker_id=osirules_constants.CHECKER_ID,
497-
emanating_entity="asam.net",
498-
standard="osi",
499-
definition_setting="3.0.0",
500-
rule_full_name="osirules.version_is_set",
501-
)
502-
503-
exp_version_rule_uid = result.register_rule(
463+
result.register_checker(
504464
checker_bundle_name=constants.BUNDLE_NAME,
505465
checker_id=osirules_constants.CHECKER_ID,
506-
emanating_entity="asam.net",
507-
standard="osi",
508-
definition_setting="3.0.0",
509-
rule_full_name="osirules.expected_version",
466+
description="Evaluates messages in the trace file against the OSI Rules of the given OSI version to guarantee they are in conformance with the standard OSI rules.",
467+
summary="Checker validating OSI Rules compliance of messages in a trace file",
510468
)
511469

512-
# Register rules from rules yml
513-
rule_uid_map = register_automatic_rules(result, rules, rules_version)
470+
try:
471+
try:
472+
trace = OSITrace(config.get_config_param("InputFile"), expected_type_name)
473+
except Exception as e:
474+
logging.error(f"Error reading input file: {e}")
475+
raise RuntimeError(f"Error reading input file: {e}") from e
476+
477+
try:
478+
with rules_file.open("rt") as file:
479+
rules = yaml.safe_load(file)
480+
rules_version = expected_version or fallback_version
481+
logging.info(
482+
f"Read {'custom' if custom_rules_file else 'standard'} rules file for version {'.'.join([str(s) for s in rules_version])}"
483+
)
514484

515-
logging.info("Executing osirules.version_is_set check")
516-
logging.info("Executing osirules.expected_version check")
517-
logging.info("Executing osirules automatic checks")
485+
except FileNotFoundError as e:
486+
logging.error(
487+
f"No {'custom' if custom_rules_file else 'standard'} rules file for expected version {'.'.join([str(s) for s in expected_version or fallback_version])}: {e}"
488+
)
489+
raise RuntimeError(
490+
f"No {'custom' if custom_rules_file else 'standard'} rules file for expected version {'.'.join([str(s) for s in expected_version or fallback_version])}: {e}"
491+
) from e
492+
493+
version_rule_uid = result.register_rule(
494+
checker_bundle_name=constants.BUNDLE_NAME,
495+
checker_id=osirules_constants.CHECKER_ID,
496+
emanating_entity="asam.net",
497+
standard="osi",
498+
definition_setting="3.0.0",
499+
rule_full_name="osirules.version_is_set",
500+
)
518501

519-
for index, message in enumerate(trace):
520-
time = (
521-
message.timestamp.seconds + message.timestamp.nanos * 1e-9
522-
if hasattr(message, "timestamp") and message.HasField("timestamp")
523-
else None
502+
exp_version_rule_uid = result.register_rule(
503+
checker_bundle_name=constants.BUNDLE_NAME,
504+
checker_id=osirules_constants.CHECKER_ID,
505+
emanating_entity="asam.net",
506+
standard="osi",
507+
definition_setting="3.0.0",
508+
rule_full_name="osirules.expected_version",
524509
)
525-
if not message.HasField("version"):
526-
register_issue(
527-
result,
528-
message,
529-
index,
530-
time,
531-
version_rule_uid,
532-
IssueSeverity.ERROR,
533-
description="Version field is not set in top-level message.",
534-
)
535-
elif (
536-
expected_version is not None
537-
and (
538-
int(message.version.version_major),
539-
int(message.version.version_minor),
540-
int(message.version.version_patch),
510+
511+
# Register rules from rules yml
512+
rule_uid_map = register_automatic_rules(result, rules, rules_version)
513+
514+
logging.info("Executing osirules.version_is_set check")
515+
logging.info("Executing osirules.expected_version check")
516+
logging.info("Executing osirules automatic checks")
517+
518+
for index, message in enumerate(trace):
519+
time = (
520+
message.timestamp.seconds + message.timestamp.nanos * 1e-9
521+
if hasattr(message, "timestamp") and message.HasField("timestamp")
522+
else None
541523
)
542-
!= expected_version
543-
):
544-
register_issue(
545-
result,
546-
message,
547-
index,
548-
time,
549-
exp_version_rule_uid,
550-
IssueSeverity.ERROR,
551-
description=f"Version field value {message.version.version_major}.{message.version.version_minor}.{message.version.version_patch} is not the expected version {'.'.join([str(s) for s in expected_version])}.",
524+
if not message.HasField("version"):
525+
register_issue(
526+
result,
527+
message,
528+
index,
529+
time,
530+
version_rule_uid,
531+
IssueSeverity.ERROR,
532+
description="Version field is not set in top-level message.",
533+
)
534+
elif (
535+
expected_version is not None
536+
and (
537+
int(message.version.version_major),
538+
int(message.version.version_minor),
539+
int(message.version.version_patch),
540+
)
541+
!= expected_version
542+
):
543+
register_issue(
544+
result,
545+
message,
546+
index,
547+
time,
548+
exp_version_rule_uid,
549+
IssueSeverity.ERROR,
550+
description=f"Version field value {message.version.version_major}.{message.version.version_minor}.{message.version.version_patch} is not the expected version {'.'.join([str(s) for s in expected_version])}.",
551+
)
552+
553+
id_message_map = {}
554+
record_message_ids(message, id_message_map)
555+
check_message_against_rules(
556+
message, rule_uid_map, id_message_map, index, time, result
552557
)
553558

554-
id_message_map = {}
555-
record_message_ids(message, id_message_map)
556-
check_message_against_rules(
557-
message, rule_uid_map, id_message_map, index, time, result
559+
logging.info(
560+
f"Issues found - {result.get_checker_issue_count(checker_bundle_name=constants.BUNDLE_NAME, checker_id=osirules_constants.CHECKER_ID)}"
558561
)
559562

560-
logging.info(
561-
f"Issues found - {result.get_checker_issue_count(checker_bundle_name=constants.BUNDLE_NAME, checker_id=osirules_constants.CHECKER_ID)}"
562-
)
563+
result.set_checker_status(
564+
checker_bundle_name=constants.BUNDLE_NAME,
565+
checker_id=osirules_constants.CHECKER_ID,
566+
status=StatusType.COMPLETED,
567+
)
563568

564-
# TODO: Add logic to deal with error or to skip it
565-
result.set_checker_status(
566-
checker_bundle_name=constants.BUNDLE_NAME,
567-
checker_id=osirules_constants.CHECKER_ID,
568-
status=StatusType.COMPLETED,
569-
)
569+
except Exception as e:
570+
logging.error(f"Error during osirules checks: {e}")
571+
logging.exception(e)
572+
result.set_checker_status(
573+
checker_bundle_name=constants.BUNDLE_NAME,
574+
checker_id=osirules_constants.CHECKER_ID,
575+
status=StatusType.ERROR,
576+
)
577+
result.add_checker_summary(
578+
checker_bundle_name=constants.BUNDLE_NAME,
579+
checker_id=osirules_constants.CHECKER_ID,
580+
content=f"Error: {str(e)}.",
581+
)

tests/test_execution.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from qc_baselib import StatusType
2+
from qc_baselib.result import Result
3+
4+
import test_setup
5+
6+
7+
def test_nonexistent_input_file(monkeypatch) -> None:
8+
"""Test that non-existent input file results in error status for all checkers"""
9+
target_file_path = "path/to/nonexistent/file.osi"
10+
target_type = "SensorView"
11+
12+
test_setup.create_test_config(target_file_path, target_type)
13+
test_setup.launch_main(monkeypatch)
14+
15+
# Load result and verify error status
16+
result = Result()
17+
result.load_from_file(test_setup.REPORT_FILE_PATH)
18+
19+
checker_bundles = result.get_checker_bundle_results()
20+
assert len(checker_bundles) > 0, "No checker bundles found in result"
21+
22+
for bundle in checker_bundles:
23+
checkers = bundle.checkers
24+
assert len(checkers) > 0, f"No checkers found in bundle {bundle}"
25+
for checker in checkers:
26+
assert checker.status == StatusType.ERROR
27+
28+
test_setup.cleanup_files()

0 commit comments

Comments
 (0)