-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Gulp 4: gulp.series repeats error log #1330
Comments
Nope. This is an artifact of us having to propagate any error that happens in the execution of the series but also logging the error when they occur (incase someone is using the |
In the current architecture, is it possible to turn off propagation when we know there is no need to? |
No |
This is actually quite bothersome. I have several nested tasks, in series and parallel, and some errors that I get are propagated and logged up to 4 or 5 times. I think Gulp should probably be catching those errors behind the scenes and output them only once. |
@adambuczynski I agree. It's kind of useless to see the same error multiple times. There is no need for the series/parallel task to report an error, because by definition the actual task noticed the error and (hopefully) reported it. |
The error object is taken over by upper tasks. (undertaker/lib/helpers/createExtensions.js)
function createExtensions(ee) {
...
error: function(error, storage) {
...
error.logged = true; // Added
}
};
(gulp-cli/lib/versioned/^4.0.0/log/events.js)
function logEvents(gulpInst) {
...
gulpInst.on('error', function(e) {
if (e.error.logged) { return; } // Added
...
});
} or (undertaker/lib/helpers/createExtensions.js)
function createExtensions(ee) {
...
error: function(error, storage) {
if (error.logged) { return; } // Added
error.logged = true; // Added
...
}
}; or (gulp-cli/lib/versioned/^4.0.0/log/events.js)
function logEvents(gulpInst) {
...
gulpInst.on('error', function(e) {
if (e.error.logged) { return; } // Added
e.error.logged = true; // Added
...
});
} |
Implementation in latest referenced PR. |
@aparajita yep, just need to take time to think through the correct and best implementation so we don't ship technical debt |
@phated Yes, I don't like the solution in my post above, too. However, in now-and-later (next and handler functions in map.js and mapSeries.js), all extension's error processes of upper tasks are called when an error caused. And the error object is the only object which is shared among those tasks. |
@sttk I've already committed a change better than the example you provided. The only place this matters is in the CLI and the underlying libraries should not be changed in their behavior. |
@phated OK. I didn't conceive any more solutions and I also think your implementation is better. |
Shouldn't this be closed? |
@aparajita I'm still working on releasing gulp-cli version 1.1 - it will be closed after release. |
gulp-cli v1.1.0 has been published with this change. You need to have the latest undertaker installed as a dependency of gulp to have everything working. |
When a task within
gulp.series
generates an error, the error is logged twice: once by the task that generated the error, and once bygulp.series
. Is there some way to suppress this by default?The text was updated successfully, but these errors were encountered: