reef-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Dudoladov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (REEF-864) Make exception handling consistent
Date Thu, 17 Dec 2015 14:59:46 GMT

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

Sergey Dudoladov commented on REEF-864:
---------------------------------------

The early  draft of exception handling policy in REEF:

h5. Preliminaries

* Bloch, 63 means "Item 63 in Effective Java";  Yuan et al means Sections 4.1 and 5 in the
[OSDI paper|https://www.usenix.org/conference/osdi14/technical-sessions/presentation/yuan]

* For custom checks, we can open a pull request at [sevntu.checkstyle | https://github.com/sevntu-checkstyle/sevntu.checkstyle]
and wait until someone implements them.

* Checks from  sevntu sometimes duplicate core checkstyle checks. In this list I preferred
the core checkstyle.

* Everything below is written forJava. Feel free to add C# issues.

h5. Throwing exceptions

* Each exception must have a non-empty message. The message must contain enough info to determine
why the exception occurred (examples in Bloch, 63). How to enforce:  code review. We can also
consider  a custom check that prohibits (a) using Exception no-args constructors  (b) passing
empty strings literals to Exception constructors.

* Exceptions thrown within try/catch  blocks  must preserve the original exception to keep
the stack trace. How to enforce: [AvoidHidingCauseExceptionCheck| https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/AvoidHidingCauseExceptionCheck.java].
The [{{getRootCause()}} | https://commons.apache.org/proper/commons-lang/javadocs/api-2.6/org/apache/commons/lang/exception/ExceptionUtils.html#getRootCause(java.lang.Throwable)]from
Apache Commons enables retrieving the initial exception.

* Log and (re)throw. Doing both at the same time is sometimes [considered an antipattern|
http://stackoverflow.com/a/6640029] ([relevant check|https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EitherLogOrThrowCheck.java
]). So we need to discuss if REEF really needs this. How to enforce: a custom check.

* Forbid throwing general exceptions: {{Error}}, {{RuntimeException}}, {{Throwable}}, {{Exception}}.
How to enforce: partially covered by [IllegalThrows |http://checkstyle.sourceforge.net/config_coding.html#IllegalThrows].


* Forbid non-final fields in user-defined exception classes. How to enforce: [MutableException|http://checkstyle.sourceforge.net/config_design.html#MutableException]

* Forbid throwing [anonymous exceptions | https://stackoverflow.com/questions/17334498/throw-anonymous-exceptions-in-java].
How to enforce: [ForbidThrowAnonymousExceptions | https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/ForbidThrowAnonymousExceptionsCheck.java]

h5.  Handling exceptions

The main idea is to prevent ignoring exceptions (Bloch, 65); some rules here impose stronger
conditions than "Effective Java" does .

* Forbid catching general exceptions: {{Error}}, {{RuntimeException}}, {{Throwable}}, {{Exception}}.
How to enforce: [IllegalCatch|http://checkstyle.sourceforge.net/config_coding.html#IllegalCatch].


* Forbid  empty catch blocks. How to enforce: [EmptyCatchBlock | http://checkstyle.sourceforge.net/config_blocks.html#EmptyCatchBlock
]. This check also enables [documenting expected exceptions | https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions].

* Forbid  catch blocks that only rethrow the original exception. How to enforce: [UselessSingleCatchCheck
|  https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/UselessSingleCatchCheck.java]

* Forbid  catch blocks that  contain only log statements. How to enforce: a custom check.
Source: Yuan et al.

* Forbid  catch blocks that  contain comments with tags {{TODO}}, {{FIXME}}, {{FIX}}, {{[XXX|http://www.oracle.com/technetwork/java/codeconventions-137265.html#395]}},
i.e. are knowingly incomplete.  How to enforce: a custom check. Source: Yuan et al.

* [Forbid  {{return}} in finally blocks| https://stackoverflow.com/questions/65035/does-finally-always-execute-in-java/65362#65362].
 How to enforce: [ForbidReturnInFinallyBlockCheck|https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/ForbidReturnInFinallyBlockCheck.java].

h5. Specific Exceptions

* {{NotImplementedException}}  and {{UnsupportedOperationException}} must (a) include a text
description of what prevents implementation  and (b) provide a {{TODO}} with the corresponding
JIRA issue. How to enforce: code review.

> Make exception handling consistent
> ----------------------------------
>
>                 Key: REEF-864
>                 URL: https://issues.apache.org/jira/browse/REEF-864
>             Project: REEF
>          Issue Type: Improvement
>          Components: Build infrastructure, Documentation
>            Reporter: Sergey Dudoladov
>            Assignee: Sergey Dudoladov
>
> The recent [OSDI paper | https://www.usenix.org/conference/osdi14/technical-sessions/presentation/yuan]
suggest that many failures in distributed systems arise from 3 trivial mistake in exception
handling:
> 1) the handler is empty
> 2) the handler silently aborts
> 3) the handler contains phrases like “TODO” and “FIXME”, i.e. is knowingly incomplete
> The authors propose [their own checker|https://github.com/diy1/aspirator] for these cases,
but we can try to enforce any of these rules via checkstyle, for example.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message