You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Mike Percy <mp...@cloudera.com> on 2012/03/01 21:13:14 UTC
Re: throws FlumeException
Hey Arvind & Will,
The thing is, you aren't really supposed to have to catch RuntimeExceptions. You generally assume disaster and propagate an abort up to the the level that it makes sense in your particular app. If you were supposed to catch them, you would declare them as checked exceptions. Which is why the language enforces that rule.
Regards,
Mike
On Feb 29, 2012, at 12:04 PM, Will McQueen wrote:
> Hi Arvind,
>
> Thanks everyonen for this great discussion. I have 2 comments:
>
> 1) If foo() calls bar(), and bar() throws a FlumeException that is just
> propagated up the stack by foo() to the client code, then the client
> calling foo() still won't know to deal with a FlumeException (eg, the
> client won't know whether it needs to wrap the call to foo() in the try{}
> block of try..finally, where finally{} does some clean-up and ensures
> consistent state of class invariants as the runtime exception gets
> propagated upwards)... right? I suppose this is the same scenario as any
> other unchecked exception, like if bar() threw an IllegalArgumentException
> then a client of foo() wouldn't know this by looking at foo's javadocs,
> since foo doesn't directly throw the IllegalArgumentException. I suppose
> that in the extreme case, if a client of foo() wanted to ensure that the
> client's own code is left in a consistent state after calling foo() (or
> after any other "alien method", as the JCIP book names it), then it seems
> that the client should basically assume that _any_ alien method called by
> the client can throw a runtime exception, and so the client code would
> always wrap all calls to alien methods in try..finally.
>
> 2) If we're including FlumeException in the throws clause, then does this
> convention also extend to including that same throws clause in all
> implementing and overriding methods as well?
>
> Cheers,
> Will
>
> Cheers,
> Will
>
> On Wed, Feb 29, 2012 at 10:16 AM, Arvind Prabhakar <ar...@cloudera.com>wrote:
>
>> Good discussion. My $0.02 inline below.
>>
>> On Wed, Feb 29, 2012 at 6:53 AM, Ralph Goers <rg...@apache.org> wrote:
>>
>>> I would agree. Your javadoc should list any exceptions that are
>> explicitly
>>> thrown, but runtime exceptions should never be declared as being thrown.
>>> The javadoc should also describe the circumstances of when they are
>> thrown
>>> and state that the are runtime exceptions. Remember that runtime
>>> exceptions are supposed to mean that there is no good way that the caller
>>> can deal with the problem, which is why no try/catch is required as the
>>> exception will be percolated up to a level where it makes sense to deal
>>> with these kinds of issues.
>>>
>>
>> The unfortunate reality is that Javadoc takes secondary precedence as
>> compared to functionality and testing coverage. While we do try to weed out
>> important exported APIs that need to be doc'd, it is never thorough enough.
>>
>> Given this unfortunate reality, I lean more towards tagging the runtime
>> exception in the throws clause in order to provide at least a visual intent
>> when Javadocs are generated about the potential that the method may raise
>> some exception.
>>
>> Regarding the practice of not including the unchecked exceptions in throws
>> clause, I feel it is dated and had value prior to the advent of IDEs and
>> tools that do static analysis. Moreover, I also feel that it is important
>> that the consumer understand the nature of exception they are forced to
>> catch, if that is the implied meaning of the throws clause here.
>>
>> Thanks,
>> Arvind Prabhakar
>>
>>
>>
>>>
>>> Ralph
>>>
>>> On Feb 29, 2012, at 1:06 AM, Mike Percy <mp...@cloudera.com> wrote:
>>>
>>>> Hi Arvind,
>>>> Thanks for helping me to understand the thinking. I think it makes
>>> expected usage less clear to users of the API because, unless you are
>>> familiar with the class structure and notice that the exception is a
>>> RuntimeException, you may assume you will need to catch it if it's there
>> in
>>> a throws clause. Since unchecked exceptions are not part of the contract
>> of
>>> the API and are not meant to be caught, but instead primarily exist to
>>> signal contract violations (which should be documented in the API docs),
>> it
>>> seems counterintuitive to me to explicitly list them.
>>>>
>>>> Regards,
>>>> Mike
>>>>
>>>> On Feb 29, 2012, at 12:50 AM, Arvind Prabhakar wrote:
>>>>
>>>>> When you add runtime exception(s) to the method declaration, it is
>>>>> captured in the class definition itself and tools like Eclipse will
>>>>> automatically generate the Javadoc stub for you.
>>>>>
>>>>> It is generally regarded as a good practice to declare all the
>>>>> exception the body of the method can potentially raise for
>>>>> documentation purposes. It is even more important to do this for
>>>>> public API that is meant for consumption outside of the
>>>>> product/framework boundaries.
>>>>>
>>>>> Thanks,
>>>>> Arvind
>>>>>
>>>>> On Wed, Feb 29, 2012 at 12:32 AM, Mike Percy <mp...@cloudera.com>
>>> wrote:
>>>>>> Hi Arvind,
>>>>>> It's helpful to specify it for documentation's sake in the Javadoc,
>>> i.e. @throws FlumeException, but a RuntimeException doesn't belong in a
>>> method declaration throws clause, right?
>>>>>>
>>>>>> ChannelFactory#create() is one example:
>>>>>>
>>>
>> https://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/ChannelFactory.java
>>>>>>
>>>>>> The Log4J appender is another place I am seeing that usage.
>>>>>>
>>>>>> Regards,
>>>>>> Mike
>>>>>>
>>>>>> On Feb 29, 2012, at 12:14 AM, Arvind Prabhakar wrote:
>>>>>>
>>>>>>> Hi Mike,
>>>>>>>
>>>>>>> The general practice of declaring every runtime exception causes the
>>>>>>> auto-generated Javadocs to have a mention in them. This is good when
>>>>>>> you want to specifically call out scenarios that will raise such
>>>>>>> exception. For example, System.setProperty(String,String) documents
>>>>>>> three exceptions, all of which are runtime exception types.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Arvind
>>>>>>>
>>>>>>> On Wed, Feb 29, 2012 at 12:07 AM, Mike Percy <mp...@cloudera.com>
>>> wrote:
>>>>>>>> Hi folks,
>>>>>>>> I see a few places in the code where APIs specify "throws
>>> FlumeException". However FlumeException extends RuntimeException and is
>>> therefore an unchecked exception. Wondering if there is any reason for
>> this
>>> or if it's just an oversight. Maybe what we really want is a checked
>>> version and an unchecked version.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Mike
>>>>>>>>
>>>>>>
>>>>
>>>
>>
Re: throws FlumeException
Posted by Ralph Goers <rg...@apache.org>.
Sent from my iPad
On Mar 3, 2012, at 1:36 AM, Arvind Prabhakar <ar...@cloudera.com> wrote:
> On Sat, Mar 3, 2012 at 12:17 AM, Ralph Goers <ra...@dslextreme.com>wrote:
>
>>
>> On Mar 2, 2012, at 10:58 PM, Arvind Prabhakar wrote:
>>
>>> Mike:
>>>
>>>> The thing is, you aren't really supposed to have to catch
>> RuntimeExceptions.
>>>
>>> I did not imply that. Neither does having a runtime exception declared
>>> on the throws clause requires that it be caught. That said, if the
>>> Javadocs did not mention NumberFormatException, there would have been
>>> a lot of pain and suffering for a lot of people. This is a good
>>> example of a runtime exception that one is forced to catch.
>>>
>>> Will:
>>>> I'd be very interested in hearing anyone's comments on these articles
>>>
>>> This debate has happened time and again and there is no clear cut
>>> right way of doing things. From my experience, the vast majority of
>>> exceptions you bake into a system you build (not a framework or
>>> library), are likely going to be relating to things that cannot be
>>> recovered from. So I lean towards using runtime exceptions a lot. This
>>> practice only works if you understand the scope of exception and
>>> associated failure (request/thread/server). It also requires having a
>>> common base type that can be caught at levels that do not have
>>> visibility to lower level implementation details.
>>
>> Once again, I disagree. Virtually all Java software engineers consider
>> Joshua Bloch's "Effective Java" to be the definitive guide on Java software
>> development. Item 62 in the second edition includes:
>>
>
> I respect your's and Josh's opinion. Effective Java is way up there in my
> opinion. Here is another relevant quote from the same book, from chapter 1
> - Introduction:
>
> While the rules in this book do not apply 100 percent of the time, they do
> characterize best programming practices in the great majority of cases. You
> should not slavishly follow these rules, but violate them only occasionally
> and with good reason. Learning the art of programming, like most other
> disciplines, consists of first learning the rules and then learning when to
> break them.
I don't disagree with this. I've simply not seen a good justification not to follow this particular rule other than personal preference.
>
>
>
>>
>> Use the Javadoc @throws tag to document each unchecked exception that a
>> method can throw, but do not use the throws keyword to include unchecked
>> exceptions in the method declaration. It is important that the programmers
>> using your API be aware of which exceptions are checked and which are
>> unchecked, as their responsibilities differ in these two cases. The
>> documentation generated by the Javadoc @throws tag in the absence of the
>> method header generated by the throws declaration provides a strong visual
>> cue to help the programmer distinguish checked exceptions from unchecked.
>>
>> Ralph
Re: throws FlumeException
Posted by Arvind Prabhakar <ar...@cloudera.com>.
On Sat, Mar 3, 2012 at 12:17 AM, Ralph Goers <ra...@dslextreme.com>wrote:
>
> On Mar 2, 2012, at 10:58 PM, Arvind Prabhakar wrote:
>
> > Mike:
> >
> >> The thing is, you aren't really supposed to have to catch
> RuntimeExceptions.
> >
> > I did not imply that. Neither does having a runtime exception declared
> > on the throws clause requires that it be caught. That said, if the
> > Javadocs did not mention NumberFormatException, there would have been
> > a lot of pain and suffering for a lot of people. This is a good
> > example of a runtime exception that one is forced to catch.
> >
> > Will:
> >> I'd be very interested in hearing anyone's comments on these articles
> >
> > This debate has happened time and again and there is no clear cut
> > right way of doing things. From my experience, the vast majority of
> > exceptions you bake into a system you build (not a framework or
> > library), are likely going to be relating to things that cannot be
> > recovered from. So I lean towards using runtime exceptions a lot. This
> > practice only works if you understand the scope of exception and
> > associated failure (request/thread/server). It also requires having a
> > common base type that can be caught at levels that do not have
> > visibility to lower level implementation details.
>
> Once again, I disagree. Virtually all Java software engineers consider
> Joshua Bloch's "Effective Java" to be the definitive guide on Java software
> development. Item 62 in the second edition includes:
>
I respect your's and Josh's opinion. Effective Java is way up there in my
opinion. Here is another relevant quote from the same book, from chapter 1
- Introduction:
While the rules in this book do not apply 100 percent of the time, they do
characterize best programming practices in the great majority of cases. You
should not slavishly follow these rules, but violate them only occasionally
and with good reason. Learning the art of programming, like most other
disciplines, consists of first learning the rules and then learning when to
break them.
>
> Use the Javadoc @throws tag to document each unchecked exception that a
> method can throw, but do not use the throws keyword to include unchecked
> exceptions in the method declaration. It is important that the programmers
> using your API be aware of which exceptions are checked and which are
> unchecked, as their responsibilities differ in these two cases. The
> documentation generated by the Javadoc @throws tag in the absence of the
> method header generated by the throws declaration provides a strong visual
> cue to help the programmer distinguish checked exceptions from unchecked.
>
> Ralph
Re: throws FlumeException
Posted by Ralph Goers <ra...@dslextreme.com>.
On Mar 2, 2012, at 10:58 PM, Arvind Prabhakar wrote:
> Mike:
>
>> The thing is, you aren't really supposed to have to catch RuntimeExceptions.
>
> I did not imply that. Neither does having a runtime exception declared
> on the throws clause requires that it be caught. That said, if the
> Javadocs did not mention NumberFormatException, there would have been
> a lot of pain and suffering for a lot of people. This is a good
> example of a runtime exception that one is forced to catch.
>
> Will:
>> I'd be very interested in hearing anyone's comments on these articles
>
> This debate has happened time and again and there is no clear cut
> right way of doing things. From my experience, the vast majority of
> exceptions you bake into a system you build (not a framework or
> library), are likely going to be relating to things that cannot be
> recovered from. So I lean towards using runtime exceptions a lot. This
> practice only works if you understand the scope of exception and
> associated failure (request/thread/server). It also requires having a
> common base type that can be caught at levels that do not have
> visibility to lower level implementation details.
Once again, I disagree. Virtually all Java software engineers consider Joshua Bloch's "Effective Java" to be the definitive guide on Java software development. Item 62 in the second edition includes:
Use the Javadoc @throws tag to document each unchecked exception that a method can throw, but do not use the throws keyword to include unchecked exceptions in the method declaration. It is important that the programmers using your API be aware of which exceptions are checked and which are unchecked, as their responsibilities differ in these two cases. The documentation generated by the Javadoc @throws tag in the absence of the method header generated by the throws declaration provides a strong visual cue to help the programmer distinguish checked exceptions from unchecked.
Ralph
Re: throws FlumeException
Posted by Arvind Prabhakar <ar...@apache.org>.
Mike:
> The thing is, you aren't really supposed to have to catch RuntimeExceptions.
I did not imply that. Neither does having a runtime exception declared
on the throws clause requires that it be caught. That said, if the
Javadocs did not mention NumberFormatException, there would have been
a lot of pain and suffering for a lot of people. This is a good
example of a runtime exception that one is forced to catch.
Will:
> I'd be very interested in hearing anyone's comments on these articles
This debate has happened time and again and there is no clear cut
right way of doing things. From my experience, the vast majority of
exceptions you bake into a system you build (not a framework or
library), are likely going to be relating to things that cannot be
recovered from. So I lean towards using runtime exceptions a lot. This
practice only works if you understand the scope of exception and
associated failure (request/thread/server). It also requires having a
common base type that can be caught at levels that do not have
visibility to lower level implementation details.
Thanks,
Arvind
On Thu, Mar 1, 2012 at 3:30 PM, Will McQueen <wi...@cloudera.com> wrote:
> Hi Mike and Arvind,
>
>>> you aren't really supposed to have to catch RuntimeExceptions
> I suppose it depends on your chosen style. As reference, here are 2 very
> good links regarding checked vs unchecked exceptions:
> http://www.artima.com/intv/handcuffs.html
> http://www.mindview.net/Etc/Discussions/CheckedExceptions
>
> I'd be very interested in hearing anyone's comments on these articles, esp
> your thoughts on catching unchecked higher-level (flume abstraction level)
> exceptions like FlumeException.
>
> Cheers,
> Will
>
> On Thu, Mar 1, 2012 at 12:13 PM, Mike Percy <mp...@cloudera.com> wrote:
>
>> Hey Arvind & Will,
>> The thing is, you aren't really supposed to have to catch
>> RuntimeExceptions. You generally assume disaster and propagate an abort up
>> to the the level that it makes sense in your particular app. If you were
>> supposed to catch them, you would declare them as checked exceptions. Which
>> is why the language enforces that rule.
>>
>> Regards,
>> Mike
>>
>> On Feb 29, 2012, at 12:04 PM, Will McQueen wrote:
>>
>> > Hi Arvind,
>> >
>> > Thanks everyonen for this great discussion. I have 2 comments:
>> >
>> > 1) If foo() calls bar(), and bar() throws a FlumeException that is just
>> > propagated up the stack by foo() to the client code, then the client
>> > calling foo() still won't know to deal with a FlumeException (eg, the
>> > client won't know whether it needs to wrap the call to foo() in the try{}
>> > block of try..finally, where finally{} does some clean-up and ensures
>> > consistent state of class invariants as the runtime exception gets
>> > propagated upwards)... right? I suppose this is the same scenario as any
>> > other unchecked exception, like if bar() threw an
>> IllegalArgumentException
>> > then a client of foo() wouldn't know this by looking at foo's javadocs,
>> > since foo doesn't directly throw the IllegalArgumentException. I suppose
>> > that in the extreme case, if a client of foo() wanted to ensure that the
>> > client's own code is left in a consistent state after calling foo() (or
>> > after any other "alien method", as the JCIP book names it), then it seems
>> > that the client should basically assume that _any_ alien method called by
>> > the client can throw a runtime exception, and so the client code would
>> > always wrap all calls to alien methods in try..finally.
>> >
>> > 2) If we're including FlumeException in the throws clause, then does this
>> > convention also extend to including that same throws clause in all
>> > implementing and overriding methods as well?
>> >
>> > Cheers,
>> > Will
>> >
>> > Cheers,
>> > Will
>> >
>> > On Wed, Feb 29, 2012 at 10:16 AM, Arvind Prabhakar <arvind@cloudera.com
>> >wrote:
>> >
>> >> Good discussion. My $0.02 inline below.
>> >>
>> >> On Wed, Feb 29, 2012 at 6:53 AM, Ralph Goers <rg...@apache.org> wrote:
>> >>
>> >>> I would agree. Your javadoc should list any exceptions that are
>> >> explicitly
>> >>> thrown, but runtime exceptions should never be declared as being
>> thrown.
>> >>> The javadoc should also describe the circumstances of when they are
>> >> thrown
>> >>> and state that the are runtime exceptions. Remember that runtime
>> >>> exceptions are supposed to mean that there is no good way that the
>> caller
>> >>> can deal with the problem, which is why no try/catch is required as the
>> >>> exception will be percolated up to a level where it makes sense to deal
>> >>> with these kinds of issues.
>> >>>
>> >>
>> >> The unfortunate reality is that Javadoc takes secondary precedence as
>> >> compared to functionality and testing coverage. While we do try to weed
>> out
>> >> important exported APIs that need to be doc'd, it is never thorough
>> enough.
>> >>
>> >> Given this unfortunate reality, I lean more towards tagging the runtime
>> >> exception in the throws clause in order to provide at least a visual
>> intent
>> >> when Javadocs are generated about the potential that the method may
>> raise
>> >> some exception.
>> >>
>> >> Regarding the practice of not including the unchecked exceptions in
>> throws
>> >> clause, I feel it is dated and had value prior to the advent of IDEs and
>> >> tools that do static analysis. Moreover, I also feel that it is
>> important
>> >> that the consumer understand the nature of exception they are forced to
>> >> catch, if that is the implied meaning of the throws clause here.
>> >>
>> >> Thanks,
>> >> Arvind Prabhakar
>> >>
>> >>
>> >>
>> >>>
>> >>> Ralph
>> >>>
>> >>> On Feb 29, 2012, at 1:06 AM, Mike Percy <mp...@cloudera.com> wrote:
>> >>>
>> >>>> Hi Arvind,
>> >>>> Thanks for helping me to understand the thinking. I think it makes
>> >>> expected usage less clear to users of the API because, unless you are
>> >>> familiar with the class structure and notice that the exception is a
>> >>> RuntimeException, you may assume you will need to catch it if it's
>> there
>> >> in
>> >>> a throws clause. Since unchecked exceptions are not part of the
>> contract
>> >> of
>> >>> the API and are not meant to be caught, but instead primarily exist to
>> >>> signal contract violations (which should be documented in the API
>> docs),
>> >> it
>> >>> seems counterintuitive to me to explicitly list them.
>> >>>>
>> >>>> Regards,
>> >>>> Mike
>> >>>>
>> >>>> On Feb 29, 2012, at 12:50 AM, Arvind Prabhakar wrote:
>> >>>>
>> >>>>> When you add runtime exception(s) to the method declaration, it is
>> >>>>> captured in the class definition itself and tools like Eclipse will
>> >>>>> automatically generate the Javadoc stub for you.
>> >>>>>
>> >>>>> It is generally regarded as a good practice to declare all the
>> >>>>> exception the body of the method can potentially raise for
>> >>>>> documentation purposes. It is even more important to do this for
>> >>>>> public API that is meant for consumption outside of the
>> >>>>> product/framework boundaries.
>> >>>>>
>> >>>>> Thanks,
>> >>>>> Arvind
>> >>>>>
>> >>>>> On Wed, Feb 29, 2012 at 12:32 AM, Mike Percy <mp...@cloudera.com>
>> >>> wrote:
>> >>>>>> Hi Arvind,
>> >>>>>> It's helpful to specify it for documentation's sake in the Javadoc,
>> >>> i.e. @throws FlumeException, but a RuntimeException doesn't belong in a
>> >>> method declaration throws clause, right?
>> >>>>>>
>> >>>>>> ChannelFactory#create() is one example:
>> >>>>>>
>> >>>
>> >>
>> https://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/ChannelFactory.java
>> >>>>>>
>> >>>>>> The Log4J appender is another place I am seeing that usage.
>> >>>>>>
>> >>>>>> Regards,
>> >>>>>> Mike
>> >>>>>>
>> >>>>>> On Feb 29, 2012, at 12:14 AM, Arvind Prabhakar wrote:
>> >>>>>>
>> >>>>>>> Hi Mike,
>> >>>>>>>
>> >>>>>>> The general practice of declaring every runtime exception causes
>> the
>> >>>>>>> auto-generated Javadocs to have a mention in them. This is good
>> when
>> >>>>>>> you want to specifically call out scenarios that will raise such
>> >>>>>>> exception. For example, System.setProperty(String,String) documents
>> >>>>>>> three exceptions, all of which are runtime exception types.
>> >>>>>>>
>> >>>>>>> Thanks,
>> >>>>>>> Arvind
>> >>>>>>>
>> >>>>>>> On Wed, Feb 29, 2012 at 12:07 AM, Mike Percy <mp...@cloudera.com>
>> >>> wrote:
>> >>>>>>>> Hi folks,
>> >>>>>>>> I see a few places in the code where APIs specify "throws
>> >>> FlumeException". However FlumeException extends RuntimeException and is
>> >>> therefore an unchecked exception. Wondering if there is any reason for
>> >> this
>> >>> or if it's just an oversight. Maybe what we really want is a checked
>> >>> version and an unchecked version.
>> >>>>>>>>
>> >>>>>>>> Regards,
>> >>>>>>>> Mike
>> >>>>>>>>
>> >>>>>>
>> >>>>
>> >>>
>> >>
>>
>>
Re: throws FlumeException
Posted by Mike Percy <mp...@cloudera.com>.
Will, thank you for sending these articles. I think Bruce Eckel and Anders Hejlsberg make very good points about the inherent problems of checked exceptions. So, maybe they are inherently flawed.
I can see that if you are going to make a design decision to avoid checked exceptions altogether, and only throw RuntimeExceptions, then you would want to document that in your throws clauses. It seems like kind of nonstandard Java to me, but if you are explicit and consistent about it then that nonstandard approach doesn't impair API usability too much in my opinion, and seems like it could prevent users from ignoring those errors in their applications with empty catch clauses, which is the main point made by Bruce. After some pondering, that does hit home for me somewhat. I think the versioning argument made by Anders has merit but I also have serious reservations on that, since you are basically adding to your API's error handling contract with the client in a silent way. That practice seems harmful if done within supposedly "safe" version differences, i.e. patch releases, since it can cause spurious exceptions to go unhandled, possibly crashing the application, after a seemingly benign package update.
If you are going to go this nonstandard route however, I think it is very confusing to be inconsistent, providing both checked and unchecked exceptions. This is what I am seeing in the latest Flume 1.x source tree:
./flume-ng-channels/flume-jdbc-channel/src/main/java/org/apache/flume/channel/jdbc/JdbcChannelException.java:public class JdbcChannelException extends ChannelException {
./flume-ng-core/src/main/java/org/apache/flume/ChannelException.java:public class ChannelException extends RuntimeException {
./flume-ng-core/src/main/java/org/apache/flume/EventDeliveryException.java:public class EventDeliveryException extends Exception {
./flume-ng-core/src/main/java/org/apache/flume/FlumeException.java:public class FlumeException extends RuntimeException {
./flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleException.java:public class LifecycleException extends Exception {
So 3 are unchecked and 2 are checked. I wouldn't be against making all of these unchecked, as long as we ban checked exceptions. Otherwise, I feel that we should go the other route, and have a policy against throwing unchecked exceptions that we expect the caller to be handling in anything but a high-level catch-all handler.
Another thing worth discussing on this list, that surely deserves its own thread, is how much we should feel free to break during a minor version upgrade (i.e. 1.0.x vs 1.1.0). I feel we should be able to break binary compatibility in minor releases if we document which classes changed, as long as we make (documented, advertised) promises that certain interfaces intended for user level integration will only change in a major (1.x.x vs 2.0.0) release. So we would specify which interfaces are intended to be internal to the Flume application, and which interfaces are intended to be external "framework" interfaces. If there is a lot of disagreement with this then I think much of this discussion becomes academic, unless we want to go quickly from a 1.x to a 2.x release.
Regards,
Mike
On Mar 1, 2012, at 3:30 PM, Will McQueen wrote:
> Hi Mike and Arvind,
>
>>> you aren't really supposed to have to catch RuntimeExceptions
> I suppose it depends on your chosen style. As reference, here are 2 very
> good links regarding checked vs unchecked exceptions:
> http://www.artima.com/intv/handcuffs.html
> http://www.mindview.net/Etc/Discussions/CheckedExceptions
>
> I'd be very interested in hearing anyone's comments on these articles, esp
> your thoughts on catching unchecked higher-level (flume abstraction level)
> exceptions like FlumeException.
>
> Cheers,
> Will
>
> On Thu, Mar 1, 2012 at 12:13 PM, Mike Percy <mp...@cloudera.com> wrote:
>
>> Hey Arvind & Will,
>> The thing is, you aren't really supposed to have to catch
>> RuntimeExceptions. You generally assume disaster and propagate an abort up
>> to the the level that it makes sense in your particular app. If you were
>> supposed to catch them, you would declare them as checked exceptions. Which
>> is why the language enforces that rule.
>>
>> Regards,
>> Mike
>>
>> On Feb 29, 2012, at 12:04 PM, Will McQueen wrote:
>>
>>> Hi Arvind,
>>>
>>> Thanks everyonen for this great discussion. I have 2 comments:
>>>
>>> 1) If foo() calls bar(), and bar() throws a FlumeException that is just
>>> propagated up the stack by foo() to the client code, then the client
>>> calling foo() still won't know to deal with a FlumeException (eg, the
>>> client won't know whether it needs to wrap the call to foo() in the try{}
>>> block of try..finally, where finally{} does some clean-up and ensures
>>> consistent state of class invariants as the runtime exception gets
>>> propagated upwards)... right? I suppose this is the same scenario as any
>>> other unchecked exception, like if bar() threw an
>> IllegalArgumentException
>>> then a client of foo() wouldn't know this by looking at foo's javadocs,
>>> since foo doesn't directly throw the IllegalArgumentException. I suppose
>>> that in the extreme case, if a client of foo() wanted to ensure that the
>>> client's own code is left in a consistent state after calling foo() (or
>>> after any other "alien method", as the JCIP book names it), then it seems
>>> that the client should basically assume that _any_ alien method called by
>>> the client can throw a runtime exception, and so the client code would
>>> always wrap all calls to alien methods in try..finally.
>>>
>>> 2) If we're including FlumeException in the throws clause, then does this
>>> convention also extend to including that same throws clause in all
>>> implementing and overriding methods as well?
>>>
>>> Cheers,
>>> Will
>>>
>>> Cheers,
>>> Will
>>>
>>> On Wed, Feb 29, 2012 at 10:16 AM, Arvind Prabhakar <arvind@cloudera.com
>>> wrote:
>>>
>>>> Good discussion. My $0.02 inline below.
>>>>
>>>> On Wed, Feb 29, 2012 at 6:53 AM, Ralph Goers <rg...@apache.org> wrote:
>>>>
>>>>> I would agree. Your javadoc should list any exceptions that are
>>>> explicitly
>>>>> thrown, but runtime exceptions should never be declared as being
>> thrown.
>>>>> The javadoc should also describe the circumstances of when they are
>>>> thrown
>>>>> and state that the are runtime exceptions. Remember that runtime
>>>>> exceptions are supposed to mean that there is no good way that the
>> caller
>>>>> can deal with the problem, which is why no try/catch is required as the
>>>>> exception will be percolated up to a level where it makes sense to deal
>>>>> with these kinds of issues.
>>>>>
>>>>
>>>> The unfortunate reality is that Javadoc takes secondary precedence as
>>>> compared to functionality and testing coverage. While we do try to weed
>> out
>>>> important exported APIs that need to be doc'd, it is never thorough
>> enough.
>>>>
>>>> Given this unfortunate reality, I lean more towards tagging the runtime
>>>> exception in the throws clause in order to provide at least a visual
>> intent
>>>> when Javadocs are generated about the potential that the method may
>> raise
>>>> some exception.
>>>>
>>>> Regarding the practice of not including the unchecked exceptions in
>> throws
>>>> clause, I feel it is dated and had value prior to the advent of IDEs and
>>>> tools that do static analysis. Moreover, I also feel that it is
>> important
>>>> that the consumer understand the nature of exception they are forced to
>>>> catch, if that is the implied meaning of the throws clause here.
>>>>
>>>> Thanks,
>>>> Arvind Prabhakar
>>>>
>>>>
>>>>
>>>>>
>>>>> Ralph
>>>>>
>>>>> On Feb 29, 2012, at 1:06 AM, Mike Percy <mp...@cloudera.com> wrote:
>>>>>
>>>>>> Hi Arvind,
>>>>>> Thanks for helping me to understand the thinking. I think it makes
>>>>> expected usage less clear to users of the API because, unless you are
>>>>> familiar with the class structure and notice that the exception is a
>>>>> RuntimeException, you may assume you will need to catch it if it's
>> there
>>>> in
>>>>> a throws clause. Since unchecked exceptions are not part of the
>> contract
>>>> of
>>>>> the API and are not meant to be caught, but instead primarily exist to
>>>>> signal contract violations (which should be documented in the API
>> docs),
>>>> it
>>>>> seems counterintuitive to me to explicitly list them.
>>>>>>
>>>>>> Regards,
>>>>>> Mike
>>>>>>
>>>>>> On Feb 29, 2012, at 12:50 AM, Arvind Prabhakar wrote:
>>>>>>
>>>>>>> When you add runtime exception(s) to the method declaration, it is
>>>>>>> captured in the class definition itself and tools like Eclipse will
>>>>>>> automatically generate the Javadoc stub for you.
>>>>>>>
>>>>>>> It is generally regarded as a good practice to declare all the
>>>>>>> exception the body of the method can potentially raise for
>>>>>>> documentation purposes. It is even more important to do this for
>>>>>>> public API that is meant for consumption outside of the
>>>>>>> product/framework boundaries.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Arvind
>>>>>>>
>>>>>>> On Wed, Feb 29, 2012 at 12:32 AM, Mike Percy <mp...@cloudera.com>
>>>>> wrote:
>>>>>>>> Hi Arvind,
>>>>>>>> It's helpful to specify it for documentation's sake in the Javadoc,
>>>>> i.e. @throws FlumeException, but a RuntimeException doesn't belong in a
>>>>> method declaration throws clause, right?
>>>>>>>>
>>>>>>>> ChannelFactory#create() is one example:
>>>>>>>>
>>>>>
>>>>
>> https://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/ChannelFactory.java
>>>>>>>>
>>>>>>>> The Log4J appender is another place I am seeing that usage.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Mike
>>>>>>>>
>>>>>>>> On Feb 29, 2012, at 12:14 AM, Arvind Prabhakar wrote:
>>>>>>>>
>>>>>>>>> Hi Mike,
>>>>>>>>>
>>>>>>>>> The general practice of declaring every runtime exception causes
>> the
>>>>>>>>> auto-generated Javadocs to have a mention in them. This is good
>> when
>>>>>>>>> you want to specifically call out scenarios that will raise such
>>>>>>>>> exception. For example, System.setProperty(String,String) documents
>>>>>>>>> three exceptions, all of which are runtime exception types.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Arvind
>>>>>>>>>
>>>>>>>>> On Wed, Feb 29, 2012 at 12:07 AM, Mike Percy <mp...@cloudera.com>
>>>>> wrote:
>>>>>>>>>> Hi folks,
>>>>>>>>>> I see a few places in the code where APIs specify "throws
>>>>> FlumeException". However FlumeException extends RuntimeException and is
>>>>> therefore an unchecked exception. Wondering if there is any reason for
>>>> this
>>>>> or if it's just an oversight. Maybe what we really want is a checked
>>>>> version and an unchecked version.
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Mike
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
Re: throws FlumeException
Posted by Will McQueen <wi...@cloudera.com>.
Hi Mike and Arvind,
>> you aren't really supposed to have to catch RuntimeExceptions
I suppose it depends on your chosen style. As reference, here are 2 very
good links regarding checked vs unchecked exceptions:
http://www.artima.com/intv/handcuffs.html
http://www.mindview.net/Etc/Discussions/CheckedExceptions
I'd be very interested in hearing anyone's comments on these articles, esp
your thoughts on catching unchecked higher-level (flume abstraction level)
exceptions like FlumeException.
Cheers,
Will
On Thu, Mar 1, 2012 at 12:13 PM, Mike Percy <mp...@cloudera.com> wrote:
> Hey Arvind & Will,
> The thing is, you aren't really supposed to have to catch
> RuntimeExceptions. You generally assume disaster and propagate an abort up
> to the the level that it makes sense in your particular app. If you were
> supposed to catch them, you would declare them as checked exceptions. Which
> is why the language enforces that rule.
>
> Regards,
> Mike
>
> On Feb 29, 2012, at 12:04 PM, Will McQueen wrote:
>
> > Hi Arvind,
> >
> > Thanks everyonen for this great discussion. I have 2 comments:
> >
> > 1) If foo() calls bar(), and bar() throws a FlumeException that is just
> > propagated up the stack by foo() to the client code, then the client
> > calling foo() still won't know to deal with a FlumeException (eg, the
> > client won't know whether it needs to wrap the call to foo() in the try{}
> > block of try..finally, where finally{} does some clean-up and ensures
> > consistent state of class invariants as the runtime exception gets
> > propagated upwards)... right? I suppose this is the same scenario as any
> > other unchecked exception, like if bar() threw an
> IllegalArgumentException
> > then a client of foo() wouldn't know this by looking at foo's javadocs,
> > since foo doesn't directly throw the IllegalArgumentException. I suppose
> > that in the extreme case, if a client of foo() wanted to ensure that the
> > client's own code is left in a consistent state after calling foo() (or
> > after any other "alien method", as the JCIP book names it), then it seems
> > that the client should basically assume that _any_ alien method called by
> > the client can throw a runtime exception, and so the client code would
> > always wrap all calls to alien methods in try..finally.
> >
> > 2) If we're including FlumeException in the throws clause, then does this
> > convention also extend to including that same throws clause in all
> > implementing and overriding methods as well?
> >
> > Cheers,
> > Will
> >
> > Cheers,
> > Will
> >
> > On Wed, Feb 29, 2012 at 10:16 AM, Arvind Prabhakar <arvind@cloudera.com
> >wrote:
> >
> >> Good discussion. My $0.02 inline below.
> >>
> >> On Wed, Feb 29, 2012 at 6:53 AM, Ralph Goers <rg...@apache.org> wrote:
> >>
> >>> I would agree. Your javadoc should list any exceptions that are
> >> explicitly
> >>> thrown, but runtime exceptions should never be declared as being
> thrown.
> >>> The javadoc should also describe the circumstances of when they are
> >> thrown
> >>> and state that the are runtime exceptions. Remember that runtime
> >>> exceptions are supposed to mean that there is no good way that the
> caller
> >>> can deal with the problem, which is why no try/catch is required as the
> >>> exception will be percolated up to a level where it makes sense to deal
> >>> with these kinds of issues.
> >>>
> >>
> >> The unfortunate reality is that Javadoc takes secondary precedence as
> >> compared to functionality and testing coverage. While we do try to weed
> out
> >> important exported APIs that need to be doc'd, it is never thorough
> enough.
> >>
> >> Given this unfortunate reality, I lean more towards tagging the runtime
> >> exception in the throws clause in order to provide at least a visual
> intent
> >> when Javadocs are generated about the potential that the method may
> raise
> >> some exception.
> >>
> >> Regarding the practice of not including the unchecked exceptions in
> throws
> >> clause, I feel it is dated and had value prior to the advent of IDEs and
> >> tools that do static analysis. Moreover, I also feel that it is
> important
> >> that the consumer understand the nature of exception they are forced to
> >> catch, if that is the implied meaning of the throws clause here.
> >>
> >> Thanks,
> >> Arvind Prabhakar
> >>
> >>
> >>
> >>>
> >>> Ralph
> >>>
> >>> On Feb 29, 2012, at 1:06 AM, Mike Percy <mp...@cloudera.com> wrote:
> >>>
> >>>> Hi Arvind,
> >>>> Thanks for helping me to understand the thinking. I think it makes
> >>> expected usage less clear to users of the API because, unless you are
> >>> familiar with the class structure and notice that the exception is a
> >>> RuntimeException, you may assume you will need to catch it if it's
> there
> >> in
> >>> a throws clause. Since unchecked exceptions are not part of the
> contract
> >> of
> >>> the API and are not meant to be caught, but instead primarily exist to
> >>> signal contract violations (which should be documented in the API
> docs),
> >> it
> >>> seems counterintuitive to me to explicitly list them.
> >>>>
> >>>> Regards,
> >>>> Mike
> >>>>
> >>>> On Feb 29, 2012, at 12:50 AM, Arvind Prabhakar wrote:
> >>>>
> >>>>> When you add runtime exception(s) to the method declaration, it is
> >>>>> captured in the class definition itself and tools like Eclipse will
> >>>>> automatically generate the Javadoc stub for you.
> >>>>>
> >>>>> It is generally regarded as a good practice to declare all the
> >>>>> exception the body of the method can potentially raise for
> >>>>> documentation purposes. It is even more important to do this for
> >>>>> public API that is meant for consumption outside of the
> >>>>> product/framework boundaries.
> >>>>>
> >>>>> Thanks,
> >>>>> Arvind
> >>>>>
> >>>>> On Wed, Feb 29, 2012 at 12:32 AM, Mike Percy <mp...@cloudera.com>
> >>> wrote:
> >>>>>> Hi Arvind,
> >>>>>> It's helpful to specify it for documentation's sake in the Javadoc,
> >>> i.e. @throws FlumeException, but a RuntimeException doesn't belong in a
> >>> method declaration throws clause, right?
> >>>>>>
> >>>>>> ChannelFactory#create() is one example:
> >>>>>>
> >>>
> >>
> https://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/ChannelFactory.java
> >>>>>>
> >>>>>> The Log4J appender is another place I am seeing that usage.
> >>>>>>
> >>>>>> Regards,
> >>>>>> Mike
> >>>>>>
> >>>>>> On Feb 29, 2012, at 12:14 AM, Arvind Prabhakar wrote:
> >>>>>>
> >>>>>>> Hi Mike,
> >>>>>>>
> >>>>>>> The general practice of declaring every runtime exception causes
> the
> >>>>>>> auto-generated Javadocs to have a mention in them. This is good
> when
> >>>>>>> you want to specifically call out scenarios that will raise such
> >>>>>>> exception. For example, System.setProperty(String,String) documents
> >>>>>>> three exceptions, all of which are runtime exception types.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Arvind
> >>>>>>>
> >>>>>>> On Wed, Feb 29, 2012 at 12:07 AM, Mike Percy <mp...@cloudera.com>
> >>> wrote:
> >>>>>>>> Hi folks,
> >>>>>>>> I see a few places in the code where APIs specify "throws
> >>> FlumeException". However FlumeException extends RuntimeException and is
> >>> therefore an unchecked exception. Wondering if there is any reason for
> >> this
> >>> or if it's just an oversight. Maybe what we really want is a checked
> >>> version and an unchecked version.
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>> Mike
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> >>
>
>