Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to ignore frontend execution error (non-zero exit code) #850

Open
vincentmigot opened this issue Sep 23, 2019 · 7 comments
Open

Allow to ignore frontend execution error (non-zero exit code) #850

vincentmigot opened this issue Sep 23, 2019 · 7 comments

Comments

@vincentmigot
Copy link

Hi,
First thanks for this awesome plugin :)

State:
I currently use it with yarn to build some JS project and I want to check existence of outdated libraries during build BUT I don't want the build to fail if there are (because it's not always a possibility to update to the latest version)

My problem is that "yarn outdated" command as well as "npm outdated" return a non-zero exit status if there is any outdated library inside my package.json.

So if I add the following execution part in my pom.xml, it break the build if any outaded library is found:

<execution>
    <id>check outdated libraries which break build if not all libraries are up to date</id>
    <goals>
        <goal>yarn</goal>
    </goals>
    <configuration>
        <arguments>outdated</arguments>
    </configuration>
</execution>

What I wish:
I wan't to be able to add an option into an execution part to ignore non-zero exit like that:

<execution>
    <id>check outdated libraries</id>
    <goals>
        <goal>yarn</goal>
    </goals>
    <configuration>
        <arguments>outdated</arguments>
        <ignoreErrors>true</ignoreErrors>
    </configuration>
</execution>

Workaround found so far:
After looking into the plugin code I found that I can do as follow:

  • Specify the phase <phase>test</phase> into the execution block
  • Specify the option <testFailureIgnore>true</testFailureIgnore> in configuration block

But this is very ugly because it log an error "There are test failures." which is not accurate at all and print an ugly non-sense stack trace in the middle of the build log.

Suggestion:
I have made a pull request (link to come) which add an available option <ignoreErrors>true</ignoreErrors> (false by default) for the configuration block.
I indicate in the javadoc that this option is NOT RECOMMENDED as well as testFailureIgnore option.

If you have any better solution I'm open to any suggestion and have some time to work on it if it requires any further development.

Thanks
Vincent Migot

@vincentmigot
Copy link
Author

Here is the pull request:
#851

@vincentmigot
Copy link
Author

I forget to mention that my pull request just log a message (INFO level) "There are ignored errors during task: " followed by the error message (but no stack trace) in case of any exception during the execution unit.

@eirslett
Copy link
Owner

I suggest that you solve this in node.js land. It might be useful if you ever switch build systems (to Gradle maybe?)

Check this stackoverflow answer: https://stackoverflow.com/questions/30341113/npm-scripts-ignore-errors and the exitzero package, then instead of the yarn goal, you use the npm goal, with something like npm run yarn-check-outdated, which is a script that you define in your package.json? I don't think this is a use case that is common enough that it has to be supported in the Maven plugin.

@vincentmigot
Copy link
Author

Thanks for your reply.

I can understand your point, the thing is that I'm running a multi-module project and the yarn "outdated" task (or npm outaded which has the same behavior) is located in the parent project of the module.

So your solution implies that any sub-module which has JS to build will have to implement this custom script in its package.json which is not very convenient.
The module system I build is made for third party users to easily add new modules adding JS modules to my main application so I don't want to add any specific requirement for their package.json unless it's really necessary.

I understand that is really specific to my needs but this behavior of yarn and npm outdated task make them unusable in practical with your plugin.

So if you don't wan't to add this feature I probably will just drop the functionality as it's just a (very convenient) help for maintainers in my case.

@vincentmigot
Copy link
Author

Just a side note, my first try was to set "arguments" to <arguments>outdated || exit 0</arguments> but in that case the command doesn't print anything.

Do you thing it's possible to find a solution in that direction (without adding any more option) ?

Thanks again for your time

@eirslett
Copy link
Owner

The normal solution is to do || exit 0 like you tried. I'm not sure why it doesn't output anything. The main problem is that it won't work on Windows (I think).

@brenuart
Copy link

brenuart commented Apr 14, 2020

I'm facing the same kind of issue. We configured the plugin to ignore errors during test execution like this:

<properties>
    <maven.test.failure.ignore>true</maven.test.failure.ignore>
</properties>

...

<execution>
    <id>npm-test</id>
    <phase>test</phase>
    <goals>
        <goal>npm</goal>
    </goals>
    <configuration>
        <arguments>run-script test --no-progress</arguments>
    </configuration>
</execution>

The reason is we want the build to continue further and we feed Jenkins (and Sonar) with the test reports afterwards.

Thanks to this configuration, the maven plugin doesn't throw any exception when run-script test returns with a non-zero code and the maven build proceeds to completion without any problem.
However, it does log the exception received from the command launcher at ERROR level which produces a stacktrace in the maven console.

A stacktrace in the logs is often the indication of something bad with the build itself and something we should worry about. However, in this case, the non-zero return code is "expected" and I would not expect such stacktrace but (may be) only an indication that the command "seems" to have failed.

Moreover, the stacktrace doesn't have any value in troubleshooting the reason why the command failed: it always reports the same information (non-zero return code).

I understand your point about "this issue must be solved on the JS side" (with something like || exit 0) - but in this case, what is the value of the failure ignore config parameter?

My feeling is that:

  • when maven.test.failure.ignore=true, then log a warning about the failed execution without any stacktrace
  • otherwise, the failure is unexpected and log it in ERROR with a stacktrace

@eirslett What do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants