You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Zheng, Kai" <ka...@intel.com> on 2015/12/29 04:46:24 UTC

Question about subtype exceptions in the thrown list in addition to a more general one

Hi,

Would it be good to add to throw a subtype exception in addition to a more general exception already there in the thrown list? Is it some coding style that's required to follow or developers can do it as they like?

It's often seen that only the general exception is in the thrown list, like in ReadableByteChannel#read in Oracle Java, where in fact the method may throw 4 subtype exceptions.
http://docs.oracle.com/javase/7/docs/api/java/nio/channels/ReadableByteChannel.html
int read(ByteBuffer dst) throws IOException

In some of Hadoop codes, it goes otherwise. For example, in Hdfs.java, ref. the following codes, note that in the thrown list, all the former 3 exceptions extends IOException.
===
  @Override
  public RemoteIterator<FileStatus> listStatusIterator(final Path f)
    throws AccessControlException, FileNotFoundException,
    UnresolvedLinkException, IOException {
    return new DirListingIterator<FileStatus>(f, false) {

      @Override
      public FileStatus next() throws IOException {
        return getNext().makeQualified(getUri(), f);
      }
    };
  }
===

Doing this way, I thought the benefit could be the caller can see clearly what kinds of exceptions could be thrown, however in this case we can achieve it by adding the Javadoc. Sure there is no hurt but it looks kinds of dummy in some IDE, hinting some information like "There is a more general exception ... in the thrown list already". Given we would list all the possible exceptions, it's a little hard to maintain the codes.

By the way, in the above example, it looks a little weird that AccessControlException and UnresolvedLinkException both extend IOException. There is a little reason to do that for the latter, but for the former, it looks rather like an issue.

Please help clarify if I missed something. Thanks.

Regards,
Kai

RE: Question about subtype exceptions in the thrown list in addition to a more general one

Posted by "Zheng, Kai" <ka...@intel.com>.
Thanks Chris for the thoughts and details. I agree it's not easy to change those IOException subclasses even not relevant to I/O. 

Regards,
Kai

-----Original Message-----
From: Chris Nauroth [mailto:cnauroth@hortonworks.com] 
Sent: Wednesday, December 30, 2015 2:52 AM
To: common-dev@hadoop.apache.org
Subject: Re: Question about subtype exceptions in the thrown list in addition to a more general one

Hello Kai,

I'm not aware of a specific coding standard we have on this topic, and I don't have a strong opinion either way.  I think it can be valuable sometimes for public APIs to document each subclass in a separate @throws in the JavaDocs if we expect the caller might want to handle each case differently.  As you said, this is less relevant for the actual throws clause in the code.

Regarding IOException, there is a great tendency in the Hadoop codebase to subclass IOException, even if the failure is not obviously related to I/O.
 This is partly a consequence of the way error handling is implemented in the RPC framework.  The RemoteException class is tightly coupled to IOException for its "unwrap" logic to pull out the root cause of the exception.  As a result, any exception that needs to cross a process boundary over RPC generally ends up needing to subclass IOException.  I don't think this is something that can be changed easily.

--Chris Nauroth




On 12/28/15, 7:46 PM, "Zheng, Kai" <ka...@intel.com> wrote:

