Reviewed and Redesigned Exception Handling (closes #446) #476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ugh] attribute to all assertion methods so the debugger stops in the code that fails the assert not inside of the assert method
…or testing internal (affects only .NET Framework)
…ng Java exception types to their closest .NET equivalent. See apache#446.
… by the new exception types (much of this is just to keep tests green but will be removed again after the transition to the new exception handling).
… scanning code in one place.
…rrected this exception type, but marked it internal, since we only want users to catch ObjectDisposedException
…erted them to use our IsThrowable() extension method (see apache#446).
…d them to use our IsError() extension method (see apache#446).
…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.
…nverted them to use our IsIOException() extension method (see apache#446).
…use defensive checks to avoid IndexOutOfRangeException for control flow
…erTest: Added comment about removal of several lines of code that were there to work around JRE bugs.
…xtDoc(): Added comment about refactoring the method to avoid IndexOutOfRangeException
…:GetParent(): Changed logic to use proper guard clauses and changed exception type from IndexOutOfRangeException to ArgumentOutOfRangeException to be consistent with .NET.
…lementation of EnsureOpen() and refactored guard clauses to throw .NET argument exceptions and defensively check for out of range issues instead of catching exceptions and re-throwing them as a different exception.
…cumentation and changed implementation to correctly throw ArgumentOutOfRangeException in cases where it is pertinent.
… about SpatialContextFactory throwing ArgumentNullException rather than NullReferenceException
…to use ArgumentNullException instead of NullReferenceException and fixed other guard clauses that throw ArgumentOutOfRangeException to show the correct messages.
… clause and throw ArgumentNullException
…ArgumentNullException rather than NullReferenceException
…o throw ArgumentNullException rather than NullReferenceException
…lause to throw an ArgumentNullException rather than a NullReferenceException
…eateNode(): Changed guard clause to throw ArgumentNullException instead of NullReferenceException
…SyntaxImpl::ReplaceIgnoreCase(): Changed guard clause to throw ArgumentNullException rather than NullReferenceException
…eption messages to match the variable name they refer to, since it had been renamed previously
…hrow InvalidOperationException instead of NullReferenceException when QueryNodeProcessor.PostProcessNode returns null
…nvalidOperationException rather than NullReferenceException when either GetFormatter() or GetScorer() return null.
…nted out catch blocks that were only used to work around Java issues that don't exist in .NET.
…cumentation to explain how to convert (see apache#446).
…d documentation to explain how to convert (see apache#446).
…d StringIndexOutOfBoundsException with static factory methods and added documentation to explain how to convert (see apache#446)
…ents into AlreadyClosedException.Create() (see apache#446)
…o NumberFormatException.Create() (see apache#446)
…s into OutOfMemoryError.Create() (see apache#446)
… constructors (see apache#446).
…we are not rethrowing inside of the catch block.
…stead of Exception
…CodePages dependency for .NET Core 2.1, as it is no longer a dependency of J2N.
…ption whereever Lucene thorws ParseException (see apache#446)
… correspond to RuntimeException in Java (see apache#446).
…ad catch block to catch types that correspond to RuntimeException using our IsRuntimeException() extension method.
…rks that we test on.
…ests.AllProjects.csproj
…ed line number in ParseException
…processors back to using FormatException, because that is the expected parse error in .NET (see apache#446)
…ow ArgumentNullException (see apache#446)
…aining why the code is commented and updated it with information about ExceptionExtensions
…ng() to build wrapped exception to prevent unnecessary stack trace overhead
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #446.
This centralizes exception handling so it includes/excludes the corresponding exception types as in Java. There were also several bugs found where the wrong exception type was either being caught or thrown, and fixing them makes the tests run more consistently and may have even improved performance overall.
Full tests were added for all known exception types in .NET, NUnit, and Lucene.NET so we have confirmation that our error handlers will only catch the exception types they are supposed to for the general exception types:
Some of the exceptions (such as
NullReferenceException,IndexOutOfRangeException, andFormatException) were factored out by checking to ensure the condition that causes the exception cannot occur.Also, the
ExceptionDispatchInfoclass has been applied to Lucene's centralized error handling and background processing so we always get a full stack trace from the point where the exception began rather than a partial clue from where the exception was rethrown.