fix: correct returned ErrorType in deserialize method (0.15)#1735
fix: correct returned ErrorType in deserialize method (0.15)#1735
Conversation
fixes: #1557 Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the error handling within the deserialization logic by ensuring that out-of-memory conditions are reported with the correct error type. This change improves the precision of error reporting, making it easier to diagnose and address memory-related issues during data loading. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request corrects the ErrorType thrown during deserialization when a std::bad_alloc exception is caught, changing it from READ_ERROR to NO_ENOUGH_MEMORY. The change is correct. For consistency, you may want to review other catch (const std::bad_alloc& e) blocks. For instance, the InnerIndexInterface::Deserialize(const ReaderSet&) method in the same file appears to have a similar issue where READ_ERROR is thrown instead of NO_ENOUGH_MEMORY.
There was a problem hiding this comment.
Pull request overview
Updates InnerIndexInterface::Deserialize to throw a more specific ErrorType when deserialization fails due to an out-of-memory condition (fix for #1557).
Changes:
- Map
std::bad_allocduring deserialization toErrorType::NO_ENOUGH_MEMORYinstead ofErrorType::READ_ERROR.
| return; | ||
| } catch (const std::bad_alloc& e) { | ||
| throw VsagException(ErrorType::READ_ERROR, "failed to Deserialize: ", e.what()); | ||
| throw VsagException(ErrorType::NO_ENOUGH_MEMORY, "failed to Deserialize: ", e.what()); |
There was a problem hiding this comment.
The error message text uses an unusual capitalization (“Deserialize”) mid-sentence. Consider changing it to a consistent, conventional form like “failed to deserialize” (and optionally include the fully-qualified function name) to improve log/search consistency.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## 0.15 #1735 +/- ##
==========================================
+ Coverage 90.18% 90.27% +0.08%
==========================================
Files 218 218
Lines 13518 13518
==========================================
+ Hits 12191 12203 +12
+ Misses 1327 1315 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
fixes: #1557