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
