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