NightOwl888 opened a new issue #446:
URL: https://github.com/apache/lucenenet/issues/446
We need to review exception types to ensure we are throwing and catching the
right exceptions.
## Why does it matter?
In Lucene, exceptions are used like glorified `goto` statements for control
flow by throwing exceptions in one place and catching them further up the
stack. While we made some good effort to remove this behavior from most of the
application, it is baked into the design of `IndexWriter`, `IndexReader` and
many types that support them. Removing the behaivor would require some major
reworking of the design and IMO would be too far removed from Lucene's design
to be maintainable.
## How it works in Lucene
Much like in .NET, Java exception families are controlled by inheritance.
However, there are certain "checked" exceptions that *must* be handled or
declared in the method signature for the caller to handle, and this behavior is
enforced by the compiler. So, to centralize places that handle common exception
scenarios, in Java it is convenient to throw an exception in many places and
handle that exception in a single place.
There are 4 main types of exceptions in Java and one more family that Lucene
cares about (`java.io.IOException`):
- `java.lang.Throwable` - Base class of all errors.
- `java.lang.Error` - indicates serious problems that a reasonable
application should not try to catch.
- `java.lang.Exception` - indicates conditions that a reasonable
application might want to catch.
- `java.lang.RuntimeException` - `RuntimeException` and its subclasses
are unchecked exceptions. Unchecked exceptions do not need to be declared in a
method or constructor's throws clause if they can be thrown by the execution of
the method or constructor and propagate outside the method or constructor
boundary.
- `java.io.IOException` - the general class of exceptions produced by
failed or interrupted I/O operations.
Basically, Java applications must handle everything that derive from
`java.lang.Exception` except for exceptions that derive from
`java.lang.RuntimeException`. As a result, a common thing to do throughout the
Lucene codebase is to catch `java.lang.Exception` (and all derived classes of
it) and wrap it in a `java.lang.RuntimeException` so it can be re-thrown. This
has a side-effect of bypassing all catch blocks and throwing the exception to
the outside world.
> **NOTE:** This bypass logic is hard to spot by the untrained eye. It is
tempting to take the catch blocks out and let the error propagate if not
considered carefully.
## How Lucene.NET is doing it
Since this is basically a line-by-line port, we use the closest exception
type we could match with the Java exception type. However:
1. We got it wrong in several cases.
2. We did it inconsistently - an exception type in one place wasn't
translated to the same exception type in another place.
3. We used `System.Exception` to replace `java.lang.Throwable`,
`java.lang.Error`, `java.lang.Exception`, and `java.lang.RuntimeException`
without considering all of the ramifications.
4. Java and .NET do not consistently derive from the same families of
exceptions, leaving several gaps when inheriting general exception types.
This means we are catching exceptions in cases where we should not, and
allowing exceptions to propagate that we should be catching and handling.
## Java Exceptions vs .NET Exceptions in Lucene
The following table is a listing of all exception types caught in Lucene
along with their inheritance hierarchy in both Java and in excpetions that
translated them to in .NET. Note that the .NET type list is not comprehensive,
but demonstrates that we aren't catching excpetions correctly because of the
errors we made above.
<details>
<summary>Click for details</summary>
<table>
<tr>
<th>Java Exception Type</th>
<th>Java Superclass Types</th>
<th>.NET Exception Type</th>
<th>.NET Superclass Types</th>
<th>Notes</th>
</tr>
<tr>
<td>java.io.IOException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>System.IO.IOException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td rowspan="2">java.lang.ArrayIndexOutOfBoundsException</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>--
java.lang.IndexOutOfBoundsException</td>
<td>System.IndexOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>System.ArgumentOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException<br/>--- System.ArgumentException</td>
<td>We are currently not catching this exception in all places where
IndexOutOfRangeException is caught.</td>
</tr>
<tr>
<td rowspan="2">java.lang.StringIndexOutOfBoundsException</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>--
java.lang.IndexOutOfBoundsException</td>
<td>System.IndexOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>System.ArgumentOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException<br/>--- System.ArgumentException</td>
<td>We are currently not catching this exception in all places where
IndexOutOfRangeException is caught.</td>
</tr>
<tr>
<td rowspan="2">java.lang.IndexOutOfBoundsException</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.IndexOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>System.ArgumentOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException<br/>--- System.ArgumentException</td>
<td>We are currently not catching this exception in all places where
IndexOutOfRangeException is caught.</td>
</tr>
<tr>
<td rowspan="3">java.text.ParseException</td>
<td rowspan="3">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>System.FormatException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>Does not explicitly support a way to keep track of the error index,
but we could add an extension method and store the information in the
FormatExcption.Data property. Also, in QueryParser there were ParseException
classes that had been generated in Java - we might be able to eliminate these
and just use FormatException consistently.</td>
</tr>
<tr>
<td>System.InvalidOperationException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>There are some places where ParseException errors were translated to
InvalidOperationException - these should probably be changed to
FormatException.</td>
</tr>
<tr>
<td>System.Exception</td>
<td>System.Object</td>
<td>There are some places where ParseException errors were translated to
Exception - these should probably be changed to FormatException.</td>
</tr>
<tr>
<td>java.lang.Exception</td>
<td>java.lang.Object <br/>- java.lang.Throwable</td>
<td>System.Exception</td>
<td>System.Object</td>
<td></td>
</tr>
<tr>
<td>java.lang.CloneNotSupportedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>None</td>
<td></td>
<td></td>
</tr>
<tr>
<td rowspan="2">java.io.FileNotFoundException</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException</td>
<td>System.IO.FileNotFoundException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>System.IO.DirectoryNotFoundException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>java.lang.NoSuchMethodException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.ReflectiveOperationException</td>
<td>System.MissingMethodException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td>This is wrong. Reflection in .NET doesn't throw exceptions when it
cannot find a method, it simply returns null. However, it can throw when there
is an ambiguous match, and not sure that is accounted for.</td>
</tr>
<tr>
<td>java.lang.reflect.InvocationTargetException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.ReflectiveOperationException</td>
<td>System.TargetInvocationException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.ApplicationException</td>
<td></td>
</tr>
<tr>
<td rowspan="2">java.lang.IllegalAccessException</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.ReflectiveOperationException</td>
<td>System.Exception</td>
<td>System.Object <br/>- System.Exception</td>
<td>This seems to be done as a catch-all in .NET because Reflection
Invoke() can throw a number of different exceptions. But this needs further
review.</td>
</tr>
<tr>
<td>System.UnauthorizedAccessException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException<</td>
<td>This exception doesn't happen during Reflection calls, so this
should be changed to System.MemberAccessException, although partial trust is
obsolete so this error is likely only valid on .NET Framework.</td>
</tr>
<tr>
<td rowspan="3">java.lang.IllegalArgumentException</td>
<td rowspan="3">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.ArgumentException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>System.ArgumentOutOfRangeException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.ArgumentException</td>
<td>Some guard clauses have been added with this type. Need to check to
ensure places that catch IndexOutOfRangeException and this exception are
correct.</td>
</tr>
<tr>
<td>System.ArgumentNullException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.ArgumentException</td>
<td>Some guard clauses have been added with this type (J2N). Since it
subclasses ArgumentException, we are covering IllegalArgumentException, but
need to review NullReferenceException to see if there are problems and perhaps
merge the two.</td>
</tr>
<tr>
<td>java.lang.NullPointerException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.NullReferenceException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>It might be best to convert everywhere this is used to
ArgumentNullException so we don't swallow NullReferenceException
unintentionally and hide problems.</td>
</tr>
<tr>
<td>java.lang.InstantiationException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.ReflectiveOperationException</td>
<td>System.TypeInitializationException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>In Java, the exception comes from Reflection. In .NET, it may happen
due to a class initializer that throws an uncaught exception. These 2 are
definitely not the same thing, but I am not sure it makes a difference.</td>
</tr>
<tr>
<td>java.lang.UnsupportedOperationException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.NotSupportedException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>java.lang.Throwable</td>
<td>java.lang.Object</td>
<td>System.Exception</td>
<td>System.Object</td>
<td></td>
</tr>
<tr>
<td>java.lang.RuntimeException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>System.Exception</td>
<td>System.Object</td>
<td></td>
</tr>
<tr>
<td>java.lang.ClassNotFoundException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.ReflectiveOperationException</td>
<td>None</td>
<td></td>
<td>Reflection in .NET has no equivalent. However, in places where it is
used we are not wrapping any exceptions in an Exception instance, which may
mean there could be problems with excpetions getting caught in calling code and
not handled appropriately.</td>
</tr>
<tr>
<td>java.util.NoSuchElementException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>None</td>
<td></td>
<td>.NET enumerators do not throw when there are no more elements, they
return false. Some places are catching InvalidOperationException instead, but
they shouldn't be.</td>
</tr>
<tr>
<td>org.xml.sax.SAXException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Sax.SaxException</td>
<td>System.Object <br/>- System.Exception</td>
<td>Exists only in the Support folder of Lucene.Net.Benchmark.</td>
</tr>
<tr>
<td>java.lang.InterruptedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>System.Threading.ThreadInterruptedException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.util.ThreadInterruptedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>-- System.SystemException</td>
<td>System.Threading.ThreadInterruptedException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>This class exists to convert the exception into RuntimeException. It
is possible we have missed something fundamental here.</td>
</tr>
<tr>
<td>org.apache.lucene.benchmark.bytask.feeds.NoMoreDataException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Lucene.Net.Benchmark.ByTask.Feeds.NoMoreDataException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>java.lang.NumberFormatException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>----
java.lang.IllegalArgumentException</td>
<td>System.FormatException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>It is possible we missed some catch blocks because FormatExcption
doesn't subclass ArgumentException, needs review to make sure the correct
exception types are being thrown.</td>
</tr>
<tr>
<td>org.apache.commons.compress.compressors.CompressorException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>None</td>
<td></td>
<td>.NET and SharpZipLib do not throw when instantiating a compression
stream.</td>
</tr>
<tr>
<td>java.util.zip.DataFormatException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>None</td>
<td></td>
<td>We aren't catching an exception for compression streams, but this
needs review.</td>
</tr>
<tr>
<td>java.lang.SecurityException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>-- java.lang.RuntimeException</td>
<td>System.Security.SecurityException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td rowspan="2">java.nio.file.NoSuchFileException</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>-- java.io.IOException <br/>--
java.nio.file.FileSystemException</td>
<td>System.IO.FileNotFoundException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>System.IO.DirectoryNotFoundException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.store.NoSuchDirectoryException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException <br/>----
java.io.FileNotFoundDirectory</td>
<td>System.IO.DirectoryNotFoundException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>java.lang.OutOfMemoryError</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>-- java.lang.Error
<br/>--- java.lang.VirtualMachineError</td>
<td>System.OutOfMemoryException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>Microsoft recommends calling Environment.FailFast() in this case,
but we need a review to ensure we are doing so. I believe some tests are also
marked [Ignore] because it was assumed that OutOfMemoryException cannot be
caught in .NET.</td>
</tr>
<tr>
<td>org.apache.lucene.store.AlreadyClosedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>---
java.lang.IllegalStateException</td>
<td>System.ObjectDisposedException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>-- System.InvalidOperationException</td>
<td>In prior versions of Lucene.NET, there were at least attempts to use
both ICloseable and IDisposable. We need to revisit this.</td>
</tr>
<tr>
<td>java.nio.charset.CharacterCodingException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException</td>
<td>All</td>
<td></td>
<td>Perhaps catching all exceptions is not the right decision here.</td>
</tr>
<tr>
<td>org.apache.lucene.util.BytesRefHash.MaxBytesLengthExceededException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>Lucene.Net.Util.BytesRefHash.MaxBytesLengthExceededException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.search.CollectionTerminatedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>Lucene.Net.Search.CollectionTerminatedException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>java.util.concurrent.ExecutionException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>All</td>
<td></td>
<td>Needs review.</td>
</tr>
<tr>
<td>java.nio.BufferUnderflowException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>J2N.IO.BufferUnderflowException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>java.lang.ClassCastException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>J2N.InvalidCastException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.store.LockObtainFailedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException</td>
<td>Lucene.Net.Store.LockObtainFailedException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>java.security.PrivilegedActionException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>None</td>
<td></td>
<td>This exception is used to work around a bug in Java in
MMapDirectory.</td>
</tr>
<tr>
<td>java.nio.channels.OverlappingFileLockException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>----
java.lang.IllegalStateException</td>
<td>None</td>
<td></td>
<td>NativeFSLockFactory was redesigned for .NET and this exception was
factored out.</td>
</tr>
<tr>
<td rowspan="2">java.lang.AssertionError</td>
<td rowspan="2">java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Error</td>
<td>System.InvalidOperationException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>Lucene.Net.Diagnostics.AssertionException was added later. Should we
migrate everything over to it?</td>
</tr>
<tr>
<td>Lucene.Net.Diagnostics.AssertionException</td>
<td>System.Object <br/>- System.Exception</td>
<td>This is only used in Debugging.Assert() method calls.</td>
</tr>
<tr>
<td>java.lang.Error</td>
<td>java.lang.Object <br/>- java.lang.Throwable</td>
<td>System.Exception</td>
<td>System.Object</td>
<td></td>
</tr>
<tr>
<td>java.io.EOFException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>-- java.io.IOException</td>
<td>System.IO.EndOfStreamException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>--- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>java.lang.IllegalStateException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.InvalidOperationException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.index.IndexFormatTooOldException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException <br/>---
org.apache.lucene.index.CorruptIndexException</td>
<td>Lucene.Net.Index.IndexFormatTooOldException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>-- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.index.CorruptIndexException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException <br/>---
org.apache.lucene.index.CorruptIndexException</td>
<td>Lucene.Net.Index.IndexFormatTooOldException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>-- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.index.TestCrashCausesCorruptIndex.CrashingException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>Lucene.Net.Index.TestCrashCausesCorruptIndex.CrashingException</td>
<td>System.Object <br/>- System.Exception</td>
<td>Only exists in one test class.</td>
</tr>
<tr>
<td>org.apache.lucene.util.SetOnce.AlreadySetException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>----
java.lang.IllegalStateException</td>
<td>Lucene.Net.Util.AlreadySetException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>-- System.InvalidOperationException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.store.MockDirectoryWrapper.FakeIOException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException</td>
<td>Lucene.Net.Store.FakeIOException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>-- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.search.TimeLimitingCollector.TimeExceededException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>Lucene.Net.Search.TimeLimitingCollector.TimeExceededException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.antlr.runtime.RecognitionException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Antlr.Runtime.RecognitionException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>java.lang.StackOverflowError</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>-- java.lang.Error
<br/>--- java.lang.VirtualMachineError</td>
<td>System.StackOverflowException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>Uncatchable in .NET. Microsoft recommends breaking out of recursive
calls with a max value setting.</td>
</tr>
<tr>
<td>java.lang.ArithmeticException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.ArithmeticException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.search.BooleanQuery.TooManyClauses</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>Lucene.Net.Search.BooleanQuery.TooManyClausesException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.classic.ParseException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Lucene.Net.QueryParsers.Classic.ParseException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.classic.TokenMgrError</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Error</td>
<td>Lucene.Net.QueryParsers.Classic.TokenMgrError</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>java.util.MissingResourceException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException</td>
<td>System.Resources.MissingManifestResourceException </td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.flexible.standard.parser.ParseException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>---
org.apache.lucene.queryparser.flexible.core.QueryNodeException <br/>----
org.apache.lucene.queryparser.flexible.core.QueryNodeParseException</td>
<td>Lucene.Net.QueryParsers.Flexible.Standard.Parser.ParseException</td>
<td>System.Object <br/>- System.Exception <br/>--
Lucene.Net.QueryParsers.Flexible.Core.QueryNodeException <br/>---
Lucene.Net.QueryParsers.Flexible.Core.QueryNodeParseException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.flexible.standard.parser.ParseException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Lucene.Net.QueryParsers.Flexible.Standard.Parser.ParseException</td>
<td>System.Object <br/>- System.Exception <br/>--
Lucene.Net.QueryParsers.Flexible.Core.QueryNodeException</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.flexible.standard.parser.TokenMgrError</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Error</td>
<td>Lucene.Net.QueryParsers.Flexible.Standard.Parser.TokenMgrError</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.surround.parser.TokenMgrError</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Error</td>
<td>Lucene.Net.QueryParsers.Surround.Parser.TokenMgrError</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.surround.ParseException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Lucene.Net.QueryParsers.Surround.ParseException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.queryparser.xml.ParserException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception</td>
<td>Lucene.Net.QueryParsers.Xml.ParserException</td>
<td>System.Object <br/>- System.Exception</td>
<td></td>
</tr>
<tr>
<td>org.apache.lucene.replicator.SessionExpiredException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException</td>
<td>Lucene.Net.Replicator.SessionExpiredException</td>
<td>System.Object <br/>- System.Exception <br/>-- System.SystemException
<br/>-- System.IO.IOException</td>
<td></td>
</tr>
<tr>
<td>java.lang.NoClassDefFoundError</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>-- java.lang.Error
<br/>--- java.lang.LinkageError</td>
<td>System.TypeLoadException</td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>Review is required to determine if this is the correct exception in
.NET.</td>
</tr>
<tr>
<td>org.junit.AssumptionViolatedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.lang.RuntimeException <br/>----
org.junit.internal.AssumptionViolatedException</td>
<td>NUnit.Framework.InconclusiveException</td>
<td>System.Object <br/>- System.Exception <br/>--
NUnit.Framework.ResultStateException</td>
<td></td>
</tr>
<tr>
<td>java.nio.file.AccessDeniedException</td>
<td>java.lang.Object <br/>- java.lang.Throwable <br/>--
java.lang.Exception <br/>--- java.io.IOException <br/>----
java.nio.file.FileSystemException</td>
<td>System.UnauthorizedAccessException </td>
<td>System.Object <br/>- System.Exception <br/>--
System.SystemException</td>
<td>Special case: Does not subclass IOException in .NET but does in
Java.</td>
</tr>
</table>
</details>
There is a lot of info in the above table, so let me just pick out a few
cases to demonstrate the kind of issues we are facing.
| Exception Type | Issue |
|---|---|
| `java.text.ParseException` | This excpetion was translated to
`FormatException` in some places, `InvalidOperationException` in some places,
and `Excpetion` in other places (there may be more). |
| `java.nio.file.AccessDeniedException` | The corresponding class in .NET,
`System.UnauthorizedAccessException` does not subclass `IOException`. As a
result, catch blocks for `IOException` are missing this one when they should be
catching it. |
| `java.io.FileNotFoundException` and `java.nio.file.NoSuchFileException` |
In .NET there is only 1 exception type for dealing with missing files. However,
in Java these exceptions are also thrown if there is a missing directory. |
| `org.apache.lucene.store.NoSuchDirectoryException` | This class was
excluded from Lucene.NET because .NET already has a
`System.IO.DirectoryNotFoundException`, however it does not subclass
`System.IO.FileNotFoundException`, so we needed to manually duplicate `catch`
blocks everywhere `System.IO.FileNotFoundExeption` was caught and duplicate
error handler logic. |
| `java.lang.OutOfMemoryError` | This excpetion derives from
`java.lang.Error`. This means in .NET when we catch `Exception`, we are
catching this exception when we shouldn't be in many cases and not allowing it
to propagate to its intended handler. |
| `java.lang.AssertionError` | Since in .NET assertions are compiled out of
the release and the behavior of `Debug.Assert()` doesn't consistently throw an
exception that can be caught, an equivalent for this exception wasn't added
until recently and is not in use in every place it was in Lucene. However, it
is also meant to be excluded from exception handling and propagate to the top
of the stack, but wouldn't be if we were using it for the same reason as
`java.lang.OutOfMemoryError`. |
| `java.lang.NullPointerException` | Since in Java the approach is to let
exceptions fly and handle them later, this exception is being caught in Lucene.
However, in most cases when there is a `null` parameter in .NET, the expected
exception is `ArgumentNullException` rather than `NullReferenceException`.
`NullReferenceException` is an unexpected exception in .NET we would expect to
let propagate so we can prevent it from being thrown more easily.
`NullReferenceException` was used in a lot of catch blocks, but this may be
hiding problems in the code where exceptions can be avoided. |
## Challenges
- Fill gaps between Java and .NET exception types in a way that doesn't rely
100% on inheritance, since we have several special cases to deal with in
mulitple places in the application
- Avoid exception translation problems when porting from Java to .NET
- Make it possible to maintain this easily - if we discover a rule about
Java exceptions we missed, it should be possible to add that rule in only 1
place
- Make the code mostly or completely self-documenting so we don't always
have to refer to documentation in order to translate exception types right
## Proposed Solution for Catching Exceptions
We can take advantage of the C# [`when` in a `catch`
statement](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/when#when-in-a-catch-statement)
to make complex rules in catch blocks. In addition, we can use extension
methods on `Exception` with names that match exception groups in Java to apply
the rules specifc to that group.
### Example
```c#
public static class ExceptionExtensions
{
public static bool IsThrowable(this Exception e)
{
return true; // All errors inherit from Throwable in Java
}
public static bool IsError(this Exception e)
{
return e is OutOfMemoryException ||
e is AssertionException ||
e is StackOverflowException;
}
public static bool IsException(this Exception e)
{
return e is Exception && !IsError(e);
}
public static bool IsIOException(this Exception e)
{
return e is IOException || e is UnauthorizedAccessException;
}
public static bool IsFileNotFoundOrNoSuchFileExcetption(this Exception e)
{
return e is FileNotFoundException || e is DirectoryNotFoundException;
}
}
```
#### FileNotFoundException Usage
<details>
<summary>Click to expand</summary>
##### Java
```java
public static boolean slowFileExists(Directory dir, String fileName) throws
IOException {
try {
dir.openInput(fileName, IOContext.DEFAULT).close();
return true;
} catch (NoSuchFileException | FileNotFoundException e) {
return false;
}
}
```
##### C# Before
```c#
public static bool SlowFileExists(Directory dir, string fileName)
{
try
{
dir.OpenInput(fileName, IOContext.DEFAULT).Dispose();
return true;
}
catch (FileNotFoundException)
{
return false;
}
// LUCENENET specific - .NET (thankfully) only has one
FileNotFoundException, so we don't need this
//catch (NoSuchFileException)
//{
// return false;
//}
// LUCENENET specific - since NoSuchDirectoryException subclasses
FileNotFoundException
// in Lucene, we need to catch it here to be on the safe side.
catch (DirectoryNotFoundException)
{
return false;
}
}
```
##### C# After
```c#
public static bool SlowFileExists(Directory dir, string fileName)
{
try
{
dir.OpenInput(fileName, IOContext.DEFAULT).Dispose();
return true;
}
catch (Exception e) when (e.IsFileNotFoundOrNoSuchFileExcetption())
{
return false;
}
}
```
</details>
#### IOException Usage
<details>
<summary>Click to expand</summary>
##### Java
```java
try {
TestUtil.rm(everything);
} catch (IOException e) {
Class<?> suiteClass = RandomizedContext.current().getTargetClass();
if (suiteClass.isAnnotationPresent(SuppressTempFileChecks.class)) {
System.err.println("WARNING: Leftover undeleted temporary files (bugUrl:
"
+ suiteClass.getAnnotation(SuppressTempFileChecks.class).bugUrl() +
"): "
+ e.getMessage());
return;
}
throw e;
}
```
##### C# Before
```c#
try
{
TestUtil.Rm(everything);
}
// LUCENENET specific: UnauthorizedAccessException doesn't subclass
IOException as
// AccessDeniedException does in Java, so we need a special case for it.
catch (UnauthorizedAccessException e)
{
// Type suiteClass =
RandomizedContext.Current.GetTargetType;
// if
(suiteClass.IsAnnotationPresent(typeof(SuppressTempFileChecks)))
// {
Console.Error.WriteLine("WARNING: Leftover undeleted temporary files " +
e.Message);
return;
// }
}
catch (IOException e)
{
// Type suiteClass =
RandomizedContext.Current.GetTargetType;
// if
(suiteClass.IsAnnotationPresent(typeof(SuppressTempFileChecks)))
// {
Console.Error.WriteLine("WARNING: Leftover undeleted temporary files " +
e.Message);
return;
// }
}
```
##### C# After
```c#
try
{
TestUtil.Rm(everything);
}
catch (Exception e) when (e.IsIOException())
{
// Type suiteClass =
RandomizedContext.Current.GetTargetType;
// if
(suiteClass.IsAnnotationPresent(typeof(SuppressTempFileChecks)))
// {
Console.Error.WriteLine("WARNING: Leftover undeleted temporary files " +
e.Message);
return;
// }
}
```
</details>
> **NOTE:** There are dozens of places where we catch `IOExcpetion` that
don't take `UnauthorizedAccessException` into account.
>
> Also, there may be other `IOExcption`-derived types in Java that don't
derive from `IOExcpetion` in .NET. We need to review all of the [direct known
subclasses](https://docs.oracle.com/javase/7/docs/api/java/io/IOException.html)
to be relatively sure we are catching everything that is supposed to be caught.
## Proposed Solution for Throwing Exceptions
Subclass .NET exceptions with internal classes that use Java exception
names. This takes the guesswork out of deciding which excpetion is the
appropriate one to throw in most cases, and we still throw the correct .NET
exception types (that can be caught) to the outside world.
There are several exceptions in Java where in .NET we don't get an
exception, but may get a `false` or `null` return value instead. We can create
fake excpetions for these and mark them with the `[Obsolete]` attribute with a
message indicating how to convert the .NET code.
### Example
```c#
internal class ParseException : FormatException
{
private const string ERROR_OFFSET_NAME = "__errorOffset";
public ParseException() {}
public ParseException(string message) : base(message) {}
public ParseException(string message, Exception innerException) :
base(message, innerException) {}
public int ErrorOffset
{
get => Convert.ToInt32(Data[ERROR_OFFSET_NAME] ?? 0); // Data is
passed on in FormatException so it is accessible even though this class is
internal
set => Data[ERROR_OFFSET_NAME] = value;
}
public override Message => base.Message + ", line: " + ErrorOffset;
}
[Obsolete("This exception is thrown by Reflection code in Java when calling
private/internal members of a Type, but is unnecessary in .NET.")]
internal class PrivilegedActionException : Exception
{
}
[Obsolete(".NET enumerators do not throw when there are no more elements. It
is recommended to convert iterators into enumerators and use a MoveNext()
method that returns false.")]
internal class NoSuchElementException: Exception
{
}
[Obsolete("Reflection doesn't throw exceptions when it cannot locate a type
in .NET, instead it will return null. Do not catch this exception and adjust
the logic to consider a null return value the equivalent of the error
condition.")]
internal class ClassNotFoundException : Exception
{
}
```
#### ParseException Usage
<details>
<summary>Click to expand</summary>
##### Java
```java
if (line.startsWith(KEEPCASE_KEY)) {
String parts[] = line.split("\\s+");
if (parts.length != 2) {
throw new ParseException("Illegal KEEPCASE declaration",
reader.getLineNumber());
}
keepcase = flagParsingStrategy.parseFlag(parts[1]);
}
```
##### C# Before
```c#
if (line.StartsWith(KEEPCASE_KEY, StringComparison.Ordinal))
{
string[] parts = whitespacePattern.Split(line).TrimEnd();
if (parts.Length != 2)
{
throw new FormatException(string.Format("Illegal KEEPCASE
declaration, line {0}", lineNumber));
}
keepcase = flagParsingStrategy.ParseFlag(parts[1]);
}
```
##### C# After
```c#
if (line.StartsWith(KEEPCASE_KEY, StringComparison.Ordinal))
{
string[] parts = whitespacePattern.Split(line).TrimEnd();
if (parts.Length != 2)
{
throw new ParseException("Illegal KEEPCASE declaration") {
ErrorOffset = lineNumber };
}
keepcase = flagParsingStrategy.ParseFlag(parts[1]);
}
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