>
>Hi,
>
>Would it be good to add to throw a subtype exception in addition to a 
>more general exception already there in the thrown list? Is it some 
>coding style that's required to follow or developers can do it as they 
>like?
>
>It's often seen that only the general exception is in the thrown list, 
>like in ReadableByteChannel#read in Oracle Java, where in fact the 
>method may throw 4 subtype exceptions.
>http://docs.oracle.com/javase/7/docs/api/java/nio/channels/ReadableByte
>Cha
>nnel.html
>int read(ByteBuffer dst) throws IOException
>
>In some of Hadoop codes, it goes otherwise. For example, in Hdfs.java, 
>ref. the following codes, note that in the thrown list, all the former 
>3 exceptions extends IOException.
>===
>  @Override
>  public RemoteIterator<FileStatus> listStatusIterator(final Path f)
>    throws AccessControlException, FileNotFoundException,
>    UnresolvedLinkException, IOException {
>    return new DirListingIterator<FileStatus>(f, false) {
>
>      @Override
>      public FileStatus next() throws IOException {
>        return getNext().makeQualified(getUri(), f);
>      }
>    };
>  }
>===
>
>Doing this way, I thought the benefit could be the caller can see 
>clearly what kinds of exceptions could be thrown, however in this case 
>we can achieve it by adding the Javadoc. Sure there is no hurt but it 
>looks kinds of dummy in some IDE, hinting some information like "There 
>is a more general exception ... in the thrown list already". Given we 
>would list all the possible exceptions, it's a little hard to maintain 
>the codes.
>
>By the way, in the above example, it looks a little weird that 
>AccessControlException and UnresolvedLinkException both extend 
>IOException. There is a little reason to do that for the latter, but 
>for the former, it looks rather like an issue.
>
>Please help clarify if I missed something. Thanks.
>
>Regards,
>Kai


Re: Question about subtype exceptions in the thrown list in addition to a more general one

Posted by Chris Nauroth <cn...@hortonworks.com>.
Hello Kai,

I'm not aware of a specific coding standard we have on this topic, and I
don't have a strong opinion either way.  I think it can be valuable
sometimes for public APIs to document each subclass in a separate @throws
in the JavaDocs if we expect the caller might want to handle each case
differently.  As you said, this is less relevant for the actual throws
clause in the code.

Regarding IOException, there is a great tendency in the Hadoop codebase to
subclass IOException, even if the failure is not obviously related to I/O.
 This is partly a consequence of the way error handling is implemented in
the RPC framework.  The RemoteException class is tightly coupled to
IOException for its "unwrap" logic to pull out the root cause of the
exception.  As a result, any exception that needs to cross a process
boundary over RPC generally ends up needing to subclass IOException.  I
don't think this is something that can be changed easily.

--Chris Nauroth




On 12/28/15, 7:46 PM, "Zheng, Kai" <ka...@intel.com> wrote:

>
>Hi,
>
>Would it be good to add to throw a subtype exception in addition to a
>more general exception already there in the thrown list? Is it some
>coding style that's required to follow or developers can do it as they
>like?
>
>It's often seen that only the general exception is in the thrown list,
>like in ReadableByteChannel#read in Oracle Java, where in fact the method
>may throw 4 subtype exceptions.
>http://docs.oracle.com/javase/7/docs/api/java/nio/channels/ReadableByteCha
>nnel.html
>int read(ByteBuffer dst) throws IOException
>
>In some of Hadoop codes, it goes otherwise. For example, in Hdfs.java,
>ref. the following codes, note that in the thrown list, all the former 3
>exceptions extends IOException.
>===
>  @Override
>  public RemoteIterator<FileStatus> listStatusIterator(final Path f)
>    throws AccessControlException, FileNotFoundException,
>    UnresolvedLinkException, IOException {
>    return new DirListingIterator<FileStatus>(f, false) {
>
>      @Override
>      public FileStatus next() throws IOException {
>        return getNext().makeQualified(getUri(), f);
>      }
>    };
>  }
>===
>
>Doing this way, I thought the benefit could be the caller can see clearly
>what kinds of exceptions could be thrown, however in this case we can
>achieve it by adding the Javadoc. Sure there is no hurt but it looks
>kinds of dummy in some IDE, hinting some information like "There is a
>more general exception ... in the thrown list already". Given we would
>list all the possible exceptions, it's a little hard to maintain the
>codes.
>
>By the way, in the above example, it looks a little weird that
>AccessControlException and UnresolvedLinkException both extend
>IOException. There is a little reason to do that for the latter, but for
>the former, it looks rather like an issue.
>
>Please help clarify if I missed something. Thanks.
>
>Regards,
>Kai