[GitHub] [accumulo] milleruntime commented on pull request #2946: Made CryptoException extend IOException

[GitBox <gi...@apache.org>]

milleruntime commented on PR #2946:
URL: https://github.com/apache/accumulo/pull/2946#issuecomment-1255231894

   > Do you think this should not be merged then and leave CryptoException as a RuntimeException. I was thinking that having a known checked Exception in the SPI methods makes sense so that errors specific to crypto can be handled.
   
   For most situations, I don't think there will be much we can do if we catch a CryptoException (coming from a TabletServer, for example) other than restart the server. Since these interfaces are in the SPI, the "client" who handles the exception will usually be server code. And with the way the CryptoService runs within the tserver, there is a good chance a restart will have to happen to reload the service. Eventually, once the Crypto is properly tested we could drop the experimental tag and be able to determine a better way of handling the errors. Or if we create a designated Server running just for Crypto, there may be better ways to handle the errors. The RFile API may be a good place to throw a checked exception for example, allowing the client to handle encryption errors.
   
   I do think there is room for improving the overall error handling though. But maybe changing the exception type isn't the right approach.


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org