Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review exception types thrown and in try/catch blocks #446

Closed
NightOwl888 opened this issue Mar 20, 2021 · 0 comments · Fixed by #476
Closed

Review exception types thrown and in try/catch blocks #446

NightOwl888 opened this issue Mar 20, 2021 · 0 comments · Fixed by #476
Assignees

Comments

@NightOwl888
Copy link
Contributor

NightOwl888 commented Mar 20, 2021

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 behavior 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.

Click for details
Java Exception Type Java Superclass Types .NET Exception Type .NET Superclass Types Notes
java.io.IOException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
System.IO.IOException System.Object
- System.Exception
-- System.SystemException
java.lang.ArrayIndexOutOfBoundsException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
-- java.lang.IndexOutOfBoundsException
System.IndexOutOfRangeException System.Object
- System.Exception
-- System.SystemException
System.ArgumentOutOfRangeException System.Object
- System.Exception
-- System.SystemException
--- System.ArgumentException
We are currently not catching this exception in all places where IndexOutOfRangeException is caught.
java.lang.StringIndexOutOfBoundsException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
-- java.lang.IndexOutOfBoundsException
System.IndexOutOfRangeException System.Object
- System.Exception
-- System.SystemException
System.ArgumentOutOfRangeException System.Object
- System.Exception
-- System.SystemException
--- System.ArgumentException
We are currently not catching this exception in all places where IndexOutOfRangeException is caught.
java.lang.IndexOutOfBoundsException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.IndexOutOfRangeException System.Object
- System.Exception
-- System.SystemException
System.ArgumentOutOfRangeException System.Object
- System.Exception
-- System.SystemException
--- System.ArgumentException
We are currently not catching this exception in all places where IndexOutOfRangeException is caught.
java.text.ParseException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
System.FormatException System.Object
- System.Exception
-- System.SystemException
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.
System.InvalidOperationException System.Object
- System.Exception
-- System.SystemException
There are some places where ParseException errors were translated to InvalidOperationException - these should probably be changed to FormatException.
System.Exception System.Object There are some places where ParseException errors were translated to Exception - these should probably be changed to FormatException.
java.lang.Exception java.lang.Object
- java.lang.Throwable
System.Exception System.Object
java.lang.CloneNotSupportedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
None
java.io.FileNotFoundException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
System.IO.FileNotFoundException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
System.IO.DirectoryNotFoundException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
java.lang.NoSuchMethodException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.ReflectiveOperationException
System.MissingMethodException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
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.
java.lang.reflect.InvocationTargetException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.ReflectiveOperationException
System.TargetInvocationException System.Object
- System.Exception
-- System.ApplicationException
java.lang.IllegalAccessException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.ReflectiveOperationException
System.Exception System.Object
- System.Exception
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.
System.UnauthorizedAccessException System.Object
- System.Exception
-- System.SystemException<
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.
java.lang.IllegalArgumentException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.ArgumentException System.Object
- System.Exception
-- System.SystemException
System.ArgumentOutOfRangeException System.Object
- System.Exception
-- System.SystemException
--- System.ArgumentException
Some guard clauses have been added with this type. Need to check to ensure places that catch IndexOutOfRangeException and this exception are correct.
System.ArgumentNullException System.Object
- System.Exception
-- System.SystemException
--- System.ArgumentException
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.
java.lang.NullPointerException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.NullReferenceException System.Object
- System.Exception
-- System.SystemException
It might be best to convert everywhere this is used to ArgumentNullException so we don't swallow NullReferenceException unintentionally and hide problems.
java.lang.InstantiationException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.ReflectiveOperationException
System.TypeInitializationException System.Object
- System.Exception
-- System.SystemException
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.
java.lang.UnsupportedOperationException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.NotSupportedException System.Object
- System.Exception
-- System.SystemException
java.lang.Throwable java.lang.Object System.Exception System.Object
java.lang.RuntimeException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
System.Exception System.Object
java.lang.ClassNotFoundException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.ReflectiveOperationException
None 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.
java.util.NoSuchElementException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
None .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.
org.xml.sax.SAXException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Sax.SaxException System.Object
- System.Exception
Exists only in the Support folder of Lucene.Net.Benchmark.
java.lang.InterruptedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
System.Threading.ThreadInterruptedException System.Object
- System.Exception
-- System.SystemException
org.apache.lucene.util.ThreadInterruptedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
-- System.SystemException
System.Threading.ThreadInterruptedException System.Object
- System.Exception
-- System.SystemException
This class exists to convert the exception into RuntimeException. It is possible we have missed something fundamental here.
org.apache.lucene.benchmark.bytask.feeds.NoMoreDataException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Lucene.Net.Benchmark.ByTask.Feeds.NoMoreDataException System.Object
- System.Exception
java.lang.NumberFormatException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
---- java.lang.IllegalArgumentException
System.FormatException System.Object
- System.Exception
-- System.SystemException
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.
org.apache.commons.compress.compressors.CompressorException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
None .NET and SharpZipLib do not throw when instantiating a compression stream.
java.util.zip.DataFormatException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
None We aren't catching an exception for compression streams, but this needs review.
java.lang.SecurityException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
-- java.lang.RuntimeException
System.Security.SecurityException System.Object
- System.Exception
-- System.SystemException
java.nio.file.NoSuchFileException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
-- java.io.IOException
-- java.nio.file.FileSystemException
System.IO.FileNotFoundException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
System.IO.DirectoryNotFoundException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
org.apache.lucene.store.NoSuchDirectoryException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
---- java.io.FileNotFoundDirectory
System.IO.DirectoryNotFoundException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
java.lang.OutOfMemoryError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
--- java.lang.VirtualMachineError
System.OutOfMemoryException System.Object
- System.Exception
-- System.SystemException
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.
org.apache.lucene.store.AlreadyClosedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
--- java.lang.IllegalStateException
System.ObjectDisposedException System.Object
- System.Exception
-- System.SystemException
-- System.InvalidOperationException
In prior versions of Lucene.NET, there were at least attempts to use both ICloseable and IDisposable. We need to revisit this.
java.nio.charset.CharacterCodingException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
All Perhaps catching all exceptions is not the right decision here.
org.apache.lucene.util.BytesRefHash.MaxBytesLengthExceededException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
Lucene.Net.Util.BytesRefHash.MaxBytesLengthExceededException System.Object
- System.Exception
org.apache.lucene.search.CollectionTerminatedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
Lucene.Net.Search.CollectionTerminatedException System.Object
- System.Exception
java.util.concurrent.ExecutionException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
All Needs review.
java.nio.BufferUnderflowException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
J2N.IO.BufferUnderflowException System.Object
- System.Exception
java.lang.ClassCastException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
J2N.InvalidCastException System.Object
- System.Exception
-- System.SystemException
org.apache.lucene.store.LockObtainFailedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
Lucene.Net.Store.LockObtainFailedException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
java.security.PrivilegedActionException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
None This exception is used to work around a bug in Java in MMapDirectory.
java.nio.channels.OverlappingFileLockException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
---- java.lang.IllegalStateException
None NativeFSLockFactory was redesigned for .NET and this exception was factored out.
java.lang.AssertionError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
System.InvalidOperationException System.Object
- System.Exception
-- System.SystemException
Lucene.Net.Diagnostics.AssertionException was added later. Should we migrate everything over to it?
Lucene.Net.Diagnostics.AssertionException System.Object
- System.Exception
This is only used in Debugging.Assert() method calls.
java.lang.Error java.lang.Object
- java.lang.Throwable
System.Exception System.Object
java.io.EOFException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
-- java.io.IOException
System.IO.EndOfStreamException System.Object
- System.Exception
-- System.SystemException
--- System.IO.IOException
java.lang.IllegalStateException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.InvalidOperationException System.Object
- System.Exception
-- System.SystemException
org.apache.lucene.index.IndexFormatTooOldException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
--- org.apache.lucene.index.CorruptIndexException
Lucene.Net.Index.IndexFormatTooOldException System.Object
- System.Exception
-- System.SystemException
-- System.IO.IOException
org.apache.lucene.index.CorruptIndexException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
--- org.apache.lucene.index.CorruptIndexException
Lucene.Net.Index.IndexFormatTooOldException System.Object
- System.Exception
-- System.SystemException
-- System.IO.IOException
org.apache.lucene.index.TestCrashCausesCorruptIndex.CrashingException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
Lucene.Net.Index.TestCrashCausesCorruptIndex.CrashingException System.Object
- System.Exception
Only exists in one test class.
org.apache.lucene.util.SetOnce.AlreadySetException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
---- java.lang.IllegalStateException
Lucene.Net.Util.AlreadySetException System.Object
- System.Exception
-- System.SystemException
-- System.InvalidOperationException
org.apache.lucene.store.MockDirectoryWrapper.FakeIOException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
Lucene.Net.Store.FakeIOException System.Object
- System.Exception
-- System.SystemException
-- System.IO.IOException
org.apache.lucene.search.TimeLimitingCollector.TimeExceededException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
Lucene.Net.Search.TimeLimitingCollector.TimeExceededException System.Object
- System.Exception
org.antlr.runtime.RecognitionException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Antlr.Runtime.RecognitionException System.Object
- System.Exception
java.lang.StackOverflowError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
--- java.lang.VirtualMachineError
System.StackOverflowException System.Object
- System.Exception
-- System.SystemException
Uncatchable in .NET. Microsoft recommends breaking out of recursive calls with a max value setting.
java.lang.ArithmeticException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.ArithmeticException System.Object
- System.Exception
-- System.SystemException
org.apache.lucene.search.BooleanQuery.TooManyClauses java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
Lucene.Net.Search.BooleanQuery.TooManyClausesException System.Object
- System.Exception
org.apache.lucene.queryparser.classic.ParseException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Lucene.Net.QueryParsers.Classic.ParseException System.Object
- System.Exception
org.apache.lucene.queryparser.classic.TokenMgrError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
Lucene.Net.QueryParsers.Classic.TokenMgrError System.Object
- System.Exception
java.util.MissingResourceException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
System.Resources.MissingManifestResourceException System.Object
- System.Exception
-- System.SystemException
org.apache.lucene.queryparser.flexible.standard.parser.ParseException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- org.apache.lucene.queryparser.flexible.core.QueryNodeException
---- org.apache.lucene.queryparser.flexible.core.QueryNodeParseException
Lucene.Net.QueryParsers.Flexible.Standard.Parser.ParseException System.Object
- System.Exception
-- Lucene.Net.QueryParsers.Flexible.Core.QueryNodeException
--- Lucene.Net.QueryParsers.Flexible.Core.QueryNodeParseException
org.apache.lucene.queryparser.flexible.standard.parser.ParseException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Lucene.Net.QueryParsers.Flexible.Standard.Parser.ParseException System.Object
- System.Exception
-- Lucene.Net.QueryParsers.Flexible.Core.QueryNodeException
org.apache.lucene.queryparser.flexible.standard.parser.TokenMgrError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
Lucene.Net.QueryParsers.Flexible.Standard.Parser.TokenMgrError System.Object
- System.Exception
org.apache.lucene.queryparser.surround.parser.TokenMgrError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
Lucene.Net.QueryParsers.Surround.Parser.TokenMgrError System.Object
- System.Exception
org.apache.lucene.queryparser.surround.ParseException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Lucene.Net.QueryParsers.Surround.ParseException System.Object
- System.Exception
org.apache.lucene.queryparser.xml.ParserException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
Lucene.Net.QueryParsers.Xml.ParserException System.Object
- System.Exception
org.apache.lucene.replicator.SessionExpiredException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
Lucene.Net.Replicator.SessionExpiredException System.Object
- System.Exception
-- System.SystemException
-- System.IO.IOException
java.lang.NoClassDefFoundError java.lang.Object
- java.lang.Throwable
-- java.lang.Error
--- java.lang.LinkageError
System.TypeLoadException System.Object
- System.Exception
-- System.SystemException
Review is required to determine if this is the correct exception in .NET.
org.junit.AssumptionViolatedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.lang.RuntimeException
---- org.junit.internal.AssumptionViolatedException
NUnit.Framework.InconclusiveException System.Object
- System.Exception
-- NUnit.Framework.ResultStateException
java.nio.file.AccessDeniedException java.lang.Object
- java.lang.Throwable
-- java.lang.Exception
--- java.io.IOException
---- java.nio.file.FileSystemException
System.UnauthorizedAccessException System.Object
- System.Exception
-- System.SystemException
Special case: Does not subclass IOException in .NET but does in Java.

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 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

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 IsFileNotFoundOrNoSuchFileException(this Exception e)
    {
        return e is FileNotFoundException || e is DirectoryNotFoundException;
    }
}

