Merge pull request #6473 from Shopify/10-02-handle_malformed_package.json_errors

Handle malformed package.json errors

Author: isaacroldan

packages/cli-kit/src/public/common/json.test.ts

@@ -0,0 +1,54 @@
+import {parseJSON} from './json.js'
+import {AbortError} from '../node/error.js'
+import {describe, expect, test} from 'vitest'
+
+describe('parseJSON', () => {
+  test('parses valid JSON with nested objects', () => {
+    // Given
+    const jsonString = '{"user": {"name": "Alice", "age": 30}}'
+
+    // When
+    const result = parseJSON(jsonString)
+
+    // Then
+    expect(result).toEqual({user: {name: 'Alice', age: 30}})
+  })
+
+  test('throws AbortError for malformed JSON without context', () => {
+    // Given
+    const malformedJSON = '{"name": "test", invalid}'
+
+    // When/Then
+    expect(() => parseJSON(malformedJSON)).toThrow(AbortError)
+    expect(() => parseJSON(malformedJSON)).toThrow(/Failed to parse JSON/)
+  })
+
+  test('throws AbortError for malformed JSON with context', () => {
+    // Given
+    const malformedJSON = '{"name": "test", invalid}'
+    const context = '/path/to/config.json'
+
+    // When/Then
+    expect(() => parseJSON(malformedJSON, context)).toThrow(AbortError)
+    expect(() => parseJSON(malformedJSON, context)).toThrow(/Failed to parse JSON from \/path\/to\/config\.json/)
+  })
+
+  test('throws AbortError with original error message', () => {
+    // Given
+    const malformedJSON = '{"trailing comma":,}'
+
+    // When/Then
+    expect(() => parseJSON(malformedJSON)).toThrow(/Unexpected token/)
+  })
+
+  test('handles null value', () => {
+    // Given
+    const jsonString = 'null'
+
+    // When
+    const result = parseJSON(jsonString)
+
+    // Then
+    expect(result).toBeNull()
+  })
+})

packages/cli-kit/src/public/common/json.ts

@@ -0,0 +1,27 @@
+import {AbortError} from '../node/error.js'
+
+/**
+ * Safely parse JSON with helpful error messages.
+ *
+ * @param jsonString - The JSON string to parse.
+ * @param context - Optional context about what's being parsed (e.g., file path, "API response").
+ * @returns The parsed JSON object.
+ * @throws AbortError if JSON is malformed.
+ *
+ * @example
+ * // Parse with context
+ * const data = parseJSON(jsonString, '/path/to/config.json')
+ *
+ * @example
+ * // Parse without context
+ * const data = parseJSON(jsonString)
+ */
+export function parseJSON<T = unknown>(jsonString: string, context?: string): T {
+  try {
+    return JSON.parse(jsonString) as T
+  } catch (error) {
+    const errorMessage = error instanceof Error ? error.message : String(error)
+    const contextMessage = context ? ` from ${context}` : ''
+    throw new AbortError(`Failed to parse JSON${contextMessage}.\n${errorMessage}`)
+  }
+}

packages/cli-kit/src/public/node/node-package-manager.ts

@@ -7,6 +7,7 @@ import {runWithTimer} from './metadata.js'
 import {inferPackageManagerForGlobalCLI} from './is-global.js'
 import {outputToken, outputContent, outputDebug} from '../../public/node/output.js'
 import {PackageVersionKey, cacheRetrieve, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js'
+import {parseJSON} from '../common/json.js'
 import latestVersion from 'latest-version'
 import {SemVer, satisfies as semverSatisfies} from 'semver'
 import type {Writable} from 'stream'
@@ -406,7 +407,7 @@ export async function readAndParsePackageJson(packageJsonPath: string): Promise<
   if (!(await fileExists(packageJsonPath))) {
     throw new PackageJsonNotFoundError(dirname(packageJsonPath))
   }
-  return JSON.parse(await readFile(packageJsonPath))
+  return parseJSON(await readFile(packageJsonPath), packageJsonPath)
 }
 
 interface AddNPMDependenciesIfNeededOptions {
@@ -673,7 +674,7 @@ function argumentsToAddDependenciesWithBun(dependencies: string[], type: Depende
 export async function findUpAndReadPackageJson(fromDirectory: string): Promise<{path: string; content: PackageJson}> {
   const packageJsonPath = await findPathUp('package.json', {cwd: fromDirectory, type: 'file'})
   if (packageJsonPath) {
-    const packageJson = JSON.parse(await readFile(packageJsonPath))
+    const packageJson = parseJSON<PackageJson>(await readFile(packageJsonPath), packageJsonPath)
     return {path: packageJsonPath, content: packageJson}
   } else {
     throw new FindUpAndReadPackageJsonNotFoundError(fromDirectory)