-
Notifications
You must be signed in to change notification settings - Fork 27
Errorhandling preprocessor #1445
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
Conversation
RamilCDISC
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please resolve the conflicts on the branch too. Thank you.
|
I also looked into the ticket Gerry referenced today in review #1432 This logic does not interfere with that portion of the codebase |
RamilCDISC
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR adds exception handling to improve error handling. This helps skipping rules instead of just breaking the execution. The PR was validated by:
- Reviewing the PR code for any unwanted changes or comments.
- Reviewing the PR Logic in context of AC.
- Validating all unit and regression testing pass.
- Ensuring all related testing is updated.
- Ensuring all the error message are clear and coherent.
- Ensuring the new custom exceptions follow the pattern of previous custom exceptions.
- Ensuring the exception is fully representative of code in the try block.
- Ensuring the validation does not break where it should just skip the in-processing rule.
This PR adds custom exceptions that get raised during several key parts of validation that were skipped by error coverage
Custom exceptions and messages with key metadata have been created to propagate up to validate single dataset which contains the try/except block that handles exceptions.
the attached issue has a rule/dataset that fails with relrec merge. Of note--i kept the caught stack trace as it is useful for development resolution of bugs but the added metadata context in the error messages should help rule authors with. Also of note--I was uncertain whether to assign a rule skip or not for these---I though it would be worthwhile to skip the rule to continue execution despite these being 'execution errors'.