Merge pull request #5964 from electron/manifest-errors

Throw errors reading/parsing manifest.json files

lib/browser/chrome-extension.js

@@ -22,12 +22,22 @@ const generateExtensionIdFromName = function (name) {
 // Create or get manifest object from |srcDirectory|.
 const getManifestFromPath = function (srcDirectory) {
   let manifest
+  let manifestContent
 
   try {
-    manifest = JSON.parse(fs.readFileSync(path.join(srcDirectory, 'manifest.json')))
+    manifestContent = fs.readFileSync(path.join(srcDirectory, 'manifest.json'))
-  } catch (err) {
+  } catch (readError) {
-    console.warn(`Attempted to load extension from ${srcDirectory}, but parsing the manifest failed.`)
+    console.warn(`Reading ${path.join(srcDirectory, 'manifest.json')} failed.`)
-    console.warn('Error encountered:', err)
+    console.warn(readError.stack || readError)
+    throw readError
+  }
+
+  try {
+    manifest = JSON.parse(manifestContent)
+  } catch (parseError) {
+    console.warn(`Parsing ${path.join(srcDirectory, 'manifest.json')} failed.`)
+    console.warn(parseError.stack || parseError)
+    throw parseError
   }
 
   if (!manifestNameMap[manifest.name]) {
@@ -47,7 +57,7 @@ const getManifestFromPath = function (srcDirectory) {
     })
     return manifest
   } else if (manifest && manifest.name) {
-    console.warn(`Attempted to load extension "${manifest.name}", but extension was already loaded!`)
+    console.warn(`Attempted to load extension "${manifest.name}" that has already been loaded.`)
   }
 }
 

spec/api-browser-window-spec.js

@@ -858,6 +858,18 @@ describe('browser-window module', function () {
         w.loadURL('about:blank')
       })
 
+      it('throws errors for missing manifest.json files', function () {
+        assert.throws(function () {
+          BrowserWindow.addDevToolsExtension(path.join(__dirname, 'does-not-exist'))
+        }, /ENOENT: no such file or directory/)
+      })
+
+      it('throws errors for invalid manifest.json files', function () {
+        assert.throws(function () {
+          BrowserWindow.addDevToolsExtension(path.join(__dirname, 'fixtures', 'devtools-extensions', 'bad-manifest'))
+        }, /Unexpected token }/)
+      })
+
       describe('when the devtools is docked', function () {
         it('creates the extension', function (done) {
           w.webContents.openDevTools({mode: 'bottom'})

spec/fixtures/devtools-extensions/bad-manifest/manifest.json

@@ -0,0 +1 @@
+}