From d5f7f60fb82144c1003ddb9815daa138b9fab319 Mon Sep 17 00:00:00 2001 From: DavidWells Date: Mon, 6 Jul 2026 10:56:06 -0700 Subject: [PATCH] fix(api): teach instead of leaking SyntaxError on malformed --data 'netlify api getSite --data site_id=123' leaked a raw SyntaxError. The error now names the flag, echoes the bad input, and shows a correct JSON example. 'Missing required path variable' errors list the method's required path parameters. --- src/commands/api/api.ts | 27 +++++++++++- tests/unit/commands/api/api.test.ts | 66 +++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/unit/commands/api/api.test.ts diff --git a/src/commands/api/api.ts b/src/commands/api/api.ts index 370c3a3730c..8696cee9fc1 100644 --- a/src/commands/api/api.ts +++ b/src/commands/api/api.ts @@ -40,7 +40,21 @@ export const apiCommand = async (apiMethodName: string, options: OptionValues, c let payload if (options.data) { - payload = typeof options.data === 'string' ? JSON.parse(options.data) : options.data + if (typeof options.data === 'string') { + try { + payload = JSON.parse(options.data) + } catch { + const received = options.data.length > 80 ? `${options.data.slice(0, 80)}…` : options.data + return logAndThrowError( + `Invalid JSON provided to the ${chalk.cyanBright('--data')} flag. +Received: ${received} +The --data flag expects a JSON object of API parameters, e.g. --data '{"site_id":"123456"}'. +Note: key=value pairs are not accepted; use JSON syntax instead.`, + ) + } + } else { + payload = options.data + } } else { payload = {} } @@ -48,6 +62,17 @@ export const apiCommand = async (apiMethodName: string, options: OptionValues, c const apiResponse = await apiMethod(payload) logJson(apiResponse) } catch (error_) { + if (error_ instanceof Error && error_.message.includes('Missing required path variable')) { + const apiMethods = methods as { operationId: string; parameters: { path?: Record } }[] + const pathVariables = apiMethods.find((method) => method.operationId === apiMethodName)?.parameters.path ?? {} + const requiredNames = Object.keys(pathVariables).join(', ') + return logAndThrowError( + `${error_.message} +The ${chalk.cyanBright('--data')} flag must include the path variable(s) required by ${apiMethodName}${ + requiredNames ? `: ${requiredNames}` : '' + }, e.g. --data '{"site_id":"123456"}'`, + ) + } return logAndThrowError(error_) } } diff --git a/tests/unit/commands/api/api.test.ts b/tests/unit/commands/api/api.test.ts new file mode 100644 index 00000000000..1750305fb29 --- /dev/null +++ b/tests/unit/commands/api/api.test.ts @@ -0,0 +1,66 @@ +import type { OptionValues } from 'commander' +import { beforeEach, describe, expect, test, vi } from 'vitest' + +vi.mock('../../../../src/utils/telemetry/report-error.js', () => ({ + reportError: vi.fn().mockResolvedValue(undefined), +})) + +import { apiCommand } from '../../../../src/commands/api/api.js' +import type BaseCommand from '../../../../src/commands/base-command.js' + +const getSite = vi.fn() + +const command = { netlify: { api: { getSite } } } as unknown as BaseCommand + +const runApi = async (data: string) => apiCommand('getSite', { data } as OptionValues, command) + +const captureError = async (data: string): Promise => { + try { + await runApi(data) + } catch (error) { + return error as Error + } + throw new Error('expected apiCommand to throw') +} + +describe('apiCommand --data parsing', () => { + beforeEach(() => { + getSite.mockReset() + }) + + test('rejects key=value input with an error naming --data', async () => { + await expect(runApi('site_id=123')).rejects.toThrowError(/--data/) + expect(getSite).not.toHaveBeenCalled() + }) + + test('error echoes the offending input and a concrete JSON example', async () => { + const error = await captureError('site_id=123') + expect(error.message).toContain('site_id=123') + expect(error.message).toContain(`--data '{"site_id":"123456"}'`) + expect(error.message).toContain('key=value') + expect(error.message).not.toContain('SyntaxError') + }) + + test('truncates long offending input in the error message', async () => { + const longInput = `site_id=${'9'.repeat(200)}` + const error = await captureError(longInput) + expect(error.message).not.toContain(longInput) + expect(error.message).toContain(longInput.slice(0, 80)) + }) + + test('still accepts a valid JSON object string', async () => { + getSite.mockResolvedValue({ id: '123456' }) + + await runApi('{"site_id":"123456"}') + + expect(getSite).toHaveBeenCalledWith({ site_id: '123456' }) + }) + + test('names --data and the required variable when the API reports a missing path variable', async () => { + getSite.mockRejectedValue(new Error("Missing required path variable 'site_id'")) + + const error = await captureError('{"other":"value"}') + expect(error.message).toContain('--data') + expect(error.message).toContain('site_id') + }) +})