From 89038fc0b874de8ddb2e8b0832fcb24ff6a55516 Mon Sep 17 00:00:00 2001
From: Robert Hurst <rhurst@dialpad.com>
Date: Tue, 26 Feb 2019 16:00:12 -0800
Subject: [PATCH] Add helpful plugin errors

---
 packages/core/parcel-bundler/.gitignore       |  3 +-
 packages/core/parcel-bundler/src/Bundler.js   |  8 ++-
 packages/core/parcel-bundler/src/Parser.js    | 17 +++++-
 .../plugins/{ => test-plugin}/index.js        |  0
 .../parcel-plugin-test/TextAsset.js           |  2 +-
 .../node_modules/parcel-plugin-test/index.js  |  0
 .../plugins/{ => test-plugin}/package.json    |  2 +-
 .../{ => test-plugin}/sub-folder/index.js     |  0
 .../{ => test-plugin}/sub-folder/test.txt     |  0
 .../plugins/{ => test-plugin}/test.txt        |  0
 .../plugins/throwing-plugin-parser/index.js   |  1 +
 .../parcel-plugin-test/TextAsset.js           |  1 +
 .../node_modules/parcel-plugin-test/index.js  |  3 +
 .../throwing-plugin-parser/package.json       |  7 +++
 .../plugins/throwing-plugin-parser/test.txt   |  1 +
 .../plugins/throwing-plugin/index.js          |  1 +
 .../node_modules/parcel-plugin-test/index.js  |  3 +
 .../plugins/throwing-plugin/package.json      |  7 +++
 .../plugins/throwing-plugin/test.txt          |  1 +
 packages/core/parcel-bundler/test/plugins.js  | 58 ++++++++++++++++++-
 20 files changed, 107 insertions(+), 8 deletions(-)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/index.js (100%)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/node_modules/parcel-plugin-test/TextAsset.js (86%)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/node_modules/parcel-plugin-test/index.js (100%)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/package.json (70%)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/sub-folder/index.js (100%)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/sub-folder/test.txt (100%)
 rename packages/core/parcel-bundler/test/integration/plugins/{ => test-plugin}/test.txt (100%)
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/index.js
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/TextAsset.js
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/index.js
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/package.json
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/test.txt
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/index.js
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/node_modules/parcel-plugin-test/index.js
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/package.json
 create mode 100644 packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/test.txt

diff --git a/packages/core/parcel-bundler/.gitignore b/packages/core/parcel-bundler/.gitignore
index bc45e23288c..7664af8e87e 100644
--- a/packages/core/parcel-bundler/.gitignore
+++ b/packages/core/parcel-bundler/.gitignore
@@ -5,7 +5,6 @@ test/input
 test/integration/**/elm-stuff
 test/integration/**/target
 test/integration/**/Cargo.lock
-test/**/node_modules
 test/**/yarn.lock
 test/**/package-lock.json
-test/integration/babel-plugin-autoinstall/package.json
\ No newline at end of file
+test/integration/babel-plugin-autoinstall/package.json
diff --git a/packages/core/parcel-bundler/src/Bundler.js b/packages/core/parcel-bundler/src/Bundler.js
index f30ae772a52..419e93b4546 100644
--- a/packages/core/parcel-bundler/src/Bundler.js
+++ b/packages/core/parcel-bundler/src/Bundler.js
@@ -206,9 +206,11 @@ class Bundler extends EventEmitter {
       return;
     }
 
+    let lastDep;
     try {
       let deps = Object.assign({}, pkg.dependencies, pkg.devDependencies);
       for (let dep in deps) {
+        lastDep = dep;
         const pattern = /^(@.*\/)?parcel-plugin-.+/;
         if (pattern.test(dep)) {
           let plugin = await localRequire(dep, relative);
@@ -216,7 +218,11 @@ class Bundler extends EventEmitter {
         }
       }
     } catch (err) {
-      logger.warn(err);
+      logger.warn(
+        `Plugin ${lastDep} failed to initialize: ${err.stack ||
+          err.message ||
+          err}`
+      );
     }
   }
 
diff --git a/packages/core/parcel-bundler/src/Parser.js b/packages/core/parcel-bundler/src/Parser.js
index 09163cb6d4a..9e0ef105b3d 100644
--- a/packages/core/parcel-bundler/src/Parser.js
+++ b/packages/core/parcel-bundler/src/Parser.js
@@ -1,4 +1,5 @@
 const path = require('path');
