From 1fa0b62317ed23d7bcc0dd97efc6839ed76f843a Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Tue, 20 Feb 2024 10:45:29 +0100
Subject: [PATCH] fix: better error message for bad `package.json` (#1547)

* fix: better error message for bad package.json

* Update src/renderer/file-manager.ts

Co-authored-by: David Sanders <dsanders11@ucsbalum.com>

* Update src/renderer/remote-loader.ts

Co-authored-by: David Sanders <dsanders11@ucsbalum.com>

* fix: initialize const variable

---------

Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
---
 src/renderer/file-manager.ts         | 15 ++++++++++-----
 src/renderer/remote-loader.ts        | 12 +++++++-----
 tests/renderer/file-manager-spec.ts  | 13 +++++++++++++
 tests/renderer/remote-loader-spec.ts | 23 +++++++++++++++++++++++
 4 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/src/renderer/file-manager.ts b/src/renderer/file-manager.ts
index a531dd125a..038f735dde 100644
--- a/src/renderer/file-manager.ts
+++ b/src/renderer/file-manager.ts
@@ -70,11 +70,16 @@ export class FileManager {
     for (const [name, value] of Object.entries(files)) {
       if (name === PACKAGE_NAME) {
         const { remoteLoader } = window.app;
-        const { dependencies, devDependencies } = JSON.parse(value);
-        const deps: Record<string, string> = {
-          ...dependencies,
-          ...devDependencies,
-        };
+
+        const deps: Record<string, string> = {};
+        try {
+          const { dependencies, devDependencies } = JSON.parse(value);
+          Object.assign(deps, dependencies, devDependencies);
+        } catch {
+          await this.appState.showErrorDialog(
+            'Could not open Fiddle - invalid JSON found in package.json',
+          );
+        }
 
         // If the project specifies an Electron version, we want to tell Fiddle to run
         // it with that version by default.
diff --git a/src/renderer/remote-loader.ts b/src/renderer/remote-loader.ts
index 023ab6ce99..5bc5cd5cc5 100644
--- a/src/renderer/remote-loader.ts
+++ b/src/renderer/remote-loader.ts
@@ -130,11 +130,13 @@ export class RemoteLoader {
           : data.content;
 
         if (id === PACKAGE_NAME) {
-          const { dependencies, devDependencies } = JSON.parse(content);
-          const deps: Record<string, string> = {
-            ...dependencies,
-            ...devDependencies,
-          };
+          const deps: Record<string, string> = {};
+          try {
+            const { dependencies, devDependencies } = JSON.parse(content);
+            Object.assign(deps, dependencies, devDependencies);
+          } catch (e) {
+            throw new Error('Invalid JSON found in package.json');
+          }
 
           // If the gist specifies an Electron version, we want to tell Fiddle to run
           // it with that version by default.
diff --git a/tests/renderer/file-manager-spec.ts b/tests/renderer/file-manager-spec.ts
index 20f116451b..9e94b4a131 100644
--- a/tests/renderer/file-manager-spec.ts
+++ b/tests/renderer/file-manager-spec.ts
@@ -63,6 +63,19 @@ describe('FileManager', () => {
       });
     });
 
+    it('handles bad JSON in package.json', async () => {
+      const badPj =
+        '{"main":"main.js","devDependencies":{"electron":"17.0.0",}}';
+      const values = { ...editorValues, [PACKAGE_NAME]: badPj };
+
+      await fm.openFiddle(filePath, values);
+      expect(app.state.showErrorDialog).toHaveBeenCalledWith(
+        expect.stringMatching(
+          /Could not open Fiddle - invalid JSON found in package.json/i,
+        ),
+      );
+    });
+
     it('respects the Electron version specified in package.json', async () => {
       const pj = {
         main: MAIN_JS,
diff --git a/tests/renderer/remote-loader-spec.ts b/tests/renderer/remote-loader-spec.ts
index 61d08bf644..a7375ee38e 100644
--- a/tests/renderer/remote-loader-spec.ts
+++ b/tests/renderer/remote-loader-spec.ts
@@ -83,6 +83,29 @@ describe('RemoteLoader', () => {
       expect(app.replaceFiddle).toBeCalledWith(editorValues, { gistId });
     });
 
+    it('handles bad JSON in package.json', async () => {
+      const gistId = 'badjsontestid';
+      const badPj =
+        '{"main":"main.js","devDependencies":{"electron":"17.0.0",}}';
+
+      store.gistId = gistId;
+      mockGistFiles[PACKAGE_NAME] = { content: badPj };
+      mockRepos.push({
+        name: PACKAGE_NAME,
+        download_url: `https://${PACKAGE_NAME}`,
+      });
+
+      mocked(getOctokit).mockResolvedValue({
+        gists: mockGetGists,
+      } as unknown as Octokit);
+
+      const result = await instance.fetchGistAndLoad(gistId);
+      expect(result).toBe(false);
+      expect(store.showErrorDialog).toHaveBeenCalledWith(
+        expect.stringMatching(/Invalid JSON found in package.json/i),
+      );
+    });
+
     it('handles gist fiddle devDependencies', async () => {
       const gistId = 'pjsontestid';
       const pj = {