You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Stefan Bodewig <bo...@apache.org> on 2021/06/29 20:24:00 UTC
[compress] [Poll] Dealing with uncaught RuntimeExceptions
Hi
I'm sorry, but I'm unable to see what would or would not work for the
people who chimed in. Short of calling for a vote, lets try with a poll
that could show whether there is some sort of solution that is
acceptable to everybody.
Please use +1 to mean "I like this option", +0 to mean "the option is
OK, but I'd prefer a different one", -0 for "I don't like the option but
I can live with it" and -1 for "this option is not acceptable to me.
Options raised during the thread:
(1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
subclass) and throw the IOException
(2) catch only a subset of all RuntimeExceptions, wrap them in an
IOException (possibly a subclass) and throw the IOException - allow
the remaining RuntimeExceptions to fly through
(3) catch all RuntimeExceptions, wrap them in an specific unchecked
exception (which one could be discussed later) and throw this one
(4) don't catch RuntimeExceptions at all, just document broken archives
can cause arbitrary RuntimeExceptions and code that tries to read
archives from untrusted sources is expected to deal with them
itself.
"Just harden all parsers" is a variation of (4) in my view as I don't
believe we would manage to cover all cases no matter how hard and long
we try.
I hope I didn't overlook any.
Thanks
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Stefan Bodewig <bo...@apache.org>.
On 2021-06-29, Stefan Bodewig wrote:
> Options raised during the thread:
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
+1
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
+0
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
-0
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
+0
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gilles Sadowski <gi...@gmail.com>.
Le mer. 30 juin 2021 à 02:32, Bernd Eckenfels <ec...@zusammenkunft.net> a écrit :
>
> I vote for the subclass of IOException - if you care about handling broken archives you would catch it,
Yes, but you can catch a runtime exception all the same.
An application that doesn't want to terminate abruptly must catch
all exceptions at some point.
The distinction between checked and unchecked is related to the
question of "recoverable or not", not about "handling or not".
> if you don’t care you have to handle IO problems anyway.
?
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Bernd Eckenfels <ec...@zusammenkunft.net>.
I vote for the subclass of IOException - if you care about handling broken archives you would catch it, if you don’t care you have to handle IO problems anyway.
Gruß
Bernd
--
https://Bernd.eckenfels.net
________________________________
From: Gilles Sadowski <gi...@gmail.com>
Sent: Wednesday, June 30, 2021 12:33:16 AM
To: Commons Developers List <de...@commons.apache.org>
Subject: Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Le mar. 29 juin 2021 à 22:24, Stefan Bodewig <bo...@apache.org> a écrit :
>
> Hi
>
> I'm sorry, but I'm unable to see what would or would not work for the
> people who chimed in. Short of calling for a vote, lets try with a poll
> that could show whether there is some sort of solution that is
> acceptable to everybody.
>
> Please use +1 to mean "I like this option", +0 to mean "the option is
> OK, but I'd prefer a different one", -0 for "I don't like the option but
> I can live with it" and -1 for "this option is not acceptable to me.
>
> Options raised during the thread:
>
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
-1
[As a matter of principle, we should not force users to handle checked
exceptions; but, as said previously, it can be mandated by design, .e.g.
the (strange) promise that no other exception can arise from calling the
library.]
>
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
-1
[Same reason (it's a variant of the above).]
But how will you choose which should be caught or allowed to fly?
>
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
+0
A sane solution that can satisfy two categories of users:
(A) those who want to easily catch everything that comes out, and
(B) those who want to make sure that no error can go unnoticed.
>
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
+1
>
> "Just harden all parsers" is a variation of (4) in my view as I don't
> believe we would manage to cover all cases no matter how hard and long
> we try.
>
> I hope I didn't overlook any.
>
> Thanks
>
> Stefan
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gilles Sadowski <gi...@gmail.com>.
Le mar. 29 juin 2021 à 22:24, Stefan Bodewig <bo...@apache.org> a écrit :
>
> Hi
>
> I'm sorry, but I'm unable to see what would or would not work for the
> people who chimed in. Short of calling for a vote, lets try with a poll
> that could show whether there is some sort of solution that is
> acceptable to everybody.
>
> Please use +1 to mean "I like this option", +0 to mean "the option is
> OK, but I'd prefer a different one", -0 for "I don't like the option but
> I can live with it" and -1 for "this option is not acceptable to me.
>
> Options raised during the thread:
>
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
-1
[As a matter of principle, we should not force users to handle checked
exceptions; but, as said previously, it can be mandated by design, .e.g.
the (strange) promise that no other exception can arise from calling the
library.]
>
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
-1
[Same reason (it's a variant of the above).]
But how will you choose which should be caught or allowed to fly?
>
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
+0
A sane solution that can satisfy two categories of users:
(A) those who want to easily catch everything that comes out, and
(B) those who want to make sure that no error can go unnoticed.
>
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
+1
>
> "Just harden all parsers" is a variation of (4) in my view as I don't
> believe we would manage to cover all cases no matter how hard and long
> we try.
>
> I hope I didn't overlook any.
>
> Thanks
>
> Stefan
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Oliver Heger <ol...@oliver-heger.de>.
Am 30.06.21 um 14:41 schrieb Gary Gregory:
> On Tue, Jun 29, 2021 at 4:24 PM Stefan Bodewig <bo...@apache.org> wrote:
>>
>> Hi
>>
>> I'm sorry, but I'm unable to see what would or would not work for the
>> people who chimed in. Short of calling for a vote, lets try with a poll
>> that could show whether there is some sort of solution that is
>> acceptable to everybody.
>>
>> Please use +1 to mean "I like this option", +0 to mean "the option is
>> OK, but I'd prefer a different one", -0 for "I don't like the option but
>> I can live with it" and -1 for "this option is not acceptable to me.
>>
>> Options raised during the thread:
>>
>> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
>> subclass) and throw the IOException
>
> -1. There will no end to this and our code will be riddled and
> cluttered with try-catch clauses.
>
>>
>> (2) catch only a subset of all RuntimeExceptions, wrap them in an
>> IOException (possibly a subclass) and throw the IOException - allow
>> the remaining RuntimeExceptions to fly through
>
> -0. Selective implementation might be OK but I prefer throwing
> unchecked exceptions for unrecoverable errors like a negative index.
>
>>
>> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
>> exception (which one could be discussed later) and throw this one
>
> -1. That one does not make sense to me.
>
>>
>> (4) don't catch RuntimeExceptions at all, just document broken archives
>> can cause arbitrary RuntimeExceptions and code that tries to read
>> archives from untrusted sources is expected to deal with them
>> itself.
>
> +0.
>
> Here what I would prefer to see on our APIs:
>
> - Throw IOException where we surface JRE IOException.
> - Throw NPE where we use methods like Objects.requireNonNull().
> - Throw IllegalArgumentException on other garbage API input arguments.
> - Throw IllegalStateException when the archive is broken.
IIUC, the problem is that [compress] states that it throws an
IOException, but in practice can throw arbitrary runtime exceptions as
well. So IMHO, it would be a great improvement for the users to come to
a definition under which circumstances what outcome is expected.
This will probably require some kind of translation from exceptions
thrown during parsing to the exceptions declared by [compress]. How such
a translation then looks like, is certainly a matter of taste. As the
API currently uses IOExceptions anyway, using sub classes of this type
would be fine with me.
In any case, I think we should implement some translation and not let
all kinds of unexpected exceptions fall on our users.
Oliver
>
> Gary
>
>> "Just harden all parsers" is a variation of (4) in my view as I don't
>> believe we would manage to cover all cases no matter how hard and long
>> we try.
>>
>> I hope I didn't overlook any.
>>
>> Thanks
>>
>> Stefan
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Jun 29, 2021 at 4:24 PM Stefan Bodewig <bo...@apache.org> wrote:
>
> Hi
>
> I'm sorry, but I'm unable to see what would or would not work for the
> people who chimed in. Short of calling for a vote, lets try with a poll
> that could show whether there is some sort of solution that is
> acceptable to everybody.
>
> Please use +1 to mean "I like this option", +0 to mean "the option is
> OK, but I'd prefer a different one", -0 for "I don't like the option but
> I can live with it" and -1 for "this option is not acceptable to me.
>
> Options raised during the thread:
>
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
-1. There will no end to this and our code will be riddled and
cluttered with try-catch clauses.
>
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
-0. Selective implementation might be OK but I prefer throwing
unchecked exceptions for unrecoverable errors like a negative index.
>
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
-1. That one does not make sense to me.
>
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
+0.
Here what I would prefer to see on our APIs:
- Throw IOException where we surface JRE IOException.
- Throw NPE where we use methods like Objects.requireNonNull().
- Throw IllegalArgumentException on other garbage API input arguments.
- Throw IllegalStateException when the archive is broken.
Gary
> "Just harden all parsers" is a variation of (4) in my view as I don't
> believe we would manage to cover all cases no matter how hard and long
> we try.
>
> I hope I didn't overlook any.
>
> Thanks
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gilles Sadowski <gi...@gmail.com>.
Le mer. 30 juin 2021 à 11:55, PeterLee <pe...@apache.org> a écrit :
>
> > (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> > subclass) and throw the IOException
>
> -1
>
> I agree with sebb about this option - this may accidentally convert a bug
> into
> something else.
>
> > (2) catch only a subset of all RuntimeExceptions, wrap them in an
> > IOException (possibly a subclass) and throw the IOException - allow
> > the remaining RuntimeExceptions to fly through
>
> +1
>
> It may take a lot of work to get the subset, but I think the work is worthy.
IIUC what has been said, the main "problem" is that (some) callers expect
a single exception (whatever the encountered issue with the input) out of
[Compress]. This option does not do them any good.
The task is ill-defined as long as the "subset" remains undefined. And
furthermore, the same low-level exception could arise from either a bug or
a corrupt input (so, depending which case is uncovered first, bugs could
still go unnoticed).
> > (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> > exception (which one could be discussed later) and throw this one
>
> -1
>
> > (4) don't catch RuntimeExceptions at all, just document broken archives
> > can cause arbitrary RuntimeExceptions and code that tries to read
> > archives from untrusted sources is expected to deal with them
> > itself.
>
> -0
>
>
> > [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by PeterLee <pe...@apache.org>.
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
-1
I agree with sebb about this option - this may accidentally convert a bug
into
something else.
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
+1
It may take a lot of work to get the subset, but I think the work is worthy.
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
-1
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
-0
On Wed, Jun 30, 2021 at 4:24 AM Stefan Bodewig <bo...@apache.org> wrote:
> Hi
>
> I'm sorry, but I'm unable to see what would or would not work for the
> people who chimed in. Short of calling for a vote, lets try with a poll
> that could show whether there is some sort of solution that is
> acceptable to everybody.
>
> Please use +1 to mean "I like this option", +0 to mean "the option is
> OK, but I'd prefer a different one", -0 for "I don't like the option but
> I can live with it" and -1 for "this option is not acceptable to me.
>
> Options raised during the thread:
>
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
>
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
>
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
>
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
>
> "Just harden all parsers" is a variation of (4) in my view as I don't
> believe we would manage to cover all cases no matter how hard and long
> we try.
>
> I hope I didn't overlook any.
>
> Thanks
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Matt Juntunen <ma...@gmail.com>.
Adding my two cents here...
It's very important to me as a user and developer to have informative
errors. If I use compress in my application and a raw NPE or out of bounds
exception flies out of the code with little to no context and makes it out
to the end user, then I am going to be getting a call as to why my code is
broken. It is also going to take me quite a while to track down the source
of the error and will most likely involve me digging into the compress
source code at some point. On the other hand, if the user receives an
exception like Stefan's UnhandledInputException [1] or a
CorruptedArchiveException, then they may try to check the archive itself
first and not even call me, and even if they do call, I'll be able to
troubleshoot much more quickly. That is why I think Stefan's PR (or
something similar) is the best way to go: it adds useful information to the
error that would be very difficult to obtain otherwise.
Regards,
Matt J
[1]
https://github.com/apache/commons-compress/compare/catch-RuntimeExceptions
On Thu, Jul 1, 2021 at 6:04 PM Gilles Sadowski <gi...@gmail.com> wrote:
> > [...]
> > > One is "sorry, this is an invalid archive" while the other is
> "something
> > > > went wrong reading the archive - printStackTrace". If that
> distinction is
> > > > not important to you - so be it. To me that's a big difference.
> > >
> > > Never said such a thing.
> > >
> >
> > Not sure what "thing" you mean exactly but In the very same email you
> wrote:
> >
> > "I don't see why the library should be expected to explain in details
> > why an archive is corrupt; it is good enough that it can detect it..."
>
> Cases in point (example provided by Stefan) is that exceptions are
> raised by bad input, but the exact reason is not always known because
> the processing could continue for some time before the optimistic
> assumption turned out to be wrong.
> For sure, it is better to provide the actual root cause of the failure but
> Stefan indicated that reviewing all the codes would be a lot of work.
> And IIUC, we are talking about the current situation, not the ideal one
> (where the caller could act based on accurate information).
>
> IOW still, what I meant by the above is that this pseudo-code (I have
> no idea how the [Compress] API looks like):
> ---CUT---
> String foo = "This is probably invalid ZIP data";
> String bar = compress.unzip(foo);
> ---CUT---
> could throw a plain IAE, without being required to give an explanation
> of why/how/where the library figured out that something was wrong
> with the input. Providing wrong input is a programming error on the
> caller's part. It's his responsibility to protect against his mistakes.
>
> > > The "sorry, this is an invalid archive" is what makes the situation
> > > > for me "recoverable".
> > >
> > > This is plain wrong:
> >
> >
> > I am not drawing that straight line to preconditions.
> > Flows or processes can also have the attribute of "recoverable".
>
> Not [Compress] responsibility.
>
> > [...]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gilles Sadowski <gi...@gmail.com>.
> [...]
> > One is "sorry, this is an invalid archive" while the other is "something
> > > went wrong reading the archive - printStackTrace". If that distinction is
> > > not important to you - so be it. To me that's a big difference.
> >
> > Never said such a thing.
> >
>
> Not sure what "thing" you mean exactly but In the very same email you wrote:
>
> "I don't see why the library should be expected to explain in details
> why an archive is corrupt; it is good enough that it can detect it..."
Cases in point (example provided by Stefan) is that exceptions are
raised by bad input, but the exact reason is not always known because
the processing could continue for some time before the optimistic
assumption turned out to be wrong.
For sure, it is better to provide the actual root cause of the failure but
Stefan indicated that reviewing all the codes would be a lot of work.
And IIUC, we are talking about the current situation, not the ideal one
(where the caller could act based on accurate information).
IOW still, what I meant by the above is that this pseudo-code (I have
no idea how the [Compress] API looks like):
---CUT---
String foo = "This is probably invalid ZIP data";
String bar = compress.unzip(foo);
---CUT---
could throw a plain IAE, without being required to give an explanation
of why/how/where the library figured out that something was wrong
with the input. Providing wrong input is a programming error on the
caller's part. It's his responsibility to protect against his mistakes.
> > The "sorry, this is an invalid archive" is what makes the situation
> > > for me "recoverable".
> >
> > This is plain wrong:
>
>
> I am not drawing that straight line to preconditions.
> Flows or processes can also have the attribute of "recoverable".
Not [Compress] responsibility.
> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
> > So IllegalArgumentException is not from the early Java days?
>
> It is, so what?
> It's the exception I use 99% of the time (to signal failed precondition)?
> It's a runtime exception.
>
When I quote standard exceptions you said:
"if you quote design decisions that date back from the early Java days"
> There should be a difference of action when the code knows the archive is
> > broken in contrast there being a problem with the code reading it (for
> > whatever reason).
>
> The difference does not entail the checked vs unchecked selection.
>
While I'd prefer "Optional", given the context of backward compatibility
and expectations I believe it does.
> One is "sorry, this is an invalid archive" while the other is "something
> > went wrong reading the archive - printStackTrace". If that distinction is
> > not important to you - so be it. To me that's a big difference.
>
> Never said such a thing.
>
Not sure what "thing" you mean exactly but In the very same email you wrote:
"I don't see why the library should be expected to explain in details
why an archive is corrupt; it is good enough that it can detect it..."
> The "sorry, this is an invalid archive" is what makes the situation
> > for me "recoverable".
>
> This is plain wrong:
I am not drawing that straight line to preconditions.
Flows or processes can also have the attribute of "recoverable".
I don't see why the library should be expected to explain in details
> why an archive is corrupt; it is good enough that it can detect it, and
> very good if it can protect the caller from himself
If you don't see the quality difference between "this archive is invalid"
and "could not read the archive" there is really no point to make here.
I think this has again gotten to the point of no longer being useful for
the wider audience.
So never mind - I'll go back to lurking.
cheers,
Torsten
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gilles Sadowski <gi...@gmail.com>.
Le jeu. 1 juil. 2021 à 11:31, Torsten Curdt <tc...@vafer.org> a écrit :
>
> > I'm not sure what you refer to exactly,
>
>
> The various links from
>
> https://docs.oracle.com/javase/7/docs/api/java/io/IOException.html
>
> or even
>
> https://docs.oracle.com/javase/7/docs/api/java/lang/Exception.html
>
>
> > but I noted already that not all
> > references are equal, especially if you quote design decisions that date
> > back from the early Java days (when every textbook touted checked
> > exceptions as _the_ solution to robust programming).
> >
>
> So IllegalArgumentException is not from the early Java days?
It is, so what?
It's the exception I use 99% of the time (to signal failed precondition)?
It's a runtime exception.
My point, all along, is that I have a hard time finding a single proper
use of checked exceptions (in libraries such as we deal with here),
not counting the cases where one was right, at the time, to mimic
JDK usage of checked exceptions.
I said from the outset of this discussion that if the purpose is to stay
true to the initial design decisions, it may be OK to continue with
"IOException".
As you indicate below, the currently advertised way to handle
recovery is with "Optional".
>
> > ...
> > Ask yourself how the programmer will handle the exception ? If the
> > programmer
> > can’t do better than e.printStackTrace() or throw new AssertionError(),
> > then, an
> > unchecked exception is called for.
> >
>
> And I guess that's the point - the programmer can do better.
>
> There should be a difference of action when the code knows the archive is
> broken in contrast there being a problem with the code reading it (for
> whatever reason).
The difference does not entail the checked vs unchecked selection.
>
> One is "sorry, this is an invalid archive" while the other is "something
> went wrong reading the archive - printStackTrace". If that distinction is
> not important to you - so be it. To me that's a big difference.
Never said such a thing.
> The "sorry, this is an invalid archive" is what makes the situation
> for me "recoverable".
This is plain wrong: From the POV of (any) library, failure to satisfy
a precondition is not recoverable, by definition of "precondition".
[Here there is often no simple check, but that does not change the
principle.]
I don't see why the library should be expected to explain in details
why an archive is corrupt; it is good enough that it can detect it, and
very good if it can protect the caller from himself (by throwing *any*
exception rather than crashing the JVM or giving root access. ;-)
> For me "recoverable" does not mean running the same code with the same
> input twice and expecting a different result.
In effect this is exactly what one does in the only case I know of,
where a checked exception may be appropriate (as per the two
conditions mentioned earlier), that is when resources are requested
that may be temporarily unavailable.
Gilles
> If you continue to read your link to "Effective Java", the use of
> "Optional" is suggested as the proper alternative to a checked exception.
> And to be honest I'd be all for that - on a new code base. But you'd still
> need figure out the "canCompute" to make the distinction between the two
> states.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
> I'm not sure what you refer to exactly,
The various links from
https://docs.oracle.com/javase/7/docs/api/java/io/IOException.html
or even
https://docs.oracle.com/javase/7/docs/api/java/lang/Exception.html
> but I noted already that not all
> references are equal, especially if you quote design decisions that date
> back from the early Java days (when every textbook touted checked
> exceptions as _the_ solution to robust programming).
>
So IllegalArgumentException is not from the early Java days?
> ...
> Ask yourself how the programmer will handle the exception ? If the
> programmer
> can’t do better than e.printStackTrace() or throw new AssertionError(),
> then, an
> unchecked exception is called for.
>
And I guess that's the point - the programmer can do better.
There should be a difference of action when the code knows the archive is
broken in contrast there being a problem with the code reading it (for
whatever reason).
One is "sorry, this is an invalid archive" while the other is "something
went wrong reading the archive - printStackTrace". If that distinction is
not important to you - so be it. To me that's a big difference.
The "sorry, this is an invalid archive" is what makes the situation
for me "recoverable".
For me "recoverable" does not mean running the same code with the same
input twice and expecting a different result.
If you continue to read your link to "Effective Java", the use of
"Optional" is suggested as the proper alternative to a checked exception.
And to be honest I'd be all for that - on a new code base. But you'd still
need figure out the "canCompute" to make the distinction between the two
states.
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gilles Sadowski <gi...@gmail.com>.
Le jeu. 1 juil. 2021 à 02:27, Torsten Curdt <tc...@vafer.org> a écrit :
>
> >
> > "If there is runtime exception there is a bug in the code"
> >
> > I don't think that's correct because IllegalArgumentException is a
> > RuntimeException.
>
>
> I have a hard time following that causality. The way I've seen this
> exception used mainly is in case of program errors, not in case of input
> validations. Similar to
>
>
> https://docs.oracle.com/javase/7/docs/api/java/lang/IllegalStateException.html
>
> It's interesting that the IllegalArgumentException from the java API is
> meant to prove a point while the ones I referenced didn't seem to count ;)
I'm not sure what you refer to exactly, but I noted already that not all
references are equal, especially if you quote design decisions that date
back from the early Java days (when every textbook touted checked
exceptions as _the_ solution to robust programming).
From[1] J. Bloch's "Effective Java" (3rd ed.):
---CUT---
Item 71 : Avoid unnecessary use of checked exceptions
Methods throwing checked exceptions can’t be used directly in streams.
An unchecked exception use is appropriate unless both of these conditions
are met :
* The exceptional condition cannot be prevented by proper use of the API
* The programmer using the API can take some useful action once confronted
with the exception
Ask yourself how the programmer will handle the exception ? If the programmer
can’t do better than e.printStackTrace() or throw new AssertionError(), then, an
unchecked exception is called for.
---CUT---
Case discussed here obviously fails the first condition: Passing a non-corrupt
archive would not trigger an exception.[2]
In my view of what is "recoverable", the second condition also fails (but YMMV):
If the library throws an exception because it cannot make sense of the archive,
the caller still has no clue how to recover (except trying again by
assuming that
the error will automagically fix itself).
Gilles
[1] Copied from
https://ahdak.github.io/blog/effective-java-part-9/
(contains more items about exception handling).
[2] Unless there is a bug in [Compress], which also mandates a runtime exception
(to be reported here in order to be fixed).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
>
> "If there is runtime exception there is a bug in the code"
>
> I don't think that's correct because IllegalArgumentException is a
> RuntimeException.
I have a hard time following that causality. The way I've seen this
exception used mainly is in case of program errors, not in case of input
validations. Similar to
https://docs.oracle.com/javase/7/docs/api/java/lang/IllegalStateException.html
It's interesting that the IllegalArgumentException from the java API is
meant to prove a point while the ones I referenced didn't seem to count ;)
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Gary Gregory <ga...@gmail.com>.
"If there is runtime exception there is a bug in the code"
I don't think that's correct because IllegalArgumentException is a
RuntimeException. See
https://docs.oracle.com/javase/8/docs/api/java/lang/IllegalArgumentException.html
Gary
On Wed, Jun 30, 2021, 16:50 Torsten Curdt <tc...@vafer.org> wrote:
> >
> > It is not the job of commons-compress to stop you from using a
> > corrupt archive. If you choose to do so, then ...
> >
>
> I don't think that's what it is.
> That would be more like getting an exception and continuing walking through
> the archive.
>
> If there is a parse exception there is a bug in the input.
> If there is runtime exception there is a bug in the code.
>
> If the user does a try/catch for a runtime exception there is no indication
> which of the two it is.
>
> I personally find it a shame if we were to blur those lines.
>
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
>
> It is not the job of commons-compress to stop you from using a
> corrupt archive. If you choose to do so, then ...
>
I don't think that's what it is.
That would be more like getting an exception and continuing walking through
the archive.
If there is a parse exception there is a bug in the input.
If there is runtime exception there is a bug in the code.
If the user does a try/catch for a runtime exception there is no indication
which of the two it is.
I personally find it a shame if we were to blur those lines.
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Jochen Wiedmann <jo...@gmail.com>.
On Tue, Jun 29, 2021 at 10:24 PM Stefan Bodewig <bo...@apache.org> wrote:
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
Strongly in favour of this one. It is the obvious implementation
strategy. If a user gets an Exception, and uses the archive anyways,
think of the old Unix saying:
It is not UNIX’s job to stop you from shooting your foot. If you
so choose to do so, then it is UNIX’s job to deliver Mr. Bullet to Mr
Foot in the most efficient way it knows.
Translated:
It is not the job of commons-compress to stop you from using a
corrupt archive. If you choose to do so, then ...
Jochen
--
Look, that's why there's rules, understand? So that you think before
you break 'em.
-- (Terry Pratchett, Thief of Time)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by sebb <se...@gmail.com>.
On Tue, 29 Jun 2021 at 21:24, Stefan Bodewig <bo...@apache.org> wrote:
>
> Hi
>
> I'm sorry, but I'm unable to see what would or would not work for the
> people who chimed in. Short of calling for a vote, lets try with a poll
> that could show whether there is some sort of solution that is
> acceptable to everybody.
>
> Please use +1 to mean "I like this option", +0 to mean "the option is
> OK, but I'd prefer a different one", -0 for "I don't like the option but
> I can live with it" and -1 for "this option is not acceptable to me.
>
> Options raised during the thread:
>
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
-0
Too easy to accidentally convert a bug into something that looks like
an input error.
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
+1
I think we should catch known RTEs and wrap with an IEO giving the
probable cause.
This may take a few iterations to catch them all, but the fuzzer
should help here.
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
-1
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
-0
> "Just harden all parsers" is a variation of (4) in my view as I don't
> believe we would manage to cover all cases no matter how hard and long
> we try.
>
> I hope I didn't overlook any.
>
> Thanks
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
> (1) catch all RuntimeExceptions, wrap them in an IOException (possibly a
> subclass) and throw the IOException
>
+0
> (2) catch only a subset of all RuntimeExceptions, wrap them in an
> IOException (possibly a subclass) and throw the IOException - allow
> the remaining RuntimeExceptions to fly through
>
+1
>
> (3) catch all RuntimeExceptions, wrap them in an specific unchecked
> exception (which one could be discussed later) and throw this one
>
-1
>
> (4) don't catch RuntimeExceptions at all, just document broken archives
> can cause arbitrary RuntimeExceptions and code that tries to read
> archives from untrusted sources is expected to deal with them
> itself.
>
-1
Re: [compress] [Poll Non Result] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
> >> That certainly doesn't prevent anybody else from trying to find a
> >> compromise :-)
>
> > It feels like Optionals could be a compromise.
>
> I must admit I've lost track of the later discussion threads. If you
> mean that we'd return Optional<> results, this would become an entirely
> different API.
>
It certainly does. And I personally don't think it's the best course of
action at this stage - unless someone is eager to work on a new major
version. But if you look for a compromise it seem to be the only one
acceptable to both "camps".
I'd very much like us to get to a compromise that helps our users and
> doesn't force us to randomly fix RuntimeExceptions that slipped through
> overly optimistic parser code.
>
Well, I am fine with the solution you proposed :)
cheers,
Torsten
Re: [compress] [Poll Non Result] Dealing with uncaught RuntimeExceptions
Posted by Stefan Bodewig <bo...@apache.org>.
On 2021-07-01, Torsten Curdt wrote:
>> That certainly doesn't prevent anybody else from trying to find a
>> compromise :-)
> It feels like Optionals could be a compromise.
I must admit I've lost track of the later discussion threads. If you
mean that we'd return Optional<> results, this would become an entirely
different API.
I'd very much like us to get to a compromise that helps our users and
doesn't force us to randomly fix RuntimeExceptions that slipped through
overly optimistic parser code.
So if you want to explore the Optionals idea further this would be
great, but I doubt enough people have seen it.
Thanks
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] [Poll Non Result] Dealing with uncaught RuntimeExceptions
Posted by Torsten Curdt <tc...@vafer.org>.
> That certainly doesn't prevent anybody else from trying to find a
> compromise :-)
>
It feels like Optionals could be a compromise.
[compress] [Poll Non Result] Dealing with uncaught RuntimeExceptions
Posted by Stefan Bodewig <bo...@apache.org>.
Hi all
there isn't a single option that hasn't at least received two -1s with
eight people indicating their preference. So neither option seems to be
an option that could lead to a compromise.
With this I run out of ideas and will rest my case and not try to find a
generic solution - but rather try to get 1.21 out with no changes in
this area.
That certainly doesn't prevent anybody else from trying to find a
compromise :-)
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org