[RFC][icedtea-web] Launch errors are not being printed to terminal.

[Danesh Dadachanji]

On 13/03/12 05:21 PM, Omair Majid wrote:
> On 03/13/2012 03:57 PM, Danesh Dadachanji wrote:
>> >  On 12/03/12 08:55 PM, Omair Majid wrote:
>>> >>  On 03/12/2012 10:59 AM, Danesh Dadachanji wrote:
>>>> >>>  I thought so too until I checked where printMessage() was called from.
>>>> >>>  See launchWarning in GuiLaunchHandler[1], it explicitly calls
>>>> >>>  DefaultLaunchHandler.printMessage(). I didn't look into why it was doing
>>>> >>>  that so if you think that's a bug, I can look into it further.
>>>> >>>
>>> >>
>>> >>  Assuming that we want to print the stack trace only in -verbose mode,
>>> >>  how does the attached patch look? (I did get a little side tracked and
>>> >>  fixed problems in printing the exceptions too).
>>> >>
>> >
>> >  This is way better than my patch! One question, why did you choose to
>> >  print the entire exception as opposed to just the cause? I realize
>> >  icedtea-web does it that way and it may even be better but I found that
>> >  when I did that to test, the cause was repeated.
>> >
> I am not entirely clear on this. Do you mean the stack trace was
> repeated? As in it had duplicate "Caused by" lines?
>
>>> >>  I can rework this if others still think printing the entire stack trace
>>> >>  in -headless mode is a good idea.
>> >
>> >  No I think you have the right idea. I completely forgot about the "Show
>> >  Details" button which can be seen as somewhat similar to supplying
>> >  -verbose. I'm fine with the one-liner in output.
>> >
>> >
>> >  Another thing, did you intentionally leave out the part that prints the
>> >  exception to stderr with -verbose in GuiLaunchHandler?
> Doh. Sorry; it was a mistake.
>
>> >  If not, can you
>> >  include it in this patch?
> Done. How does the attached patch look?
>

Great stuff, one more nitpick below and then it's fine with me.

> diff --git a/netx/net/sourceforge/jnlp/GuiLaunchHandler.java b/netx/net/sourceforge/jnlp/GuiLaunchHandler.java
> --- a/netx/net/sourceforge/jnlp/GuiLaunchHandler.java
> +++ b/netx/net/sourceforge/jnlp/GuiLaunchHandler.java

[snip]

> @@ -71,6 +77,9 @@
>                   BasicExceptionDialog.show(exception);
>               }
>           });
> +        if (JNLPRuntime.isDebug()) {
> +            exception.printStackTrace(outputStream);

Could you change that to printMessage(exception); instead? I think 
consistency here between what is printed to terminal between GUI and 
headless runs is better. We might base future tests off misinterpretations!
> +        }
>       }
>

Also, you forgot to include AbstractLaunchHandler. =)

Cheers,
Danesh