Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subclass RuntimeException? #313

Closed
gnebehay opened this issue Apr 20, 2017 · 3 comments
Closed

Subclass RuntimeException? #313

gnebehay opened this issue Apr 20, 2017 · 3 comments

Comments

@gnebehay
Copy link

As far as I can see, the error handling in influxdb-java consists of Exceptions wrapped in a RuntimeException, like here: https://github.com/influxdata/influxdb-java/blob/master/src/main/java/org/influxdb/impl/InfluxDBImpl.java#L101
I presume that one reason for doing so was to make the Exception handling unchecked, which I am in principle not opposed to. However, this pattern forces me to surround calls to influxdb-java in try-catch blocks catching RuntimeExceptions. I just now ran into a case where I had a NullPointerException within this try-catch block that, being an instance of RuntimeException also gets caught. Wouldn't it be much better to create a custom exception, e.g. InfluxException that inherits from RuntimeException? This way client-side error handling gets much easier and if I am not mistaken, it would not even break existing code.

@majst01
Copy link
Collaborator

majst01 commented Apr 25, 2017

Yes, checked Exceptions are considered harmful nowadays. Where did you get a NPE ? If this is caused by influxdb-java it can be considererd as a bug and should reported as such. Im not keen to introduce a new exception for no clear reason.

@gnebehay
Copy link
Author

The NPE was caused by a bug in my own code, which ended up being caught by the try-catch block surrounding the influxdb-java calls, which is undesirable.

To make the whole thing clearer by an example:

try {
InfluxDB influxDB = InfluxDBFactory.connect(connectionString, username, password);

//myOwnCustomObject is set to null due to a bug
Object myOwnCustomObject = null;

//NPE occurs            
influxDB.write(... myCustomObject.myProperty ...)

catch (RuntimeException e) {
  // Want to handle only influxDB errors here, but the NPE gets caught here, too
}

If there was a custom InfluxException (extending RuntimeException), this would allow for this error handling:

try {
InfluxDB influxDB = InfluxDBFactory.connect(connectionString, username, password);

//myOwnCustomObject is set to null due to a bug
Object myOwnCustomObject = null;

//NPE occurs            
influxDB.write(... myCustomObject.myProperty ...)

catch (InfluxException e) {
  // Only influxDB errors end up here, the NPE bubbles up the stack
}

Not only NPEs cause this behaviour, but a ton of other Exeptions, too, as can be seen from the docs: https://docs.oracle.com/javase/7/docs/api/java/lang/RuntimeException.html

@gnebehay
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants