Gulp: Direct path (non-glob) .src streams should fail on file not found

[contra]

Example: https://gist.github.com/anonymous/9774298

If it isn't a glob it should probably fail if the file isn't found.

Reported by heyladies via IRC

[contra]

This could also probably apply to globs like somefolder/** when somefolder doesn't exist

I think this should be behind a flag because not everyone cares about this

[callumacrae]

I can do this. Would listening for "nomatch" (edit: wrong library, whoops. Same idea, though) and making sure the glob is just a string that doesn't contain an asterisk be enough, for the first case?

[contra]

@callumacrae You should rely on the internal representation from node-glob to determine if it's a flat file or not. It will make an array of all path segments and each glob gets turned into a regex, so you can iterate the array and if there are no regexs then it can be assumed it's a flat file.

[contra]

Sample of me messing with the internal representation to find the base path for vinyl files https://github.com/wearefractal/glob2base/blob/master/index.js

[laurelnaiad]

What is the purpose of having .src fail if a directory in a glob doesn't exist or a specific path (without wildcards) doesn't match? It seems inconsistent with the model of globbing returning all matching files -- 0 is still a valid number of matching files, and a pattern with no wildcards or that specifies a non-existent directory is still a pattern.

What if your plugin is combining two sources, one may have no files and the other may have some (e.g. the using-multiple-sources recipe). You'd have to code around the error thrown from .src()...

[contra]

@stu-salsbury This is about flat paths, not globs.

Example: gulp.src('file.js')
Example 2: gulp.src(['file.js', 'file.jpg'])

[contra]

This also brings up a good point performance wise. We shouldn't even be passing flat file paths to node-glob which then walks directories, we should just scoop them off the fs normally since we already have the path and know it isn't a glob.

[laurelnaiad]

I understand that the paths don't have wildcards, but to my mind they're still globs... but aside from semantics, I think that when you have an array of files, some of which might not exist, you may still want the plugin (or other code) that's receiving the stream(s) to operate on the files that are there, if there are any, or to behave in a plugin/other-code-specific way rather than having to catch an error in the gulpfile.

Also, in the case of the example in the recipe, this would just break:

    return streamqueue({ objectMode: true },
        gulp.src('foo/*'),
        gulp.src('bar/*')
    )

What is the reason for throwing an error if a file or directory isn't found? Is it solving a particular problem?

[contra]

@stu-salsbury No, that wouldn't break because those are globs not direct paths to files.

[contra]

@stu-salsbury Also FYI gulp.src maintains order now so you don't need to use streamqueue

You could rewrite that example as gulp.src(['foo/*', 'bar/*'])

[laurelnaiad]

I was under the impression that the scope of this is also directories that don't exist... but my point is in general.... what problem is being solved by throwing an error? If a plugin's purpose is to add files to a pipe, starting with zero is a perfectly reasonable place to start. Heck, the first plugin after .src might be one that is lazy-creating files that aren't present... I guess I'm not understanding the value in throwing an error. Why not just have a plugin whose purpose is to throw an error if it gets 0 files prior to the stream ending instead of jumping to the conclusion that we're always in an error state when a file or directory doesn't exist.

[contra]

@stu-salsbury It's misleading to have something like gulp.src('file.js').pipe(uglify()).pipe(gulp.dest('folder')) do nothing but say everything is okay. Technically it makes sense but from a user perspective it didn't do what they told it to do.

Also, disregard my thoughts about directories.

[laurelnaiad]

I agree and I get your point -- I would definitely want that pipeline to inform me that 0 files were processed. Where I'm taking issue is that I think it is up to uglify to decide whether or not 0 files is ok, whether to log it, emit an error, throw an error, whatever... and perhaps .src, at a certain log level, should also log something about it. I think there's plenty of room between apparent success (neither .src nor uglify makes any mention that 0 files were processed) vs. just crashing a pipeline altogether. Perhaps an option to src might enable the new error throwing feature (or is it error emiting?) to be turned off?

[laurelnaiad]

Basically, I don't want to be baby-sat into place where I have to write error handling code to accomplish my goal. Errors should be reserved for things that are definitely errors, not just things that are probably errors -- especially when you can't easily catch (err) {} them away, such as in the case of the pipeline function chain.

[laurelnaiad]

I guess I just figured out that I'd prefer to see a warn-level log entry from .src:

WARN: 'file.js' not found

and if the file being absent is a possible valid case, I could squelch the warning from my gulpfile.

[contra]

@stu-salsbury If added it would definitely be behind an option. I totally get your concerns. Also RE your comments about error handling being a pain in the ass - that is going to be fixed in gulp 4. One .on(error) handler or a domain will fix everything then

[laurelnaiad]

I'd make the case that the default should in fact be to error. I think it is the exceptional case where it's ok that a hardcoded (non-wildcarded) file path points to a non-existent file. I was saying I think there should be an option in the second param to .src() to disable the error.

[callumacrae]

Imo, the default behaviour should be to log "WARN: 'file.js' not found" to the console, and there should be a flag available (and an option to gulp.src) to make it throw an error instead of just logging a warning.

Also, gulp.src should have an option to suppress the warning entirely.

[milyord]

Any word on where this is going?

[PowerKiKi]

Agreeing with @callumacrae on this. I am actually in favor of fail-fast system, so I would even consider to make it an error.

My use case is when concatenating specific files. If I explicitly src() a non-glob path to a single filename, I expect the file to exists on disk. And I expect the whole task to fail if it doesn't exist. Why would I src() a non-glob file that does not exist in the first place ?

With the current situation my build breaks because the file src()ed does not exist and thus does not get concatenated correctly. All of that because of a typo in the filename that I should have been warned about.

[greim]

Just got bit by this and it led me here. A task failed silently and it took hours to track down and realize nothing was happening in a gulp pipeline. The name gulp.src() doesn't make it clear to a neophyte such as myself that it's a glob, even if that's the intended semantics. I'm with @stu-salsbury on this one; make it an error when non-globs return nothing. Maybe have a gulp.glob() that's super-lenient about there being zero matches.

[contra]

How does gulp.src('file.js', {strict: false}) sound, with true being the default? This will error on 0 files when the given param is not a glob

[PowerKiKi]

As long as the default value is strict, it sounds good to me.

[callumacrae]

I'd prefer it to be the default and have an allowEmpty option.

[Spy-Seth]

Is this issue still alive ? I'm right now dealing with this case.

For me options is a good proposal. With strict = true and allowEmpty = false by default.

Maybe a third options to configure the error behaviour: onError = "warn|error|notice"

[henriknorberg]

As a temporary hack, you can fs.statSync the files before you gulp.src 'em:

function statAll(files) {
  files.forEach(fs.statSync) //will trow an error if file not found
}

[datfinesoul]

This would be really nice to have. I like Spy-Seth's idea with the {strict: true, allowEmpty: false} options to control this.

[PowerKiKi]

@datfinesoul, @Spy-Seth, I don't understand the meaning of the multiple options strict and allowEmpty. What each of those control ?

I believe a single option should be enough to control that behavior. Same goes for onError option, I think it's over-bloating the API for a "feature" that I find hard to find an actual use for.

I would stick with original proposition of {strict: false} and defaulting to true.

[Spy-Seth]

The allowEmpty option have the aim to protect from empty srcset. For exemple, if this path exist this/path/extis/*.jsbut give no result: with allowEmpty = false this will result to an error. You expected to have file, but something is wrong.

But this is a must have. The important here is to have the strict option.

[contra]

I'm short on time right now but I will take a PR for this on the vinyl-fs repo if somebody wants to work on one

[doberkofler]

+1 (when using complex build environment this would really help a lot)

[shouze]

:+1:

[dmythro]

Hi, any news about this? Wold be really helpful.

[dmythro]

At the moment I use simple check with glob for that and a simple Gulp task. Something like:

var glob = require('glob');
// ...

// synchronously, because have to leave one message when all done and fine,
// or error messages for each mask
var files = glob.sync(mask);
if (!files || !files.length) {
  // handle empty list
}

But in this case have to copy-paste within projects, most of them have complex build and several file lists, which I actually have to store to check before build.
So, I'd really appreciate an option to warn about all empty masks - in my case few vendor files weren't added into repo.

[patrickheeney]

+1 This would save a lot of time debugging.

[AliMD]

:+1:
any temporary way to check all paths correct ?

[callumacrae]

Programmatically? Not too easily, you'd have to use through2 or another streams library. Visually, you can use gulp-debug and it will output the number of files passed through.

[callumacrae]

@contra: I've got some time and want to have another crack at this, but I'm not sure how I would do it. Would I want to start by checking for nomatch below here, and then making a change to vinyl-fs once glob-stream has been changed, or is there an easier way to do it?

I'm on IRC for the next hour or so if it's easier to discuss there.

[contra]

@callumacrae Yep check for nomatch in glob-stream and throw an error if the glob is not plural, unless an option is specified as false (default will be that this behavior occurs). This and https://github.com/wearefractal/glob-stream/blob/master/index.js#L30 should be a .once

[contra]

Also on IRC if you need a hand

[contra]

So this just got merged into glob-stream, going to publish and an npm update should have you on your way

[callumacrae]

I'll send a PR to add docs to gulp in a bit

[phated]

@callumacrae's PR for documenting this was merged 6 days ago. The 4.0 branch pulls from vinyl-fs master currently (will update that to be published soon). Closing. Many thanks to @callumacrae for landing this.