Re: [compress] [Poll] Dealing with uncaught RuntimeExceptions

[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

[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

[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

[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

[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

[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

[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