nebula-java
nebula-java copied to clipboard
Need more explicit exceptions so its easier to test in application code
For instance
new RuntimeException(
"Switch space `"
+ config.getSpaceName()
+ "' failed: "
+ resultSet.getErrorMessage());
If this was a NoSpaceFoundException it would be easier to test. Frequently we'd like know if an exception is one we can retry so having ones that implement TransitentException or some ClientException is helpful.
For instance this NoSpaceFoundException is not retryable.
The client exposed the ResultSet's errorCode to users and expect users handle the business code according to errorCode. Can you check the errorCode type to decide whether you can try the statement?
@Nicole00 I'm confused, the error codes are present in the code however, the exception thrown from library does not contain the code or a clarifying exception.
getSessionWrapper()
// create new session
try {
Session session =
pool.getSession(
config.getUserName(), config.getPassword(), config.isReconnect());
ResultSet resultSet = session.execute("USE " + config.getSpaceName());
if (!resultSet.isSucceeded()) {
throw new RuntimeException(
"Switch space `"
+ config.getSpaceName()
+ "' failed: "
+ resultSet.getErrorMessage());
}
SessionWrapper sessionWrapper = new SessionWrapper(session);
sessionList.add(sessionWrapper);
return sessionWrapper;
} catch (AuthFailedException | NotValidConnectionException | IOErrorException e) {
throw new RuntimeException("Get session failed: " + e.getMessage());
}
This is from SessionManager.java basically it just throws RuntimeException wtihout an error code so the user would need to parse the error message.
Basically how does this code tell the client its a AuthFailedException or a Switch space, as in either case it throws RuntimeException.
This is a common pattern in the library code it throws a lot of plain RuntimeException with out passing the error code or a clarifying exception. Basically taking AuthFailedException and have it inherit from RuntimeException would help. Or better yet a basse ClientException with the error code and then extend it from there for something like AuthFailedFailed or CertificateException etc.
I agree with the view on runtime exception, it should indeed throw the specific exception. What I mean above is about the errorCode returned by server, it is contained in ResultSet.
Perhaps the SessionWrapper will be Deprecated later.