Replace RuntimeException with new UncheckedIOException #190
Conversation
This is the same as Java 8's [UncheckedIOException](https://docs.oracle.com/javase/8/docs/api/java/io/UncheckedIOException.html). We've copied it so that we can maintain compatibility with Java 7. Other options: * Continue to throw RuntimeException with a nicer error message (i.e. remove the "todo") * Create a custom exception Tape class like we had in tape v1 (ref #154) I think this is nicer than the second as it keeps the ergonomics of Java 8 for those stuck on having to support Java 7.
|
opened this since this is the last blocker to releasing tape 2 (from #152). |
| * @param cause the {@code IOException} | ||
| * @throws NullPointerException if the cause is {@code null} | ||
| */ | ||
| UncheckedIOException(IOException cause) { |
There was a problem hiding this comment.
Should this be public so people can create instances for testing?
|
I agree a custom retrofit subclass is not great and just expands the api area folks have to deal with. I'm cagey around copying anything oracle, but I'm not sure how things fare in general open-source projects. I guess I tend towards dropping the TODO? Or maybe make the text more explicitly that we'll move toward UncheckedIOException eventually? That is, something like "TODO UncheckedIOException". |
| * This class is available in Java 8 (https://docs.oracle.com/javase/8/docs/api/java/io/UncheckedIOException.html). | ||
| * We've copied the implementation so that we can maintain Java 7 compatibility. | ||
| */ | ||
| public class UncheckedIOException extends RuntimeException { |
| * @param cause the {@code IOException} | ||
| * @throws NullPointerException if the cause is {@code null} | ||
| */ | ||
| UncheckedIOException(IOException cause) { |
| * @return the {@code IOException} which is the cause of this exception. | ||
| */ | ||
| @Override | ||
| public synchronized IOException getCause() { |
There was a problem hiding this comment.
why is this synchronized?
There was a problem hiding this comment.
It's because Throwable.getCause is synchronized.
There was a problem hiding this comment.
oh, TIL. initCause and getCause are indeed.
i saw java.io.UncheckedIOException's getCause wasn't synchronized and didn't look further. thanks.
|
tried this out with doing a sneaky throw for IOException: NightlyNexus@9b0ccc1 |
|
I actually didn't think of sneaky throw here. I think it might be a better option than than a custom class. Should we just merge in NightlyNexus@9b0ccc1 ? |
|
sent #193. |
|
Closed by #193 |
This is the same as Java 8's UncheckedIOException. We've copied it so that we can maintain compatibility with Java 7.
Other options:
I think this is nicer than the second as it keeps the ergonomics of Java 8 for those stuck on having to support Java 7.