Merge pull request #525 from HipsterBrown/catch-inject-errors

catches/rejects swallowed copySync errors
tessel · Jan 17, 2016 · 3288afe · 3288afe
2 parents c92d32e + 7718240
commit 3288afe
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 79 deletions.
diff --git a/lib/tessel/deploy.js b/lib/tessel/deploy.js
@@ -522,7 +522,7 @@ actions.resolveBinaryModules = function(opts) {
 
 actions.injectBinaryModules = function(globRoot, tempBundlePath) {
 
-  return new Promise(resolve => {
+  return new Promise((resolve) => {
     // For every binary module in use...
     binaryModulesUsed.forEach(details => {
       var sourceBinary = path.join(details.extractPath, 'Release', details.binName);
@@ -636,35 +636,37 @@ actions.tarBundle = function(opts) {
             }
           });
 
-          actions.injectBinaryModules(globRoot, tempBundleDir).then(() => {
-            var fstream = new Reader({
-              path: tempBundleDir,
-              type: 'Directory',
-            });
+          actions.injectBinaryModules(globRoot, tempBundleDir)
+            .then(() => {
+              var fstream = new Reader({
+                path: tempBundleDir,
+                type: 'Directory',
+              });
 
-            fstream
-              .on('entry', (entry) => {
-                entry.root = {
-                  path: entry.path
-                };
-              })
-              .pipe(packer)
-              .on('data', (chunk) => {
-                buffers.push(chunk);
-              })
-              .on('error', (data) => {
-                reject(data);
-              })
-              .on('end', () => {
-                fs.remove(tempBundleDir, (error) => {
-                  if (error) {
-                    reject(error);
-                  } else {
-                    resolve(Buffer.concat(buffers));
-                  }
+              fstream
+                .on('entry', (entry) => {
+                  entry.root = {
+                    path: entry.path
+                  };
+                })
+                .pipe(packer)
+                .on('data', (chunk) => {
+                  buffers.push(chunk);
+                })
+                .on('error', (data) => {
+                  reject(data);
+                })
+                .on('end', () => {
+                  fs.remove(tempBundleDir, (error) => {
+                    if (error) {
+                      reject(error);
+                    } else {
+                      resolve(Buffer.concat(buffers));
+                    }
+                  });
                 });
-              });
-          });
+            })
+            .catch(reject);
         }
       });
     });
@@ -675,62 +677,64 @@ actions.tarBundle = function(opts) {
       // This allows us a safe way to "swap" binary modules.
       fs.copySync(globRoot, tempBundleDir);
 
-      return actions.injectBinaryModules(globRoot, tempBundleDir).then(() => {
-        var fstream = new Ignore({
-          basename: '',
-          ignoreFiles: ['.tesselignore'],
-          path: tempBundleDir,
-        });
+      return actions.injectBinaryModules(globRoot, tempBundleDir)
+        .then(() => {
+          var fstream = new Ignore({
+            basename: '',
+            ignoreFiles: ['.tesselignore'],
+            path: tempBundleDir,
+          });
 
-        // Don't send the actual rules files
-        fstream.addIgnoreRules([
-          '**/.tesselignore',
-          '**/.tesselinclude',
-        ]);
+          // Don't send the actual rules files
+          fstream.addIgnoreRules([
+            '**/.tesselignore',
+            '**/.tesselinclude',
+          ]);
 
-        if (includeNegateRules.length) {
-          fstream.addIgnoreRules(includeNegateRules);
-        }
-
-        if (!opts.single && includeFiles.length) {
-          // Instead of making a complete subclass of Ignore (as is done in fstream-npm,
-          // https://github.com/npm/fstream-npm/blob/master/fstream-npm.js#L91-L183),
-          // we'll over-ride the just the `applyIgnores` method for cases where there
-          // are .tesselinclude entries that have explicit inclusion rules.
-          fstream.applyIgnores = function(entry, partial, entryObj) {
-            if (includeFiles.indexOf(entry) !== -1) {
-              return true;
-            }
+          if (includeNegateRules.length) {
+            fstream.addIgnoreRules(includeNegateRules);
+          }
 
-            return Ignore.prototype.applyIgnores.call(fstream, entry, partial, entryObj);
-          };
-        }
+          if (!opts.single && includeFiles.length) {
+            // Instead of making a complete subclass of Ignore (as is done in fstream-npm,
+            // https://github.com/npm/fstream-npm/blob/master/fstream-npm.js#L91-L183),
+            // we'll over-ride the just the `applyIgnores` method for cases where there
+            // are .tesselinclude entries that have explicit inclusion rules.
+            fstream.applyIgnores = function(entry, partial, entryObj) {
+              if (includeFiles.indexOf(entry) !== -1) {
+                return true;
+              }
 
-        if (opts.single) {
-          fstream.addIgnoreRules(['*', '!' + opts.resolvedEntryPoint]);
-        }
+              return Ignore.prototype.applyIgnores.call(fstream, entry, partial, entryObj);
+            };
+          }
 
-        // This ensures that the remote root directory
-        // is the same level as the directory containing
-        // our program entry-point files.
-        fstream.on('entry', (entry) => {
-          entry.root = {
-            path: entry.path
-          };
-        });
+          if (opts.single) {
+            fstream.addIgnoreRules(['*', '!' + opts.resolvedEntryPoint]);
+          }
 
-        // Send the ignore-filtered file stream into the tar packer
-        fstream.pipe(packer)
-          .on('data', (chunk) => {
-            buffers.push(chunk);
-          })
-          .on('error', (data) => {
-            reject(data);
-          })
-          .on('end', () => {
-            resolve(Buffer.concat(buffers));
+          // This ensures that the remote root directory
+          // is the same level as the directory containing
+          // our program entry-point files.
+          fstream.on('entry', (entry) => {
+            entry.root = {
+              path: entry.path
+            };
           });
-      });
+
+          // Send the ignore-filtered file stream into the tar packer
+          fstream.pipe(packer)
+            .on('data', (chunk) => {
+              buffers.push(chunk);
+            })
+            .on('error', (data) => {
+              reject(data);
+            })
+            .on('end', () => {
+              resolve(Buffer.concat(buffers));
+            });
+        })
+        .catch(reject);
     });
   }
 };

diff --git a/test/unit/deploy.js b/test/unit/deploy.js
@@ -1,3 +1,6 @@
+process.on('uncaughtException', function(err) {
+  console.error(err.stack);
+});
 // Test dependencies are required and exposed in common/bootstrap.js
 
 var meminfo = fs.readFileSync('test/unit/fixtures/proc-meminfo', 'utf8');
@@ -1676,9 +1679,41 @@ exports['deploy.injectBinaryModules'] = {
         );
 
         test.done();
-      }).catch(error => test.fail(error));
-    }).catch(error => test.fail(error));
+      }).catch(error => {
+        test.fail(error);
+        test.done();
+      });
+    }).catch(error => {
+      test.fail(error);
+      test.done();
+    });
   },
+
+  throwError: function(test) {
+    test.expect(1);
+
+    var errorMessage = 'Test Error';
+    this.copySync.onCall(0).throws(errorMessage);
+
+    this.getRoot = sandbox.stub(bindings, 'getRoot', () => {
+      return path.normalize('node_modules/bufferutil/');
+    });
+
+    deploy.resolveBinaryModules({
+      target: this.target
+    }).then(() => {
+      deploy.injectBinaryModules(this.globRoot, fsTemp.mkdirSync()).then(() => {
+        test.fail('Should not pass');
+        test.done();
+      }).catch(error => {
+        test.equal(error, errorMessage);
+        test.done();
+      });
+    }).catch(error => {
+      test.fail(error);
+      test.done();
+    });
+  }
 };