[jira] [Commented] (HADOOP-9447) Configuration to include name of failing file/resource when wrapping an XML parser exception

["Harsh J (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-9447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13708002#comment-13708002 ] 

Harsh J commented on HADOOP-9447:
---------------------------------

Can you please explain why we're throwing a previously logged-only exception here? Would these not be mildly incompatible changes? I feel they should go in separately, as they have nothing to do with this JIRA's summary.

{code}
         LOG.warn("Unexpected SecurityException in Configuration", se);
+        throw new RuntimeException("Unexpected SecurityException in Configuration:"
+            + se, se);
{code}

Likewise for:
{code}
+        throw new RuntimeException("Failed to set setXIncludeAware(true) for parser "
+            + docBuilderFactory + ":" + e, e);
{code}

Other comments/nits:
- (Optional) Capitalize sentences:
{code}
+   * [V]erify that when an XML parse exception is raised (and wrapped),
+   * the wrapper Exception includes the name of the resource at fault
{code}
- Test is good. Test failure message could be more helpful for easier debugging of regressions in future. Perhaps you could say "Filename was not found in message: <msg>" instead of just "wrong error text <msg>".
{code}
+      assertTrue("wrong error text "+ msg,
+          msg.contains(fileResource.getName()));
+    }
{code}
- Likewise for {{fail("Expected a failure");}}, which could even be "Expected a failure due to a premature input file."
                
> Configuration to include name of failing file/resource when wrapping an XML parser exception
> --------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-9447
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9447
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: conf
>    Affects Versions: 3.0.0
>            Reporter: Steve Loughran
>            Priority: Trivial
>         Attachments: HADOOP-9447-2.patch, HADOOP-9447.patch, HADOOP-9447-v2.1.patch, HADOOP-9447-v3.patch, HADOOP-9447-v4.patch
>
>
> Currently, when there is an error parsing an XML file, the name of the file at fault is logged, but not included in the (wrapped) XML exception. If that same file/resource name were included in the text of the wrapped exception, people would be able to find out which file was causing problems

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira