> So ideally Compress will return a CE for parsing errors, and that CE
> should give some context to the error.
> Care must be taken however not to catch every RTE, lest this hide a
coding bug.

+1 for this.

Lee

On Tue, Jun 29, 2021 at 7:57 PM sebb <seb...@gmail.com> wrote:

> On Tue, 29 Jun 2021 at 12:16, Gary Gregory <garydgreg...@gmail.com> wrote:
> >
> > The best reason to create a new IO exception is if you are going to catch
> > it instead of the original and recover differently. I don't think that's
> > the case here, there is no "recovery" possible on a corrupted archive.
> For
> > my money, the exception can remain unchecked as it is unrealistic that
> > recovery is not possible. We can throw a better unchecked exception with
> a
> > better message at best. A plain IOException is possible and some folks
> > favor that but again, recovery is not possible on broken archives.
>
> Recovery may not be possible for a single archive, but the caller may
> want to process multiple archives.
> (Or potentially try again, if the error was caused by a transient error
> [1])
>
> So ideally Compress will return a CE for parsing errors, and that CE
> should give some context to the error.
> Care must be taken however not to catch every RTE, lest this hide a coding
> bug.
>
> It may take a few iterations to catch and wrap all the possible RTEs,
> but the end result would be a more robust and easier to use component.
>
> Sebb
> [1] It's not unknown for files to be temporarily locked, or for
> network errors to occur, etc.
>
> > Gary
> >
> > On Tue, Jun 29, 2021, 03:54 Miguel Munoz <swingguy1...@yahoo.com
> .invalid>
> > wrote:
> >
> > > Catching all RuntimeExceptions and wrapping them in an IOException
> looks
> > > like the cleanest solution. RuntimeExceptions usually mean bugs, so if
> the
> > > archive code is throwing them due to a corrupted archive, it makes
> sense to
> > > wrap it in a checked exception. I would like to suggest creating a new
> > > class looking something like this:
> > >
> > > public class CorruptedArchiveException extends IOException { }
> > > In fact, since we will only be generating them in response to a
> > > RuntimeException, we may want to make sure all constructors require an
> > > RuntimeException as a parameter, to guarantee that we wrap the original
> > > unchecked exception.
> > > — Miguel Muñoz
> > > ———
> > > 4210 Via Arbolada #226Los Angeles, CA 90042
> > > 323-225-7285
> > > –
> > >
> > > The Sun, with all those planets going around it and dependent on it,
> can
> > > still ripen a vine of grapes like it has nothing else to do in the
> world.
> > >   — Galileo
> > >
> > >     On Monday, June 28, 2021, 06:52:02 AM PDT, Gilles Sadowski <
> > > gillese...@gmail.com> wrote:
> > >
> > >  Le lun. 28 juin 2021 à 08:52, Stefan Bodewig <bode...@apache.org> a
> > > écrit :
> > > >
> > > > On 2021-06-27, Gilles Sadowski wrote:
> > > >
> > > > > Le dim. 27 juin 2021 à 21:15, Stefan Bodewig <bode...@apache.org>
> a
> > > écrit :
> > > >
> > > > >> As I said, we can as well document that each method could throw
> > > > >> arbitrary RuntimeExceptions, but I don't believe we can list the
> kinds
> > > > >> of RuntimeExceptions exhaustively
> > > >
> > > > > Why not?
> > > > > Listing all runtime exceptions is considered part of good
> > > > > documentation.
> > > >
> > > > Because we do not know which RuntimeExceptions may be caused by an
> > > > invalid archive.
> > > >
> > > > Most of the RuntimeException happen because our parsers believe in a
> > > > world where every archive is valid. For example we may read a few
> bytes
> > > > that are the size of an array and then create the array without
> checking
> > > > whether the size is negative and a NegativeArraySizeException occurs.
> > > >
> > > > As we haven't considered the case of a negative size in code, we
> also do
> > > > not know this exception could be thrown. If we had considered the
> case
> > > > of negative sizes, the parser could have avoided the exception
> > > > alltogether. This is what I meant with
> > > >
> > > > >> - if we knew which exceptions can be thrown, then we could as well
> > > > >> check the error conditions ourselves beforehand.
> > > >
> > > > Our parsers could of course be hardened, but this is pretty
> difficult to
> > > > do years after they've been written and would certainly miss a few
> > > > corner cases anyway.
> > >
> > > Thank you for the example.  I now understand that the aim is not
> > > to increase the number of precondition checks but only to ensure
> > > that a single exception can escape from calls with "undefined"
> > > behaviour whenever some precondition is not fulfilled.
> > >
> > > IIUC, the situation where the library assumes some precondition
> > > but does not check it, and continues its operations until something
> > > unexpected occurs, would be described by "IllegalStateException".
> > > [A very much "non-recoverable" state, as the library doesn't know
> > > the root cause (which may be a long way from where the symptom
> > > appeared).]
> > >
> > > In principle, it looks interesting information for users to be able to
> > > distinguish between a known failure (i.e. the library knows the root
> > > cause) and an unknown failure (?).
> > >
> > > > And then there is a certain category of exceptions thrown by Java
> > > > classlib methods we invoke. We've just added a catch block around
> > > > java.util.zip.ZipEntry#setExtra because certain invalid archives
> cause a
> > > > RuntimeException there - and if I remember correctly a
> RuntimeException
> > > > the method's javadoc doesn't list.
> > >
> > > It shows that similar JDK functionality can indeed throw a runtime
> > > exception when it gets unexpected garbage.  What is their rationale
> > > for choosing this or "IOException" (I have no idea)?
> > >
> > > Regards,
> > > Gilles
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > > For additional commands, e-mail: dev-h...@commons.apache.org
> > >
> > >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Reply via email to