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

[Andrew Azores]

On 01/22/2014 01:37 PM, Jiri Vanek wrote:
> By hitting https://bugzilla.redhat.com/show_bug.cgi?id i realized, 
> that we made an serious error by not dying on ConfigureException.
> This patch is fixing it. It also fix an bug, when 
> deployment.system.config.mandatory is true, and invlaid url is proposed.
> It also add few cosmetic changes
>  - few docs
>  - better error reporting from loading the global config
>  - better handling of non fatal (==non ConfigureException) ) exception
>    - logged
>    - defautls are used
>    - if it is not processed, then instantiation error is thrown 
> instead, which is not much helpful...
>
> The reproducers for this are hardly to be possible, as both global 
> files needs root permissions. However some unitsts will be hopefully 
> possible.
>
> Thanx,
>   J.

+            if (systemPropertiesMandatory == true){
+                throw new ConfigurationException("Invalid url to system properties, which are mandatory");


" == true" ? ;)

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

+                //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.

[1] 
http://docs.oracle.com/javase/7/docs/api/java/lang/InstantiationError.html
[2] http://docs.oracle.com/javase/7/docs/api/java/lang/Error.html

Thanks,

-- 
Andrew A