fix stream error handling for bundling and watching. Fixes #43 and #31

dowjones · Mar 1, 2015 · d0cd319 · d0cd319
1 parent aaee626
commit d0cd319
Show file tree

Hide file tree

Showing 15 changed files with 152 additions and 29 deletions.
diff --git a/examples/6to5/gulpfile.js b/examples/6to5/gulpfile.js
@@ -1,5 +1,6 @@
 var gulp = require('gulp'),
   rimraf = require('gulp-rimraf'),
+  path = require('path'),
   bundle = require('../../');
 
 gulp.task('bundle', ['clean'], function () {
@@ -11,4 +12,12 @@ gulp.task('bundle', ['clean'], function () {
 gulp.task('clean', function () {
   return gulp.src('./public', { read: false })
     .pipe(rimraf());
+});
+
+// rebuild bundle on change of a source file
+gulp.task('watch', function () {
+  bundle.watch({
+    configPath: path.join(__dirname, 'bundle.config.js'),
+    dest: path.join(__dirname, 'public')
+  });
 });
diff --git a/examples/browserify/bundle.config.js b/examples/browserify/bundle.config.js
@@ -1,7 +1,8 @@
 var transformHelper = require('../../index.js').transformHelper,
   browserify = require('browserify'),
   sourceStream = require('vinyl-source-stream'),
-  isDebug = (process.env.NODE_ENV !== 'production');
+  isDebug = (process.env.NODE_ENV !== 'production'),
+  gutil = require('gulp-util');
 
 var mainStream = function (file, done) {
   browserify({
@@ -16,6 +17,15 @@ var mainStream = function (file, done) {
     //.transform(reactify)
     //.require(vendorLibs)
     .bundle()
+    .on('error', function (err) {
+      // make sure browserify errors don't break the pipe during watch
+      if (file.bundleOptions.isWatch) {
+        gutil.log(gutil.colors.red(err.toString()));
+        this.emit('end');
+      } else {
+        throw err;
+      }
+    })
     .pipe(sourceStream('app.js')) // convert to gulp stream
     .pipe(done); // make sure to pipe to the "done" stream
 };

diff --git a/examples/browserify/package.json b/examples/browserify/package.json
@@ -11,6 +11,7 @@
   "devDependencies": {
     "browserify": "^8.1.3",
     "gulp": "^3.8.10",
+    "gulp-util": "^3.0.4",
     "gulp-rimraf": "^0.1.1",
     "vinyl-source-stream": "^1.0.0"
   }

diff --git a/examples/custom-transforms/bundle.config.js b/examples/custom-transforms/bundle.config.js
@@ -44,9 +44,11 @@ module.exports = {
       ],
       styles: [
         './content/**/*.scss',
-        './content/**/*.less'
+        './content/**/*.less',
+        './content/**/*.css'
       ],
       options: {
+        minCSS: true,
         transforms: {
           scripts: scriptTransforms,
           styles: styleTransforms

diff --git a/examples/custom-transforms/bundle.result.json b/examples/custom-transforms/bundle.result.json
@@ -1,6 +1,6 @@
 {
   "article": {
     "scripts": "<script src='/public/article-0e717735.js' type='text/javascript'></script>",
-    "styles": "<link href='/public/article-05a1266f.css' media='all' rel='stylesheet' type='text/css'/>"
+    "styles": "<link href='/public/article-950e9844.css' media='all' rel='stylesheet' type='text/css'/>"
   }
 }
diff --git a/examples/custom-transforms/content/styles/regular.css b/examples/custom-transforms/content/styles/regular.css
@@ -0,0 +1,3 @@
+.my-regular-css {
+  background-color: blue;
+}
diff --git a/examples/custom-transforms/gulpfile.js b/examples/custom-transforms/gulpfile.js
@@ -1,9 +1,9 @@
 var gulp = require('gulp'),
   rimraf = require('gulp-rimraf'),
+  path = require('path'),
   bundle = require('../../index');
 
 gulp.task('bundle', ['clean'], function () {
-
   return gulp.src('./bundle.config.js')
     .pipe(bundle())
     .pipe(bundle.results({
@@ -14,6 +14,13 @@ gulp.task('bundle', ['clean'], function () {
 });
 
 gulp.task('clean', function () {
-  return gulp.src('./public', { read: false })
+  return gulp.src('./public', {read: false})
     .pipe(rimraf());
+});
+
+gulp.task('watch', function () {
+  bundle.watch({
+    configPath: path.join(__dirname, 'bundle.config.js'),
+    dest: path.join(__dirname, 'public')
+  });
 });
diff --git a/...om-transforms/public/article-05a1266f.css → ...om-transforms/public/article-950e9844.css b/...om-transforms/public/article-05a1266f.css → ...om-transforms/public/article-950e9844.css
diff --git a/examples/custom-transforms/public/maps/article-05a1266f.css.map b/examples/custom-transforms/public/maps/article-05a1266f.css.map
diff --git a/examples/custom-transforms/public/maps/article-950e9844.css.map b/examples/custom-transforms/public/maps/article-950e9844.css.map
diff --git a/examples/custom-transforms/readme.md b/examples/custom-transforms/readme.md
@@ -7,6 +7,10 @@ NOTE: any plugin you wish to use should support [sourcemaps](https://github.com/
 otherwise it will break the resulting sourcemaps generation. 
 See [here on how to add this support to a gulp plugin](https://github.com/floridoo/gulp-sourcemaps#plugin-developers-only-how-to-add-source-map-support-to-plugins).
 
+NOTE 2: By default, the bundler will detect whether you're doing a typical build or running in watch mode. If it's the
+ later, the bundler will simply log the error and not stop the stream. That said, browserify works slightly differently
+ and you'll need to handle those errors manually. [See the browserify example for how to do this](../browserify).
+
 ## How to define custom transforms
 
 Define your custom transforms as part of a [`lazypipe`](https://github.com/OverZealous/lazypipe) stream.

diff --git a/examples/result-json/bundle.result.json b/examples/result-json/bundle.result.json
@@ -7,7 +7,7 @@
     "scripts": "<script src='/public/main-2742a3c0.js' type='text/javascript'></script>"
   },
   "vendor": {
-    "scripts": "<script src='/public/vendor-6873f46e.js' type='text/javascript'></script>",
-    "styles": "<link href='/public/vendor-7c38ff67.css' media='all' rel='stylesheet' type='text/css'/>"
+    "styles": "<link href='/public/vendor-7c38ff67.css' media='all' rel='stylesheet' type='text/css'/>",
+    "scripts": "<script src='/public/vendor-6873f46e.js' type='text/javascript'></script>"
   }
 }
diff --git a/lib/stream-bundles-watch.js b/lib/stream-bundles-watch.js
@@ -67,7 +67,8 @@ function _bundle(config, env) {
                 bundleName: bundleName,
                 bundleOptions: namedBundleObj[BundleKeys.OPTIONS],
                 isBundleAll: isBundleAll,
-                minSrcs: minSrcs
+                minSrcs: minSrcs,
+                isWatch: true
               })
                 .pipe(gif(resultOpts, results(resultOpts)))
                 .pipe(gulp.dest(config.options.dest))
@@ -102,7 +103,8 @@ function _bundle(config, env) {
                 bundleName: bundleName,
                 bundleOptions: namedBundleObj[BundleKeys.OPTIONS],
                 isBundleAll: isBundleAll,
-                minSrcs: minSrcs
+                minSrcs: minSrcs,
+                isWatch: true
               })
                 .pipe(gif(resultOpts, results(resultOpts)))
                 .pipe(gulp.dest(config.options.dest))

diff --git a/lib/stream-files.js b/lib/stream-files.js
@@ -16,22 +16,43 @@ var gulp = require('gulp'),
   BundleKeys = require('./model/bundle-keys'),
   isMinEnabled = require('./is-min-enabled');
 
-module.exports.handleTransformError = function (bundleName, bundleKey, err) {
+module.exports.handleTransformError = function (thisStream, isWatch, bundleName, bundleKey, err) {
   logger.log(gutil.colors.red("ERROR in custom transforms for '" + bundleName + "." + bundleKey + "':"));
-  logger.log(err);
+  logger.log(err.toString());
+  if (isWatch) {
+    thisStream.emit('end');
+  } else {
+    process.exit(1);
+  }
+};
+
+module.exports.attachStreamOptions = function (file, opts) {
+  // add some useful options to file for possible inspection later in external custom gulp streams
+  file.bundleOptions = {
+    env: opts.env,
+    type: opts.type,
+    bundleName: opts.bundleName,
+    isWatch: opts.isWatch,
+    isBundleAll: opts.isBundleAll
+  };
 };
 
 module.exports.scripts = function (opts) {
   var self = this;
   return gulp.src(opts.src, {base: opts.base})
     .pipe(using.bundle(opts.bundleName, BundleKeys.SCRIPTS, opts.env, opts.isBundleAll))
+    .pipe(through.obj(function (file, enc, cb) {
+      self.attachStreamOptions(file, opts);
+      this.push(file);
+      cb();
+    }))
     .pipe(gif(function (file) {
         return sourcemaps.isEnabled(opts);
       }, gsourcemaps.init({loadMaps: true})
     ))
     .pipe(opts.bundleOptions.transforms[BundleKeys.SCRIPTS]())
     .on('error', function (e) {
-      self.handleTransformError(opts.bundleName, BundleKeys.SCRIPTS, e);
+      self.handleTransformError(this, opts.isWatch, opts.bundleName, BundleKeys.SCRIPTS, e);
     })
     .pipe(gif(function (file) {
         return sourcemaps.isEnabled(opts);
@@ -47,6 +68,9 @@ module.exports.scripts = function (opts) {
           uglify()),
         uglify()
       )))
+    .on('error', function (e) {
+      self.handleTransformError(this, opts.isWatch, opts.bundleName, BundleKeys.SCRIPTS, e);
+    })
     .pipe(gif(function (file) {
         return file.isStream();
       },
@@ -69,14 +93,19 @@ module.exports.styles = function (opts) {
   var self = this;
   return gulp.src(opts.src, {base: opts.base})
     .pipe(using.bundle(opts.bundleName, BundleKeys.STYLES, opts.env, opts.isBundleAll))
+    .pipe(through.obj(function (file, enc, cb) {
+      self.attachStreamOptions(file, opts);
+      this.push(file);
+      cb();
+    }))
     .pipe(
     gif(function (file) {
         return sourcemaps.isEnabled(opts);
       }, gsourcemaps.init({loadMaps: true})
     ))
     .pipe(opts.bundleOptions.transforms[BundleKeys.STYLES]())
     .on('error', function (e) {
-      self.handleTransformError(opts.bundleName, BundleKeys.STYLES, e);
+      self.handleTransformError(this, opts.isWatch, opts.bundleName, BundleKeys.STYLES, e);
     })
     .pipe(gif(function (file) {
         return sourcemaps.isEnabled(opts);

diff --git a/test/unit/stream-files-test.js b/test/unit/stream-files-test.js
@@ -15,7 +15,9 @@ describe('stream-files', function () {
     describe('styles', function () {
 
       it('should call error handler when custom transform fails', function (done) {
-        streamFiles.handleTransformError = function (bundleName, bundleKey, err) {
+        streamFiles.handleTransformError = function (thisStream, isWatch, bundleName, bundleKey, err) {
+          should.not.exist(isWatch);
+          should.exist(thisStream);
           bundleName.should.eql('main');
           bundleKey.should.eql('styles');
           //should(err).typeof('object');
@@ -43,7 +45,9 @@ describe('stream-files', function () {
     describe('scripts', function () {
 
       it('should call error handler when custom transform fails', function (done) {
-        streamFiles.handleTransformError = function (bundleName, bundleKey, err) {
+        streamFiles.handleTransformError = function (thisStream, isWatch, bundleName, bundleKey, err) {
+          should.not.exist(isWatch);
+          should.exist(thisStream);
           bundleName.should.eql('main');
           bundleKey.should.eql('scripts');
           //should(err).typeof('object');
@@ -70,23 +74,74 @@ describe('stream-files', function () {
 
   });
 
-
   describe('handleTransformError', function () {
 
-    var mockLogger = {
-      log: function () {
-      }
-    };
-    var spy = sinon.spy(mockLogger, 'log');
-    var streamFiles = proxyquire(libPath + '/stream-files', {
-      './service/logger': mockLogger
+    it('should log errors during watch', function () {
+      var mockLogger = {
+        log: function () {
+        }
+      };
+      var mockPipe = {
+        emit: function () {
+        }
+      };
+      var logSpy = sinon.spy(mockLogger, 'log');
+      var pipeSpy = sinon.spy(mockPipe, 'emit');
+      var streamFiles = proxyquire(libPath + '/stream-files', {
+        './service/logger': mockLogger
+      });
+      var errObj = new Error('my err');
+      streamFiles.handleTransformError(mockPipe, true, 'my_bundle_name', 'my_bundle_key', errObj);
+      logSpy.calledTwice.should.be.ok;
+      pipeSpy.calledOnce.should.be.ok;
+      pipeSpy.withArgs('end').calledOnce.should.be.ok;
     });
 
-    it('should log errors', function () {
+    it('should process.exit when errors during bundle', function () {
+      var mockProcessExit = sinon.spy(),
+        origExit = process.exit;
+      process.exit = mockProcessExit;
+
+      var mockLogger = {
+        log: function () {
+        }
+      };
+      var logSpy = sinon.spy(mockLogger, 'log');
+      var streamFiles = proxyquire(libPath + '/stream-files', {
+        './service/logger': mockLogger
+      });
       var errObj = new Error('my err');
-      streamFiles.handleTransformError('my_bundle_name', 'my_bundle_key', errObj);
-      spy.calledTwice.should.be.ok;
-      spy.withArgs(errObj).calledOnce.should.be.ok;
+      streamFiles.handleTransformError(null, undefined, 'my_bundle_name', 'my_bundle_key', errObj);
+      logSpy.calledTwice.should.be.ok;
+      mockProcessExit.calledOnce.should.be.ok;
+
+      process.exit = origExit;
+    });
+
+  });
+
+  describe('attachStreamOptions', function () {
+
+    var streamFiles = require(libPath + '/stream-files');
+
+    it('should attach specific options to file obj', function () {
+
+      var fakeFile = {};
+      streamFiles.attachStreamOptions(fakeFile, {
+        env: 'production',
+        type: 'scripts',
+        bundleName: 'my_bundle_name',
+        isWatch: true,
+        isBundleAll: false
+      });
+
+      fakeFile.bundleOptions.should.be.ok;
+      fakeFile.bundleOptions.env.should.eql('production');
+      fakeFile.bundleOptions.type.should.eql('scripts');
+      fakeFile.bundleOptions.bundleName.should.eql('my_bundle_name');
+      fakeFile.bundleOptions.isWatch.should.be.ok;
+      fakeFile.bundleOptions.isBundleAll.should.not.be.ok;
+
     });
 
   });