[MRESOURCES-268] - Report path of offending file on copy/filter error. #15
Conversation
sebthom
changed the title
| @@ -110,7 +110,8 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt | |||
| } | |||
| catch ( IOException e ) | |||
| { | |||
| throw new MavenFilteringException( e.getMessage(), e ); | |||
| throw new MavenFilteringException( ( filtering ? "filtering " : "copying " ) + from.getPath() + " to " | |||
There was a problem hiding this comment.
What is the purpose to duplicate information like here?
There was a problem hiding this comment.
What do you mean with duplicate information? When maven-resources-plugin currently fails, it only displays a cryptic error message like [ERROR] Failed to execute goal org.apache.maven.plugins:maven-resources-plugin:3.2.0:resources (default-resources) on project test: Input length = 1 -> [Help 1] without any context information. Even when using mvn -e the displayed stacktrace does not show any information about the file that failed during the operation.
There was a problem hiding this comment.
The the root cause does not bubble up, then this is crap. Likely the log message is not ok. You should not try for this the symptom here. Please look in MRESOURCES where this exception is handled.
There was a problem hiding this comment.
The maven resources plugin is delegating the iteration over files to MavenResourcesFiltering, thus is not aware of the file for which filtering fails and therefore cannot report the file path.
Adding the file paths to the error message here has the advantage that all potential code paths to DefaultMavenFileFilter will be covered.
There was a problem hiding this comment.
To illustrate the effect of this PR some screenshots:
- error message before this PR
- error message with this PR
|
@khmarbaise @hboutemy any thoughts? |
|
I starred this, will pick up when I can unless someone else beats me in. |
Changes DefaultMavenFileFilter#copyFile to contain the path to the offending file as part of the thrown MavenFilteringException. Without this change end-users won't see which file caused the error.
|
I pushed a change since apparently not only checked IOExceptions but also intenral runtime exceptions (e.g. IllegalArgumentExcpetion) are thrown since version 3.2.0 of the maven-resource-plugin when a filtered file has an unexpected encoding. Here is real world example. A submodule of a very large multi-artifact/module enterprise application. Without this PR it have would been very laborious (and maybe impossible for the average developer) to discover the root cause. Before this PR: After this PR: |
Changes DefaultMavenFileFilter#copyFile to contain the path to the offending file as part of the thrown MavenFilteringException. Without this change end-users won't see which file caused the error. Closes #15
|
I see this pull request being referenced from the Apache Maven Resource Plugins changelog: https://blogs.apache.org/maven/entry/apache-maven-resources-plugin-version1 I would like to know if this fixes the What is the current status of the issue right now? |
Changes
DefaultMavenFileFilter#copyFileto contain the path to the offending file as part of the thrownMavenFilteringException.Without this change end-users won't see which file caused the error.