You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-user@lucene.apache.org by Michael-O <19...@gmx.net> on 2012/11/02 14:20:00 UTC

Excessive use of IOException without proper documentation

Hi,

why does virtually every method (exaggerating) throw an IOE? I know there might be a failure in the underlying IO (corrupt files, passing checked exc up, etc) but

1. Almost none of the has a JavaDoc on it
2. Throwing an IOE from most of the methods doesn't really help. You cannot create separate catch blocks. You have to rip your code apart in tens of try catch blocks. 

Here is a simple example: Both IndexSearcher#search and IndexSearcher#doc throw an IOE with any further documentation. I don't have the chance to detect where the exception has happened nor I can pass something reasonable back to the user. And both methods are keypoints in Lucene.

E.g., there are exceptions thrown in IndexSearcher without any message. Simply empty stubs.

Lucene 4.0 received a complete API overhaul. Why was no action taken to clean up exception management?

Mike

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Excessive use of IOException without proper documentation

Posted by Simon Willnauer <si...@gmail.com>.
On Sun, Nov 4, 2012 at 8:25 PM, Shai Erera <se...@gmail.com> wrote:
> Hey Mike,
>
> I'm not sure that I like the idea of throwing LuceneException or
> SearchException everywhere. I've been there (long time ago) and I always
> hated it.
>
> First, what's the difference between 'new SearchException("Failed to read
> the index", ioe)' and 'new IOException("Failed to read the index",
> anotherIOE/anyE)'? Both don't give you any real way of detecting the
> problem in your code, maybe only in the logs.
>
> Perhaps you would like us to throw better IOE, i.e. rather than re-throwing
> low-level IO exceptions, wrap them w/ another IOE w/ a detailed message.
> That I can support, but do you have a concrete case where the exception
> that was thrown was not descriptive enough?
>
> For instance, there's that "read past EOF" exception which is annoying b/c
> you can tell by the stacktrace where it happened, but not necessarily why
> it happened. However, these are fairly low in our call stack, and I doubt
> that even Lucene code itself can give meaningful messages to all low-level
> IOEs. Yet, the exception does tell you that's usually an index corruption
> case, or some other bug ... when Lucene detects the index is corrupt, it
> throws the explicit CorruptIndexEx, otherwise, this must be inferred from
> the stacktrace. Not perfect, but throwing IndexingException won't improve
> it either.
>
> So I'm +1 for making sure our exceptions are as informative as they can
> get. I'm -1 for starting to throw LuceneException all over the place - to
> me it's like IOE. Worse, if we start throwing LuceneException, people might
> be tempted to wrap any Ex w/ LuceneEx, even ArrayIndexOutOfBound etc. I
> don't want that.
>
> If you've hit exceptions which could use better messages, I prefer that we
> concentrate on them, rather than complicating the exceptions throwing
> mechanism.

+1 to all you said! I could not have phrased it better!

