[jira] [Comment Edited] (FLINK-19384) Source API exception signatures are inconsistent

["Arvid Heise (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-19384?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17213648#comment-17213648 ] 

Arvid Heise edited comment on FLINK-19384 at 10/14/20, 6:16 AM:
----------------------------------------------------------------

Please don't go the {{Exception}} route: it's impossible to react appropriately except by resorting to Pokemon exception handling (gonna catch them all). Either list all the possible checked exceptions explicitly or go with runtime exceptions.

Also read Joshua Bloch's take of exception handling in [Effective Java|https://ahdak.github.io/blog/effective-java-part-9/].

{noformat}
Item 70: "Use checked exceptions for conditions from which the caller can reasonably be expected to recover. You force the caller to handle the exception in a catch block or to propagate it."
{noformat}
I'd argue it's impossible to recover in a reasonable way if you catch all {{Exception}}s, since that also includes all {{RuntimeException}}s, which are by this definition not recoverable.

{noformat}
Item 71: "Ask yourself how the programmer will handle the exception ? If the programmer can’t do better than e.printStackTrace() or throw new AssertionError(), then, an unchecked exception is called for."
Without any way to detect which exception is thrown, you are pretty much forced to just propagate it.

{noformat}
Item 72: "Do not reuse Exception, RuntimeException , Throwable or Error directly. Treat these classes as if they were abstract."
{noformat}
This is more about not throwing an {{Exception}}, but at the same time it also means that if your API simply declares {{throws Exception}}, it is well possible in the API to use it in this way.


was (Author: aheise):
Please don't go the {{Exception}} route: it's impossible to react appropriately accept resorting to Pokemon exception handling (gonna catch them all). Either list all the possible checked exceptions explicitly or go with runtime exceptions.

Also read Joshua Bloch's take of exception handling in [Effective Java|https://ahdak.github.io/blog/effective-java-part-9/].

{noformat}
Item 70: "Use checked exceptions for conditions from which the caller can reasonably be expected to recover. You force the caller to handle the exception in a catch block or to propagate it."
{noformat}
I'd argue it's impossible to recover in a reasonable way if you catch all {{Exception}}s, since that also includes all {{RuntimeException}}s, which are by this definition not recoverable.

{noformat}
Item 71: "Ask yourself how the programmer will handle the exception ? If the programmer can’t do better than e.printStackTrace() or throw new AssertionError(), then, an unchecked exception is called for."
Without any way to detect which exception is thrown, you are pretty much forced to just propagate it.

{noformat}
Item 72: "Do not reuse Exception, RuntimeException , Throwable or Error directly. Treat these classes as if they were abstract."
{noformat}
This is more about not throwing an {{Exception}}, but at the same time it also means that if your API simply declares {{throws Exception}}, it is well possible in the API to use it in this way.

> Source API exception signatures are inconsistent
> ------------------------------------------------
>
>                 Key: FLINK-19384
>                 URL: https://issues.apache.org/jira/browse/FLINK-19384
>             Project: Flink
>          Issue Type: Bug
>          Components: API / Core
>    Affects Versions: 1.11.0, 1.11.1, 1.11.2
>            Reporter: Stephan Ewen
>            Priority: Blocker
>             Fix For: 1.12.0
>
>
> The methods in {{org.apache.flink.api.connector.source.Source}} have inconsistent exception signatures:
>   - the methods to create reader and enumerator do not throw a checked exception
>   - the method to restore an enumerator throws an {{IOException}}.
> Either all methods should allow throwing checked IOExceptions or all methods should not allo any checked exceptions.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)