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

Shouldn't an error be thrown if concat can't find an input file? #17

Closed
zakdances opened this issue Mar 13, 2013 · 14 comments
Closed

Shouldn't an error be thrown if concat can't find an input file? #17

zakdances opened this issue Mar 13, 2013 · 14 comments

Comments

@zakdances
Copy link

Currently it just silently fails.

@shama
Copy link
Member

shama commented Mar 13, 2013

It should issue a warning: https://github.com/gruntjs/grunt-contrib-concat/blob/master/tasks/concat.js#L40 Does it not for you?

@zakdances
Copy link
Author

I messed around with my paths so they're incorrect and I'm not seeing any "not found" warnings in the log after build...

@shama
Copy link
Member

shama commented Mar 13, 2013

Can you post your gruntfile and output?

@zakdances
Copy link
Author

gruntfile: https://gist.github.com/zakdances/af5b7feacfde32f5c063

output: http://pastebin.com/QKEAVDP4

gist when I tried to paste my output, so I used pastebin

@shama
Copy link
Member

shama commented Jul 10, 2013

Hey sorry for the super late reply on this. I think you just need to set the nonull option to true: http://gruntjs.com/configuring-tasks#files In order to get the matching pattern to not ignore non-existent files.

@webberig
Copy link

I've had the same problem.
By default, missing files are skipped silently. If you add the nonull option, you get a warning. However, Grunt still continues and JS files are still being generated.

This can cause problems in automated builds, because the build task doesn't fail and it deploys a broken application.

Can I suggest the following:

  • Deprecate the nonull option and show warnings anyway (I don't see why anyone would want to let this pass silently)
  • Have a new option that aborts the task so the automated build will detect a failure

@johnhunter
Copy link

See Issue #15

I have a fork that will abort on missing files. Haven't had the time to sort out tests for it so have not issued a pull request.
https://github.com/Huddle/grunt-contrib-concat/#failonmissing

@shama
Copy link
Member

shama commented Aug 8, 2013

FWIW, we get requests both ways on this. Some think tasks should display a warning, some think it should fail and some even think it should pass silently. See gruntjs/grunt-contrib-less#65

I think we need to draft a contrib task error handling guide, we all discuss it, merge it into the docs and normalize how things like this are handled across the contrib tasks.

@johnhunter
Copy link

Sounds good. I can see use cases for silent, warning and fail depending on the task.

Thanks.

@johnhunter
Copy link

The problem I've had with automated builds (Teamcity in my case) is not so much that the grunt tasks continue, but that the build runner doesn't recognise grunt warnings as a sign that something has failed. Exiting with < 0 is a blunt way to communicate and as @shama points out users have different needs. I'm not sure this is a problem grunt should solve.

I took another approach by hooking into grunt log.warn calls and generating a specifically formatted message that the build runner recognises as an error. A more elegant solution might be through events - if grunt emitted log events perhaps. In the meantime the plugin for Teamcity is at https://npmjs.org/package/grunt-teamcity

@keenmedia
Copy link

It should be "grunt.fail.warn" instead of "grunt.log.warn". A warning without failure is useless. You could silently fail by leaving the 'nonull' option off, or running grunt with the --force flag.

@keenmedia
Copy link

I am working around this issue by running the following task before concat.


  grunt.registerTask('warn-fail', 'Fail on warning log', function() {
    var log = grunt.log;
    var _warn = log.warn;
    log.warn = function() {
      _warn.apply(log, arguments);
      grunt.fail.warn("Warning log has triggered failure");
    };
  });

@shama
Copy link
Member

shama commented Dec 16, 2013

Ideally it should be configured by the end user. But rather then implementing yet another option™ I think we should do it right and have it configurable in the logging. prolog will be the library that will replace the grunt logging and will let us do that and more.

@thomasphorton
Copy link

👍

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

7 participants