simon
>
> Shai
>
> On Sun, Nov 4, 2012 at 7:25 PM, Michael-O <19...@gmx.net> wrote:
>
>> Hi Simon,
>>
>> there are generally two very good resources how exceptions should be
>> handled in Java. I will quote both:
>>
>> 1. Oracle's JavaDoc Style Guide: http://www.oracle.com/**
>> technetwork/java/javase/**documentation/index-137868.**html#throwstag<http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#throwstag>
>> 2. Joshua Bloch's book Effective Java, chapter 9:
>> http://my.safaribooksonline.**com/book/programming/java/**9780137150021<http://my.safaribooksonline.com/book/programming/java/9780137150021>
>>
>> Am 2012-11-02 22:27, schrieb Simon Willnauer:
>>
>>  Hey,
>>>
>>> On Fri, Nov 2, 2012 at 2:20 PM, Michael-O <19...@gmx.net>
>>> wrote:
>>>
>>>> Hi,
>>>>
>>>> why does virtually every method (exaggerating) throw an IOE? I know
>>>> there might be a failure in the underlying IO (corrupt files,
>>>> passing checked exc up, etc) but
>>>>
>>>> 1. Almost none of the has a JavaDoc on it
>>>>
>>>
>>> what should the javadoc say? I mean it would repeat itself all the
>>> time no?
>>>
>>
>> At least one would know why this one is thrown. See source one.
>>
>>
>>  2. Throwing an IOE from most of the methods doesn't really help.
>>>> You cannot create separate catch blocks. You have to rip your code
>>>> apart in tens of try catch blocks.
>>>>
>>>> Here is a simple example: Both IndexSearcher#search and
>>>> IndexSearcher#doc throw an IOE with any further documentation. I
>>>> don't have the chance to detect where the exception has happened
>>>> nor I can pass something reasonable back to the user. And both
>>>> methods are keypoints in Lucene.
>>>>
>>>
>>> can you elaborate what you would change to make this easier
>>> digestible for you? I mean specialized exceptions class would be a
>>> mess here really.
>>>
>>
>> Not only for me but for everyone.
>>
>> Generally, good exception handling should setup a simple exception
>> hierarchy, not more than three levels. Two base classes for checked and
>> unchecked exceptions. A main exception for a logical group like searching,
>> retreival, query checking, etc.
>>
>> LuceneException
>> |-- SearchException
>> |-- RetrievalException
>> |-- ...
>>
>> One should really wrap IOE in something like 'new SearchException("Failed
>> to read the index", ioe)'. I would at least know that this one came from
>> the index searcher.
>>
>> Really advisable are items 61, 62 and especially 63.
>>
>>
>>  E.g., there are exceptions thrown in IndexSearcher without any
>>>> message. Simply empty stubs.
>>>>
>>>
>>> I agree we should fix them. I already committed some fixes to
>>> IndexSearcher. Can you come up with a patch that fixes some more in
>>> core?
>>>
>>> running grep -R "new.*Exception()" * | wc -l
>>>
>>> yields 82 in core so there is room for improvement.
>>>
>>>
>>>> Lucene 4.0 received a complete API overhaul. Why was no action
>>>> taken to clean up exception management?
>>>>
>>>
>>> I'd really like to hear what you have in mind. can you elaborate?
>>>
>>
>> Continuing my answer from above. Have you ever worked with the Spring
>> Framework? They apply a very nice exception translation pattern. All
>> internal exceptions are turned to specialized unchecked exceptions like
>> AuthenticationExceptoin, DaoAccessException, etc.
>>
>> Lucene has already good exceptions like CorruptIndex, IndexNotFound,
>> LockObtainFailed, etc.
>>
>> Of course, such a step would break backwards-compat but this is possible
>> for 5.0 and this would require a general consensus of the devs improving
>> this issue.
>>
>> I think that such a great framework can do better to inform programmers
>> and users what has really failed especially if you design a public API.
>> We have succeeded exceptional results with the Lucene framework. I am
>> someone who likes giving help back to OSS projects.
>>
>> Hopefully, this is enough input for this message. We could discuss this
>> further is there is a real interest from the devs.
>>
>>
>> Mike
>>
>> ------------------------------**------------------------------**---------
>> To unsubscribe, e-mail: java-user-unsubscribe@lucene.**apache.org<ja...@lucene.apache.org>
>> For additional commands, e-mail: java-user-help@lucene.apache.**org<ja...@lucene.apache.org>
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Excessive use of IOException without proper documentation

Posted by Dawid Weiss <da...@gmail.com>.
> There is a tremendous difference. I have possibility at compile time to
detect whether an IOE came from Lucene or some other IO related action.

I agree with Shai -- there are just as many (or more) reasons not to create
custom exception classes and just reuse existing classes with known
semantics or propagate the original exception without wrapping it.

If you need to know exactly what caused the error then it's really your use
case and you'll have to live with bloated code. For most people IOException
is sufficient -- something related to I/O went wrong, that's it. These are
basically different extreme viewpoints of looking at the same code, there
is no right or wrong (regardless of the excellent references you provide).

Dawid

Re: Excessive use of IOException without proper documentation

Posted by Michael-O <19...@gmx.net>.
Shalom Shai,

Am 2012-11-04 20:25, schrieb Shai Erera:
> Hey Mike,
>
> I'm not sure that I like the idea of throwing LuceneException or
> SearchException everywhere. I've been there (long time ago) and I always
> hated it.

Not everywhere but in the top-level API would be sufficient. Some code 
is never used/should be directly by the client. What is the reason for 
why you dislike such an approach?

> First, what's the difference between 'new SearchException("Failed to read
> the index", ioe)' and 'new IOException("Failed to read the index",
> anotherIOE/anyE)'? Both don't give you any real way of detecting the
> problem in your code, maybe only in the logs.
>
> Perhaps you would like us to throw better IOE, i.e. rather than re-throwing
> low-level IO exceptions, wrap them w/ another IOE w/ a detailed message.
> That I can support, but do you have a concrete case where the exception
> that was thrown was not descriptive enough?

There is a tremendous difference. I have possibility at compile time to 
detect whether an IOE came from Lucene or some other IO related action.

Consider this code:

try {
File mike = new File(...);
Reader r = new InputStreamReader(new FIS(mike)):
searcher.search(...);
}
catch (IOE e) {
}

Now, how am I supposed to figure out at compile time where this IOE came 
from? I cannot neither react properly nor present a suitable user 
response unless I do create for every IOE piece a separate try-catch-block.
This is just bloat. I cannot even log correctly because I do not know 
what the source of the IOE was.

What am I excepted to do when every method throws just new Exception()?
Absolutely nothing they would all look alike at compile time.

> For instance, there's that "read past EOF" exception which is annoying b/c
> you can tell by the stacktrace where it happened, but not necessarily why
> it happened. However, these are fairly low in our call stack, and I doubt
> that even Lucene code itself can give meaningful messages to all low-level
> IOEs. Yet, the exception does tell you that's usually an index corruption
> case, or some other bug ... when Lucene detects the index is corrupt, it
> throws the explicit CorruptIndexEx, otherwise, this must be inferred from
> the stacktrace. Not perfect, but throwing IndexingException won't improve
> it either.

It would cover at least the case described above.

> So I'm +1 for making sure our exceptions are as informative as they can
> get. I'm -1 for starting to throw LuceneException all over the place - to
> me it's like IOE. Worse, if we start throwing LuceneException, people might
> be tempted to wrap any Ex w/ LuceneEx, even ArrayIndexOutOfBound etc. I
> don't want that.

That should not be the case. IndexOutOfBoundsException is an unchecked 
exception.
There is a huge difference to IOE which is a checked exception. Joshua 
Bloch exactly describes the case of the IOOBE in item 58. IIOOBE must 
remain as unchecked because it indicates a violation of the API 
contract. E.g., a corrupt index is recoverable and not a programming 
error. This can happen through various reasons you cannot control.

> If you've hit exceptions which could use better messages, I prefer that we
> concentrate on them, rather than complicating the exceptions throwing
> mechanism.

I definitively will. As I always do with OSS.

> On Sun, Nov 4, 2012 at 7:25 PM, Michael-O <19...@gmx.net> wrote:
>
>> Hi Simon,
>>
>> there are generally two very good resources how exceptions should be
>> handled in Java. I will quote both:
>>
>> 1. Oracle's JavaDoc Style Guide: http://www.oracle.com/**
>> technetwork/java/javase/**documentation/index-137868.**html#throwstag<http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#throwstag>
>> 2. Joshua Bloch's book Effective Java, chapter 9:
>> http://my.safaribooksonline.**com/book/programming/java/**9780137150021<http://my.safaribooksonline.com/book/programming/java/9780137150021>
>>
>> Am 2012-11-02 22:27, schrieb Simon Willnauer:
>>
>>   Hey,
>>>
>>> On Fri, Nov 2, 2012 at 2:20 PM, Michael-O <19...@gmx.net>
>>> wrote:
>>>
>>>> Hi,
>>>>
>>>> why does virtually every method (exaggerating) throw an IOE? I know
>>>> there might be a failure in the underlying IO (corrupt files,
>>>> passing checked exc up, etc) but
>>>>
>>>> 1. Almost none of the has a JavaDoc on it
>>>>
>>>
>>> what should the javadoc say? I mean it would repeat itself all the
>>> time no?
>>>
>>
>> At least one would know why this one is thrown. See source one.
>>
>>
>>   2. Throwing an IOE from most of the methods doesn't really help.
>>>> You cannot create separate catch blocks. You have to rip your code
>>>> apart in tens of try catch blocks.
>>>>
>>>> Here is a simple example: Both IndexSearcher#search and
>>>> IndexSearcher#doc throw an IOE with any further documentation. I
>>>> don't have the chance to detect where the exception has happened
>>>> nor I can pass something reasonable back to the user. And both
>>>> methods are keypoints in Lucene.
>>>>
>>>
>>> can you elaborate what you would change to make this easier
>>> digestible for you? I mean specialized exceptions class would be a
>>> mess here really.
>>>
>>
>> Not only for me but for everyone.
>>
>> Generally, good exception handling should setup a simple exception
>> hierarchy, not more than three levels. Two base classes for checked and
>> unchecked exceptions. A main exception for a logical group like searching,
>> retreival, query checking, etc.
>>
>> LuceneException
>> |-- SearchException
>> |-- RetrievalException
>> |-- ...
>>
>> One should really wrap IOE in something like 'new SearchException("Failed
>> to read the index", ioe)'. I would at least know that this one came from
>> the index searcher.
>>
>> Really advisable are items 61, 62 and especially 63.
>>
>>
>>   E.g., there are exceptions thrown in IndexSearcher without any
>>>> message. Simply empty stubs.
>>>>
>>>
>>> I agree we should fix them. I already committed some fixes to
>>> IndexSearcher. Can you come up with a patch that fixes some more in
>>> core?
>>>
>>> running grep -R "new.*Exception()" * | wc -l
>>>
>>> yields 82 in core so there is room for improvement.
>>>
>>>
>>>> Lucene 4.0 received a complete API overhaul. Why was no action
>>>> taken to clean up exception management?
>>>>
>>>
>>> I'd really like to hear what you have in mind. can you elaborate?
>>>
>>
>> Continuing my answer from above. Have you ever worked with the Spring
>> Framework? They apply a very nice exception translation pattern. All
>> internal exceptions are turned to specialized unchecked exceptions like
>> AuthenticationExceptoin, DaoAccessException, etc.
>>
>> Lucene has already good exceptions like CorruptIndex, IndexNotFound,
>> LockObtainFailed, etc.
>>
>> Of course, such a step would break backwards-compat but this is possible
>> for 5.0 and this would require a general consensus of the devs improving
>> this issue.
>>
>> I think that such a great framework can do better to inform programmers
>> and users what has really failed especially if you design a public API.
>> We have succeeded exceptional results with the Lucene framework. I am
>> someone who likes giving help back to OSS projects.
>>
>> Hopefully, this is enough input for this message. We could discuss this
>> further is there is a real interest from the devs.
>>
>>
>> Mike
>>
>> ------------------------------**------------------------------**---------
>> To unsubscribe, e-mail: java-user-unsubscribe@lucene.**apache.org<ja...@lucene.apache.org>
>> For additional commands, e-mail: java-user-help@lucene.apache.**org<ja...@lucene.apache.org>
>>
>>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Excessive use of IOException without proper documentation

Posted by Shai Erera <se...@gmail.com>.
Hey Mike,

I'm not sure that I like the idea of throwing LuceneException or
SearchException everywhere. I've been there (long time ago) and I always
hated it.

First, what's the difference between 'new SearchException("Failed to read
the index", ioe)' and 'new IOException("Failed to read the index",
anotherIOE/anyE)'? Both don't give you any real way of detecting the
problem in your code, maybe only in the logs.

Perhaps you would like us to throw better IOE, i.e. rather than re-throwing
low-level IO exceptions, wrap them w/ another IOE w/ a detailed message.
That I can support, but do you have a concrete case where the exception
that was thrown was not descriptive enough?

For instance, there's that "read past EOF" exception which is annoying b/c
you can tell by the stacktrace where it happened, but not necessarily why
it happened. However, these are fairly low in our call stack, and I doubt
that even Lucene code itself can give meaningful messages to all low-level
IOEs. Yet, the exception does tell you that's usually an index corruption
case, or some other bug ... when Lucene detects the index is corrupt, it
throws the explicit CorruptIndexEx, otherwise, this must be inferred from
the stacktrace. Not perfect, but throwing IndexingException won't improve
it either.

So I'm +1 for making sure our exceptions are as informative as they can
get. I'm -1 for starting to throw LuceneException all over the place - to
me it's like IOE. Worse, if we start throwing LuceneException, people might
be tempted to wrap any Ex w/ LuceneEx, even ArrayIndexOutOfBound etc. I
don't want that.

If you've hit exceptions which could use better messages, I prefer that we
concentrate on them, rather than complicating the exceptions throwing
mechanism.

Shai

On Sun, Nov 4, 2012 at 7:25 PM, Michael-O <19...@gmx.net> wrote:

> Hi Simon,
>
> there are generally two very good resources how exceptions should be
> handled in Java. I will quote both:
>
> 1. Oracle's JavaDoc Style Guide: http://www.oracle.com/**
> technetwork/java/javase/**documentation/index-137868.**html#throwstag<http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#throwstag>
> 2. Joshua Bloch's book Effective Java, chapter 9:
> http://my.safaribooksonline.**com/book/programming/java/**9780137150021<http://my.safaribooksonline.com/book/programming/java/9780137150021>
>
> Am 2012-11-02 22:27, schrieb Simon Willnauer:
>
>  Hey,
>>
>> On Fri, Nov 2, 2012 at 2:20 PM, Michael-O <19...@gmx.net>
>> wrote:
>>
>>> Hi,
>>>
>>> why does virtually every method (exaggerating) throw an IOE? I know
>>> there might be a failure in the underlying IO (corrupt files,
>>> passing checked exc up, etc) but
>>>
>>> 1. Almost none of the has a JavaDoc on it
>>>
>>
>> what should the javadoc say? I mean it would repeat itself all the
>> time no?
>>
>
> At least one would know why this one is thrown. See source one.
>
>
>  2. Throwing an IOE from most of the methods doesn't really help.
>>> You cannot create separate catch blocks. You have to rip your code
>>> apart in tens of try catch blocks.
>>>
>>> Here is a simple example: Both IndexSearcher#search and
>>> IndexSearcher#doc throw an IOE with any further documentation. I
>>> don't have the chance to detect where the exception has happened
>>> nor I can pass something reasonable back to the user. And both
>>> methods are keypoints in Lucene.
>>>
>>
>> can you elaborate what you would change to make this easier
>> digestible for you? I mean specialized exceptions class would be a
>> mess here really.
>>
>
> Not only for me but for everyone.
>
> Generally, good exception handling should setup a simple exception
> hierarchy, not more than three levels. Two base classes for checked and
> unchecked exceptions. A main exception for a logical group like searching,
> retreival, query checking, etc.
>
> LuceneException
> |-- SearchException
> |-- RetrievalException
> |-- ...
>
> One should really wrap IOE in something like 'new SearchException("Failed
> to read the index", ioe)'. I would at least know that this one came from
> the index searcher.
>
> Really advisable are items 61, 62 and especially 63.
>
>
>  E.g., there are exceptions thrown in IndexSearcher without any
>>> message. Simply empty stubs.
>>>
>>
>> I agree we should fix them. I already committed some fixes to
>> IndexSearcher. Can you come up with a patch that fixes some more in
>> core?
>>
>> running grep -R "new.*Exception()" * | wc -l
>>
>> yields 82 in core so there is room for improvement.
>>
>>
>>> Lucene 4.0 received a complete API overhaul. Why was no action
>>> taken to clean up exception management?
>>>
>>
>> I'd really like to hear what you have in mind. can you elaborate?
>>
>
> Continuing my answer from above. Have you ever worked with the Spring
> Framework? They apply a very nice exception translation pattern. All
> internal exceptions are turned to specialized unchecked exceptions like
> AuthenticationExceptoin, DaoAccessException, etc.
>
> Lucene has already good exceptions like CorruptIndex, IndexNotFound,
> LockObtainFailed, etc.
>
> Of course, such a step would break backwards-compat but this is possible
> for 5.0 and this would require a general consensus of the devs improving
> this issue.
>
> I think that such a great framework can do better to inform programmers
> and users what has really failed especially if you design a public API.
> We have succeeded exceptional results with the Lucene framework. I am
> someone who likes giving help back to OSS projects.
>
> Hopefully, this is enough input for this message. We could discuss this
> further is there is a real interest from the devs.
>
>
> Mike
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.**apache.org<ja...@lucene.apache.org>
> For additional commands, e-mail: java-user-help@lucene.apache.**org<ja...@lucene.apache.org>
>
>

Re: Excessive use of IOException without proper documentation

Posted by Shai Erera <se...@gmail.com>.
I think that specific exceptions should be thrown only in case we expect
the user to do something with it. E.g. LockObtainException is something
that I can catch and try to recover from in the code, maybe retry to obtain
the lock.

But all IOExceptions, maybe excluding FNFE, are unrecoverable in the sense
that there's nothing that you can really do in your code, besides
re-throwing it. I agree with TX - it would have been better if we just
threw IOError, but I don't think it's necessary to change the API at this
point.

As long as Lucene throws IOE with meaningful messages, which will be
informative for logging, I'm ok with that. If there are exceptions that are
thrown with obscured messages, let's fix them.

And yes, let's make sure that Lucene doesn't throw *only* IOE. I.e., if
there's code that needs to throw an Exception, and it chooses IOE just
because the rest of the code throws it, then that's wrong. IOE should
indicate IO errors. Most of Lucene code is low-level enough to trip on IO
errors. But if there's a QueryParser code that never does IO, then it
should pick a proper exception to throw. That's my preference, others may
disagree.

In the end, as Dawid put it, these are different viewpoints, there's no
right or wrong. As long as we're consistent and informative, our API will
be in good shape.

Shai

On Mon, Nov 5, 2012 at 4:30 AM, Trejkaz <tr...@trypticon.org> wrote:

> On Mon, Nov 5, 2012 at 4:25 AM, Michael-O <19...@gmx.net> wrote:
> > Continuing my answer from above. Have you ever worked with the Spring
> > Framework? They apply a very nice exception translation pattern. All
> > internal exceptions are turned to specialized unchecked exceptions like
> > AuthenticationExceptoin, DaoAccessException, etc.
>
> I quite like unchecked exceptions. I think that checked exceptions
> have their place as a special return value, but once you're out of the
> one method which has the special return value, all other exceptions
> should really be unchecked. It wouldn't be so bad if it weren't for
> Java's compile-time checking that you have caught them. (A lot of the
> time you're dealing with parallel execution or caching APIs and the
> existence of a single checked exception means you now really do have
> to catch Exception...)
>
> IOException in particular is just awful, in my opinion. Everything
> which does I/O throws the exception, even though in tons of cases, the
> only thing which can cause it to be thrown is hardware failure,
> unplugging a disk or some rogue user going through and marking all
> your files read-only. All of these scenarios to me are along the same
> lines as running out of memory, yet OutOfMemoryError is an Error while
> IOException is a checked Exception.
>
> They realised their mistake and started to introduce IOError for these
> unexpected situations, but the APIs which already throw IOException
> are probably poisoned for life.
>
> I understand that there are legitimate cases of IOException, such as
> FileNotFoundException when trying to open a file which doesn't exist.
> But that is certainly not the sort which Lucene is propagating out
> through its API (or if it is, it shouldn't be.)
>
> As far as what users of Lucene are doing, in our case we're catching
> any IOException Lucene throws and wrapping it in an unchecked
> exception. If anything goes wrong with stores it's a pretty
> catastrophic situation which we won't be able to recover from anyway.
> And because IOException is being thrown with no additional
> information, it's not like we can make use of the information in the
> exception even if we caught it.
>
> TX
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-user-help@lucene.apache.org
>
>

Re: Excessive use of IOException without proper documentation

Posted by Trejkaz <tr...@trypticon.org>.
On Mon, Nov 5, 2012 at 4:25 AM, Michael-O <19...@gmx.net> wrote:
> Continuing my answer from above. Have you ever worked with the Spring
> Framework? They apply a very nice exception translation pattern. All
> internal exceptions are turned to specialized unchecked exceptions like
> AuthenticationExceptoin, DaoAccessException, etc.

I quite like unchecked exceptions. I think that checked exceptions
have their place as a special return value, but once you're out of the
one method which has the special return value, all other exceptions
should really be unchecked. It wouldn't be so bad if it weren't for
Java's compile-time checking that you have caught them. (A lot of the
time you're dealing with parallel execution or caching APIs and the
existence of a single checked exception means you now really do have
to catch Exception...)

IOException in particular is just awful, in my opinion. Everything
which does I/O throws the exception, even though in tons of cases, the
only thing which can cause it to be thrown is hardware failure,
unplugging a disk or some rogue user going through and marking all
your files read-only. All of these scenarios to me are along the same
lines as running out of memory, yet OutOfMemoryError is an Error while
IOException is a checked Exception.

They realised their mistake and started to introduce IOError for these
unexpected situations, but the APIs which already throw IOException
are probably poisoned for life.

I understand that there are legitimate cases of IOException, such as
FileNotFoundException when trying to open a file which doesn't exist.
But that is certainly not the sort which Lucene is propagating out
through its API (or if it is, it shouldn't be.)

As far as what users of Lucene are doing, in our case we're catching
any IOException Lucene throws and wrapping it in an unchecked
exception. If anything goes wrong with stores it's a pretty
catastrophic situation which we won't be able to recover from anyway.
And because IOException is being thrown with no additional
information, it's not like we can make use of the information in the
exception even if we caught it.

TX

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Excessive use of IOException without proper documentation

Posted by Michael-O <19...@gmx.net>.
Hi Simon,

there are generally two very good resources how exceptions should be 
handled in Java. I will quote both:

1. Oracle's JavaDoc Style Guide: 
http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#throwstag
2. Joshua Bloch's book Effective Java, chapter 9: 
http://my.safaribooksonline.com/book/programming/java/9780137150021

Am 2012-11-02 22:27, schrieb Simon Willnauer:
> Hey,
>
> On Fri, Nov 2, 2012 at 2:20 PM, Michael-O <19...@gmx.net>
> wrote:
>> Hi,
>>
>> why does virtually every method (exaggerating) throw an IOE? I know
>> there might be a failure in the underlying IO (corrupt files,
>> passing checked exc up, etc) but
>>
>> 1. Almost none of the has a JavaDoc on it
>
> what should the javadoc say? I mean it would repeat itself all the
> time no?

At least one would know why this one is thrown. See source one.

>> 2. Throwing an IOE from most of the methods doesn't really help.
>> You cannot create separate catch blocks. You have to rip your code
>> apart in tens of try catch blocks.
>>
>> Here is a simple example: Both IndexSearcher#search and
>> IndexSearcher#doc throw an IOE with any further documentation. I
>> don't have the chance to detect where the exception has happened
>> nor I can pass something reasonable back to the user. And both
>> methods are keypoints in Lucene.
>
> can you elaborate what you would change to make this easier
> digestible for you? I mean specialized exceptions class would be a
> mess here really.

Not only for me but for everyone.

Generally, good exception handling should setup a simple exception 
hierarchy, not more than three levels. Two base classes for checked and 
unchecked exceptions. A main exception for a logical group like 
searching, retreival, query checking, etc.

LuceneException
|-- SearchException
|-- RetrievalException
|-- ...

One should really wrap IOE in something like 'new 
SearchException("Failed to read the index", ioe)'. I would at least know 
that this one came from the index searcher.

Really advisable are items 61, 62 and especially 63.

>> E.g., there are exceptions thrown in IndexSearcher without any
>> message. Simply empty stubs.
>
> I agree we should fix them. I already committed some fixes to
> IndexSearcher. Can you come up with a patch that fixes some more in
> core?
>
> running grep -R "new.*Exception()" * | wc -l
>
> yields 82 in core so there is room for improvement.
>
>>
>> Lucene 4.0 received a complete API overhaul. Why was no action
>> taken to clean up exception management?
>
> I'd really like to hear what you have in mind. can you elaborate?

Continuing my answer from above. Have you ever worked with the Spring 
Framework? They apply a very nice exception translation pattern. All 
internal exceptions are turned to specialized unchecked exceptions like
AuthenticationExceptoin, DaoAccessException, etc.

Lucene has already good exceptions like CorruptIndex, IndexNotFound, 
LockObtainFailed, etc.

Of course, such a step would break backwards-compat but this is possible 
for 5.0 and this would require a general consensus of the devs improving 
this issue.

I think that such a great framework can do better to inform programmers 
and users what has really failed especially if you design a public API.
We have succeeded exceptional results with the Lucene framework. I am 
someone who likes giving help back to OSS projects.

Hopefully, this is enough input for this message. We could discuss this 
further is there is a real interest from the devs.

Mike

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Excessive use of IOException without proper documentation

Posted by Simon Willnauer <si...@gmail.com>.
Hey,

On Fri, Nov 2, 2012 at 2:20 PM, Michael-O <19...@gmx.net> wrote:
> Hi,
>
> why does virtually every method (exaggerating) throw an IOE? I know there might be a failure in the underlying IO (corrupt files, passing checked exc up, etc) but
>
> 1. Almost none of the has a JavaDoc on it

what should the javadoc say? I mean it would repeat itself all the time no?

> 2. Throwing an IOE from most of the methods doesn't really help. You cannot create separate catch blocks. You have to rip your code apart in tens of try catch blocks.
>
> Here is a simple example: Both IndexSearcher#search and IndexSearcher#doc throw an IOE with any further documentation. I don't have the chance to detect where the exception has happened nor I can pass something reasonable back to the user. And both methods are keypoints in Lucene.

can you elaborate what you would change to make this easier digestible
for you? I mean specialized exceptions class would be a mess here
really.

>
> E.g., there are exceptions thrown in IndexSearcher without any message. Simply empty stubs.

I agree we should fix them. I already committed some fixes to
IndexSearcher. Can you come up with a patch that fixes some more in
core?

running grep -R "new.*Exception()" * | wc -l

yields 82 in core so there is room for improvement.

>
> Lucene 4.0 received a complete API overhaul. Why was no action taken to clean up exception management?

I'd really like to hear what you have in mind. can you elaborate?

simon
>
> Mike
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-user-help@lucene.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org