mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701045992
##########
File path: clicache/src/Cache.cpp
##########
@@ -215,12 +233,18 @@ namespace Apache
GC::KeepAlive(m_nativeptr);
}
- _GF_MG_EXCEPTION_CATCH_ALL2
+ }
+ catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+ }
+ catch (System::AccessViolationException^ ex) {
Review comment:
Interesting question that deserves some thought. AccessViolations are
baaaad. So you don't want to just eat them. OK, good, we don't. We rethrow
them. But why catch them in the first place?
The only thing I can come up with is to help track down bugs in our code. We
catch/rethrow AccessViolations all over the place (212 places), so it allows
setting breakpoints which can be caught in the debugger so we can look around.
This may have been useful early on, but I doubt it provides any value now.
I'm good with removing: catch (System::AccessVilationException) everywhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
