[rfc][icedtea-web] tuning of properties loading

[Andrew Azores]

On 01/23/2014 11:18 AM, Jiri Vanek wrote:
>
>>           } catch (IOException e) {
>> +            if (mandatory){
>> +                throw new 
>> ConfigurationException(e.getMessage());//unluckily
>> ConfigurationException can not have cause
>> +            }
>>
>>
>> ConfigurationException does have initCause(Throwable), does this not 
>> match what you need? It gets
>> this from its superclass NamingException which implements Throwable.
>
> Ah I overlooked!. Tahnx. Fixed.

I still see this:

+            if (mandatory){
+                throw new 
ConfigurationException(e.getMessage());//unluckily 
ConfigurationException can not have cause
+            }

why not use it here as well?

Also, this is just a style thing and feel free to ignore, but:

+                ConfigurationException ce = new 
ConfigurationException("Invalid url to system properties, which are 
mandatory");
+                ce.initCause(e);
+                throw ce;

could also simply become:

+                throw new ConfigurationException("Invalid url to system 
properties, which are mandatory").initCause(e);

since Throwable#initCause(Throwable) is defined to return "this" at the end.

>>
>> +                //to be sure - we MUST die
>> -http://docs.oracle.com/javase/6/docs/technotes/guides/deployment/deployment-guide/properties.html 
>>
>> +            }catch(Throwable t){
>> +                //all exceptions are causing InstantiatizationError 
>> so this do it much more readble
>> + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, t);
>> + OutputController.getLogger().log(OutputController.Level.WARNING_ALL,
>> getMessage("RFailingToDefault"));
>> +                //try to survive this unlikely exception
>> +                config.resetToDefaults();
>>
>>
>> Hmm, what's the motivation behind adding this? What is there that's 
>> causing InstantiationErrors [1],
>> and if these occur, should it be trying to survive? Usually you don't 
>> want to be catching Errors
>> [2], so catch-all Throwable is even worse than catch-all Exception.
>
> a) the NoClassDefFoundError error is fatal and non recoverable
> b) no solid info *why* it happened  appeared (consumed any 
> NoClassDefFoundError?)
> c) we can run pretty well with defaults (and msut warn user - I'm 
> thinking also about poping up an dialogue in non headless mode, but 
> I'm afraid that it cnabe to early to (maybe another chnageset)
>
> I do not insist on throwable, If you wish I can lower to general 
> exception or whatever you wont. But defintley do not wont this static 
> block to throw something
>

Well, NoClassDefFoundError is a a subclass of Error, not Exception, so 
catching general Exception will still not catch this! If you want this 
method to absolutely never throw anything without first catching it on 
its own, then Throwable is the only choice you have.

>>
>> [1] 
>> http://docs.oracle.com/javase/7/docs/api/java/lang/InstantiationError.html
>
> Ugh sorry - it is java.lang.NoClassDefFoundError: Could not initialize 
> class 
> net.sourceforge.jnlp.runtime.JNLPRuntime$DeploymentConfigurationHolder 
> what is thrown when static initializer throw some excption out.

Ah okay this makes sense, I understand the motivation now. Catching all 
Throwable still sounds kind of scary to me though, because this includes 
all kinds of Error subclasses that are really not meant to be 
recoverable. Are you sure you want to be consuming and ignoring 
VirtualMachineError, AssertionError, and LinkageError?

Thanks,

-- 
Andrew A