This repository has been archived by the owner on May 21, 2024. It is now read-only.
Improve libaktualizr API exceptions. #1754
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In each case, the "TODO(OTA-2178)" comments were no longer helpful. The iterator exceptions are fine as is, and if the SQL code fails, it is acceptable to throw that through the API, since that is probably a serious issue. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
The user won't understand what to do with that exception, so better to print an error with some context. This scenario is most likely recoverable anyway. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Since these calls can call custom/external code, this is the safest course of action. All errors are printed to the log and errors are returned through the API as appropriate. The assumption is that these cases are generally recoverable. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Memory and mutex-related exceptions can be thrown almost everywhere, and those represent situations that are most likely difficult to recover from. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
This is definitely recoverable. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Most were already caught as part of the metadata parsing in the Uptane namespace, but some miscellaneous things could still slip through. They are highly unlikely, but just in case, catch them. They are generally recoverable and do not need to be passed through the API. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Use SQLInternalException for the specifically low-level errors. Improve some of the context as well. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
pattivacek
force-pushed
the
feat/OTA-5089/better-exceptions
branch
from
26688f1
to
39c90b5
Compare
Codecov Report
@@ Coverage Diff @@
## master #1754 +/- ##
==========================================
- Coverage 81.98% 76.35% -5.64%
==========================================
Files 178 178
Lines 12481 13415 +934
==========================================
+ Hits 10233 10243 +10
- Misses 2248 3172 +924
Continue to review full report at Codecov.
|
aodukha
approved these changes
Sep 1, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Generally, only fatal exceptions should be thrown through the API. Non-fatal exceptions are caught inside libaktualizr and converted to error codes. Also:
There is probably still room for improvement in the SQL code. However, it's generally fairly low-level and only internally relevant, so it didn't seem worth the effort to refactor it too substantially. Some functions return bools while others throw exceptions, and some do both. It might be nice to only throw exceptions, but that would take some work to get right, and no one seems to bothered by the current state of affairs.