+const logger = require('@parcel/logger');
 const RawAsset = require('./assets/RawAsset');
 const GlobAsset = require('./assets/GlobAsset');
 const {isGlob} = require('./utils/glob');
@@ -74,7 +75,21 @@ class Parser {
     let extension = path.extname(filename).toLowerCase();
     let parser = this.extensions[extension] || RawAsset;
     if (typeof parser === 'string') {
-      parser = this.extensions[extension] = require(parser);
+      try {
+        parser = this.extensions[extension] = require(parser);
+      } catch (err) {
+        let relFilename = path.relative(process.cwd(), filename);
+        let relParserName = path.relative(process.cwd(), parser);
+        if (relParserName.slice(0, 12) === 'node_modules') {
+          relParserName = relParserName.slice(13);
+        }
+        logger.warn(
+          `Parser "${relParserName}" failed to initialize when processing ` +
+            `asset "${relFilename}". Threw the following error:\n` +
+            `${err.stack || err.message || err} falling back to RawAsset`
+        );
+        return RawAsset;
+      }
     }
 
     return parser;
diff --git a/packages/core/parcel-bundler/test/integration/plugins/index.js b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/index.js
similarity index 100%
rename from packages/core/parcel-bundler/test/integration/plugins/index.js
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/index.js
diff --git a/packages/core/parcel-bundler/test/integration/plugins/node_modules/parcel-plugin-test/TextAsset.js b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/node_modules/parcel-plugin-test/TextAsset.js
similarity index 86%
rename from packages/core/parcel-bundler/test/integration/plugins/node_modules/parcel-plugin-test/TextAsset.js
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/node_modules/parcel-plugin-test/TextAsset.js
index 75e82f36a6d..0c8093d9791 100644
--- a/packages/core/parcel-bundler/test/integration/plugins/node_modules/parcel-plugin-test/TextAsset.js
+++ b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/node_modules/parcel-plugin-test/TextAsset.js
@@ -3,7 +3,7 @@ if (parseInt(process.versions.node, 10) < 8) {
   require("@babel/register");
 }
 
-const {Asset} = require('../../../../../src/Bundler');
+const {Asset} = require('../../../../../../src/Bundler');
 
 class TextAsset extends Asset {
   constructor(name, pkg, options) {
diff --git a/packages/core/parcel-bundler/test/integration/plugins/node_modules/parcel-plugin-test/index.js b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/node_modules/parcel-plugin-test/index.js
similarity index 100%
rename from packages/core/parcel-bundler/test/integration/plugins/node_modules/parcel-plugin-test/index.js
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/node_modules/parcel-plugin-test/index.js
diff --git a/packages/core/parcel-bundler/test/integration/plugins/package.json b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/package.json
similarity index 70%
rename from packages/core/parcel-bundler/test/integration/plugins/package.json
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/package.json
index 9f0ca2d53ea..b7101fdef97 100644
--- a/packages/core/parcel-bundler/test/integration/plugins/package.json
+++ b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/package.json
@@ -1,5 +1,5 @@
 {
-  "name": "parcel-test",
+  "name": "test-plugin-project",
   "private": true,
   "dependencies": {
     "parcel-plugin-test": "*"
diff --git a/packages/core/parcel-bundler/test/integration/plugins/sub-folder/index.js b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/sub-folder/index.js
similarity index 100%
rename from packages/core/parcel-bundler/test/integration/plugins/sub-folder/index.js
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/sub-folder/index.js
diff --git a/packages/core/parcel-bundler/test/integration/plugins/sub-folder/test.txt b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/sub-folder/test.txt
similarity index 100%
rename from packages/core/parcel-bundler/test/integration/plugins/sub-folder/test.txt
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/sub-folder/test.txt
diff --git a/packages/core/parcel-bundler/test/integration/plugins/test.txt b/packages/core/parcel-bundler/test/integration/plugins/test-plugin/test.txt
similarity index 100%
rename from packages/core/parcel-bundler/test/integration/plugins/test.txt
rename to packages/core/parcel-bundler/test/integration/plugins/test-plugin/test.txt
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/index.js b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/index.js
new file mode 100644
index 00000000000..81fba7b4e53
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/index.js
@@ -0,0 +1 @@
+module.exports = require('./test.txt');
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/TextAsset.js b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/TextAsset.js
new file mode 100644
index 00000000000..839e51a5754
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/TextAsset.js
@@ -0,0 +1 @@
+throw new Error('Parser error');
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/index.js b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/index.js
new file mode 100644
index 00000000000..1878e88fb5c
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/node_modules/parcel-plugin-test/index.js
@@ -0,0 +1,3 @@
+module.exports = function (bundler) {
+  bundler.addAssetType('txt', require.resolve('./TextAsset'));
+};
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/package.json b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/package.json
new file mode 100644
index 00000000000..a5c5e623931
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "throwing-plugin-parser-project",
+  "private": true,
+  "dependencies": {
+    "parcel-plugin-test": "*"
+  }
+}
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/test.txt b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/test.txt
new file mode 100644
index 00000000000..95d09f2b101
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin-parser/test.txt
@@ -0,0 +1 @@
+hello world
\ No newline at end of file
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/index.js b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/index.js
new file mode 100644
index 00000000000..81fba7b4e53
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/index.js
@@ -0,0 +1 @@
+module.exports = require('./test.txt');
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/node_modules/parcel-plugin-test/index.js b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/node_modules/parcel-plugin-test/index.js
new file mode 100644
index 00000000000..1fcb2169116
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/node_modules/parcel-plugin-test/index.js
@@ -0,0 +1,3 @@
+module.exports = function () {
+  throw new Error('Plugin error')
+};
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/package.json b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/package.json
new file mode 100644
index 00000000000..054ee943804
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "throwing-plugin-project",
+  "private": true,
+  "dependencies": {
+    "parcel-plugin-test": "*"
+  }
+}
diff --git a/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/test.txt b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/test.txt
new file mode 100644
index 00000000000..95d09f2b101
--- /dev/null
+++ b/packages/core/parcel-bundler/test/integration/plugins/throwing-plugin/test.txt
@@ -0,0 +1 @@
+hello world
\ No newline at end of file
diff --git a/packages/core/parcel-bundler/test/plugins.js b/packages/core/parcel-bundler/test/plugins.js
index d8e69fdb879..d796d03ef2e 100644
--- a/packages/core/parcel-bundler/test/plugins.js
+++ b/packages/core/parcel-bundler/test/plugins.js
@@ -1,10 +1,14 @@
 const assert = require('assert');
+const sinon = require('sinon');
 const path = require('path');
+const logger = require('@parcel/logger');
 const {bundle, run, assertBundleTree} = require('@parcel/test-utils');
 
 describe('plugins', function() {
   it('should load plugins and apply custom asset type', async function() {
-    let b = await bundle(path.join(__dirname, '/integration/plugins/index.js'));
+    let b = await bundle(
+      path.join(__dirname, '/integration/plugins/test-plugin/index.js')
+    );
 
     await assertBundleTree(b, {
       name: 'index.js',
@@ -22,7 +26,10 @@ describe('plugins', function() {
 
   it('should load package.json from parent tree', async function() {
     let b = await bundle(
-      path.join(__dirname, '/integration/plugins/sub-folder/index.js')
+      path.join(
+        __dirname,
+        '/integration/plugins/test-plugin/sub-folder/index.js'
+      )
     );
 
     await assertBundleTree(b, {
@@ -38,4 +45,51 @@ describe('plugins', function() {
     let output = await run(b);
     assert.equal(output, 'hello world');
   });
+
+  it('log a warning if a plugin throws an exception during initialization', async function() {
+    sinon.stub(logger, 'warn');
+
+    let b = await bundle(
+      path.join(__dirname, '/integration/plugins/throwing-plugin/index.js')
+    );
+
+    await run(b);
+
+    sinon.assert.calledWith(
+      logger.warn,
+      sinon.match(
+        'Plugin parcel-plugin-test failed to initialize: Error: Plugin error'
+      )
+    );
+
+    logger.warn.restore();
+  });
+
+  it('log a warning if a parser throws an exception during initialization', async function() {
+    sinon.stub(logger, 'warn');
+
+    let b = await bundle(
+      path.join(
+        __dirname,
+        '/integration/plugins/throwing-plugin-parser/index.js'
+      )
+    );
+
+    await run(b);
+
+    sinon.assert.calledWith(
+      logger.warn,
+      sinon.match(
+        'Parser "test/integration/plugins/throwing-plugin-parser/node_modules/' +
+          'parcel-plugin-test/TextAsset.js" failed to initialize when processing ' +
+          'asset "test/integration/plugins/throwing-plugin-parser/test.txt". ' +
+          'Threw the following error:\nError: Parser error'.replace(
+            /\//g,
+            path.sep
+          )
+      )
+    );
+
+    logger.warn.restore();
+  });
 });