FileNotFoundException Usage

Click to expand
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
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
public static bool SlowFileExists(Directory dir, string fileName)
{
    try
    {
        dir.OpenInput(fileName, IOContext.DEFAULT).Dispose();
        return true;
    }
    catch (Exception e) when (e.IsFileNotFoundOrNoSuchFileException())
    {
        return false;
    }
}

IOException Usage

Click to expand
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
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
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;
    //                    }
}

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 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

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

Click to expand
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
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
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]);
}
@NightOwl888 NightOwl888 added this to the 4.8.0 milestone Mar 20, 2021
@NightOwl888 NightOwl888 modified the milestones: 4.8.0, 4.8.0-beta00015 Apr 1, 2021
@NightOwl888 NightOwl888 self-assigned this Apr 1, 2021
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…ng Java exception types to their closest .NET equivalent. See apache#446.
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…erted them to use our IsThrowable() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…d them to use our IsError() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…erted them to use our IsException() extension method (see apache#446). Added overload for printStackTrace() that accepts a TextWriter so we can specify another output.
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…nverted them to use our IsIOException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…eption and converted them to use our IsIllegalArgumentException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…gException and converted them to use our IsUnsupportedEncodingException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…ion and converted them to use our IsIllegalStateException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
… converted them to use our IsParseException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…ion and converted them to use our IsNumberFormatException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…n || FileNotFoundException and converted them to use our IsNoSuchFileExceptionOrFileNotFoundException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…onException and converted them to use our IsUnsupportedOperationException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…onverted them to use our IsEOFException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…eption and converted them to use our IsMissingResourceException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…or and converted them to use our IsNoClassDefFoundError() extension method (see apache#446). Also, changed some catch blocks that were using TypeLoadException to use IsClassNotFoundException(), as there are places where they overlap.
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…n and converted them to use our IsArithmeticException() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…laced the exception that is thrown in Lucene.NET with Lucene.Net.Diagnostics.AssertionException (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
… converted them to use our IsAssertionError() extension method (see apache#446).
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 14, 2021
…eption and converted them to use our IsIndexOutOfBoundsException() extension method (see apache#446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…nvalidOperationException instead of Exception (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ements to ServiceConfigurationError.Create() (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…Exception when the set is readonly, not InvalidOperationException to match .NET collection behavior (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…umer(): Throw NotSupportedException rather than InvalidOperationException (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…c(): Throw AssertionError rather than InvalidOperationException (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ertionError rather than InvalidOperationException (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…nError rather than InvalidOperationException (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ements to IllegalStateException.Create() (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ts to UnsupportedOperationException.Create() (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ception rather than InvalidOperationException (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ception rather than InvalidOperationException (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…constructors Obsolete with instructions how to convert (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…d documentation to explain how to convert (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…d StringIndexOutOfBoundsException with static factory methods and added documentation to explain how to convert (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ents into AlreadyClosedException.Create() (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…ption whereever Lucene thorws ParseException (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
… correspond to RuntimeException in Java (see #446).
NightOwl888 added a commit that referenced this issue Apr 26, 2021
…processors back to using FormatException, because that is the expected parse error in .NET (see #446)
NightOwl888 added a commit that referenced this issue Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant