fix: make VsagException public inherit from std::exception#1788
fix: make VsagException public inherit from std::exception#1788
Conversation
The class was using private inheritance which prevented catching VsagException with std::exception handler, breaking standard C++ exception handling patterns. Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
inabao
requested review from
LHT129,
ShawnShawnYou,
jiaweizone and
wxyucs
as code owners
![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 correctly updates the inheritance of VsagException to public and reorders the catch blocks in Engine::CreateIndex to ensure that the more specific VsagException is caught before the general std::exception. A review comment suggests optimizing the logging macro call to avoid unnecessary string concatenation by passing arguments separately.
| LOG_ERROR_AND_RETURNS( | ||
| ErrorType::NO_ENOUGH_MEMORY, "failed to create index(not enough memory): ", e.what()); | ||
| } catch (const vsag::VsagException& e) { | ||
| LOG_ERROR_AND_RETURNS(e.error_.type, "failed to create index: " + e.error_.message); |
There was a problem hiding this comment.
For consistency with other LOG_ERROR_AND_RETURNS calls in this file and to avoid an unnecessary temporary string allocation, you should pass the error message as a separate argument to the macro instead of using the + operator. The macro is variadic and handles concatenation internally using a stringstream.
| LOG_ERROR_AND_RETURNS(e.error_.type, "failed to create index: " + e.error_.message); | |
| LOG_ERROR_AND_RETURNS(e.error_.type, "failed to create index: ", e.error_.message); |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## 0.18 #1788 +/- ##
==========================================
- Coverage 91.30% 91.10% -0.21%
==========================================
Files 328 328
Lines 19340 19340
==========================================
- Hits 17658 17619 -39
- Misses 1682 1721 +39
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:
|
2 participants
Summary
Cherry-pick of PR #1776 to branch 0.18
catch (std::exception& e)handlers, following standard C++ exception handling patternsProblem
The class was using private inheritance (default for class):
This prevented catching VsagException with std::exception handler, breaking standard exception handling.
Solution
Changed to public inheritance:
And reordered catch blocks to ensure derived class (VsagException) is caught before base class (std::exception).
Changes
src/vsag_exception.h: Changed to public inheritancesrc/factory/engine.cpp: Reordered catch blocks (VsagException before std::exception)Impact