Handling non-IOExceptions in Take is broken

[piotrkot]

Take interface expects to throw IOException. This is limiting. In my Take implementations I often make SQL calls or JSON manipulations which throw checked SQLException or checked JsonException.
Why Take cannot throw Exception, instead? Especially, as in the blog I can read "use only Exception, without any sub-types.".

[0crat]

@paulodamaso/z please, pay attention to this issue

[0crat]

@piotrkot/z this project will fix the problem faster if you donate a few dollars to it; just click here and pay via Stripe, it's very fast, convenient and appreciated; thanks a lot!

[paulodamaso]

@piotrkot This is a very sensitive question. Looks like we have three options here:

1. Let Take exception handling as is and wrap other exceptions in IOException
2. Update Take to throw Exception as said in Yegor's article
3. Remove all exception handling for Take interface considering the arguments presented in this article by Sergey Kapralov

Drawbacks:

1. Take will still limit exception type to IOException
2. Catching a generic exception such Exception is marked as a violation in checkstyle, so it's considered a bad pratice
3. Not using checked exceptions at all makes us blind to potential failures in some methods

@yegor256 as this is a very controversial subject I'd like to know your last opinion about this

[piotrkot]

@paulodamaso Thank you for listing out the options.

If you asked me, I prefer option no. 2. In fact, Takes is already catching Throwable in BkBasic and TkFallback. And that is OK because you can never trust what users put into their Take implementations. But after all it's the framework which must handle it.

Personally, I don't agree with the article by Sergey Kapralov. Please, keep Take throwing checked exceptions.

[paulodamaso]

@yegor256 please take a look at this

[paulodamaso]

@yegor256 ping

[yegor256]

@piotrkot @paulodamaso I think we should continue using IOException

[piotrkot]

First of all, Merry Christmas and Happy New Year to everyone! :)

@yegor256 Please, explain why you think so.
Let me share some thoughts.
Takes is a web framework. Unlike in libraries, you expect/invite users to put their logic (business logic) into your code skeleton. Here, you want them to make new implementations of org.takes.Take interface. I make Take implementations that send emails, call external services, persist data in storage systems, etc. All such implementations throw different checked exceptions. Why do I have to wrap each exception into IOException? Why did you decide to use IOException as a general type exception? From your blog I see you are an advocate of using general Exception class, aren't you? As you are already catching Exception in BkBasic and TkFallback, why don't you let it throw in general purpose Take implementations?

[fabriciofx]

@yegor256 So, why did you allow change IOException to Exception in Cactoos, even in the Input and Output interfaces?

[yegor256]

@piotrkot @fabriciofx OK, OK, you got me. I agree, let's do throws Exception.

[skapral]

@yegor256 @piotrkot @fabriciofx @paulodamaso

As an author of the post, mentioned in this discussion, let me bring up my point of view on this.

I strongly disagree with this statement:

Not using checked exceptions at all makes us blind to potential failures in some methods

Actually, it completely doesn't matter whether your exceptions are checked or unchecked - you will still be blind to potential failures. Because the fact that some method is annotated with throws Exception won't give you any hint on whether the exception is thrown by this method, or by some method call on the object dependencies. Checked exceptions don't make any difference, while bringing only troubles I mentioned in the post.

I'd vote for:

• removing all throws Exception statements in the project
• always throwing unchecked exceptions
• in case when some 3rd party API throws some checked exception, immediately wrap it to some unchecked one and rethrow.

[piotrkot]

@skapral The issue here is about something else. However, as you were mentioned, I personally don't mind to discuss a bit more for clarity. (If you want to remove all checked exceptions from Takes, you would have to open a new issue).

throws keyword declares an exception. It gives an information to the programmer that there may occur an exception and it must be handled. This information is quite helpful for me. Why do you want to have it missing?

[fabriciofx]

@yegor256 @piotrkot @paulodamaso @skapral As you show us your point of view, let me show mine. I think there are two main points: what is an expected error and an unexpected error. What's is an expected error? It's something you know normal to fail, like validate a Social Security Number. Somebody can inform some wrong number, it's normal and happens all the time. Is expected to happens. But an unexpected error is something not normal to fail, like access a file into disk. The disk can fail, the file can has been moved, etc. It's not normal to fail.

Comparing with Functional Programing (FP), there're two kinds of functions: pure and not pure. Pure functions, if you pass the same input will returns always the same output. For example, pass two numbers and get the sum of them. The not pure functions, on the other hand, not always returns the same output. They're unsafe. Why? Because the output depends on others things, like the OS, the disk, the network interfaces... So, they can (or not) fail.

Thus, I see a method can throws an Exception as not pure function (unsafe) and I must take care when working with it. And how about what kind exception is throws? I think it's can be helpful, but the more important is detect the exception and show us what caused the exception (message).
Makes sense to you?

[skapral]

@piotrkot well, I just saw three options enumerated here and one unreasonable concern about my favorite one, that's why I interrupted.

I will make a separate issue, okay.

UPD: done - #920

[fabriciofx]

@yegor256 @piotrkot @paulodamaso @skapral

A plus: a James Gosling interview where the first question (3:18) is about checked exceptions.

[paulodamaso]

@yegor256 Do we have a final decision on this according to EO beliefs and your PO needs?

[piotrkot]

@paulodamaso I believe it's clearly stated here

OK, OK, you got me. I agree, let's do throws Exception.

[fabriciofx]

@paulodamaso @yegor256 @skapral just for record: I've changed my mind about checked exceptions. So I think the best solution is remove the checked exceptions and use only runtime exceptions.

[piotrkot]

@paulodamaso Can I help you? I can create a PR with code changed to "throws Exception".

[paulodamaso]

@yegor256 Do we have a final decision on this according to EO beliefs and your PO needs? We had some new arguments since your last veredict.

[piotrkot]

@yegor256 ping

[yegor256]

I still believe that we should throws Exception, even though I do hear what @skapral is saying. We simply can't predict all possible situations where some code may throw checked exceptions. Better let our users throw whatever they want and still be compatible with Takes.

[paulodamaso]

@piotrkot Please close

[0crat]

Job gh:yegor256/takes#872 is not assigned, can't get performer

[0crat]

This job is not in scope