fix(api): teach instead of leaking SyntaxError on malformed --data

[DavidWells]

Split from #8300 (5 of 9).

What

netlify api getSite --data 'site_id=123' used to leak a raw parser error:

 ›   SyntaxError: Unexpected token 's', "site_id=123" is not valid JSON

Now:

 ›   Error: Invalid JSON provided to the --data flag.
Received: site_id=123
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.

"Missing required path variable" API errors also now list the method's required path parameters.

Why it helps agents

We found a real agent session with 18 consecutive failures on exactly this key=value vs JSON confusion. An error that names the flag, echoes the input, and shows a working example turns that into one failure and one self-correction.

Testing

• New unit tests for both error paths
• typecheck ✓ lint ✓ full unit suite ✓

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved --data handling for API commands with clearer validation for JSON input.
  • Invalid --data values now show a helpful error with a sample JSON format and a preview of the problematic input.
  • Added clearer guidance when required path variables are missing, including which values to provide.
  • Accepted JSON input is now passed through correctly, and empty --data defaults to an empty object.

Walkthrough

The apiCommand implementation in src/commands/api/api.ts now wraps --data JSON parsing in a try/catch, throwing a descriptive error with a truncated preview when parsing fails. The catch block for API invocation errors was extended to detect "Missing required path variable" errors, compute required path variables from OpenAPI method definitions, and throw a --data-specific error with guidance and an example payload; other errors fall back to prior behavior. A new unit test file validates these behaviors against the getSite command.

Estimated code review effort: 2 (Simple) | ~12 minutes

Changes

File	Summary
src/commands/api/api.ts	Added try/catch JSON parsing for --data with detailed error messaging; extended error handling for missing required path variables.
tests/unit/commands/api/api.test.ts	New test suite validating --data parsing, error messages, truncation, and missing path variable handling.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant apiCommand
  participant JSONParse
  participant NetlifyAPI

  User->>apiCommand: run(apiMethodName, options.data)
  apiCommand->>JSONParse: JSON.parse(options.data)
  alt parse fails
    JSONParse-->>apiCommand: throws SyntaxError
    apiCommand-->>User: throw invalid --data JSON error
  else parse succeeds
    JSONParse-->>apiCommand: payload
    apiCommand->>NetlifyAPI: call apiMethodName(payload)
    alt Missing required path variable
      NetlifyAPI-->>apiCommand: throws Missing required path variable error
      apiCommand-->>User: throw --data path variable error with example
    else other error
      NetlifyAPI-->>apiCommand: throws error
      apiCommand-->>User: logAndThrowError(error)
    end
  end

Loading

Related issues: None specified in the provided context.

Related PRs: None specified in the provided context.

Suggested labels: cli-api, tests

Suggested reviewers: None specified in the provided context.

🐰 A rabbit typed a payload with care,
JSON braces dancing in the air,
When "--data" goes wrong it won't despair,
It shows the fix with an example fair,
Tests confirm each burrow-tested snare!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately highlights the main change: improved handling of malformed --data input instead of surfacing a raw SyntaxError.
Description check	✅ Passed	The description matches the implemented changes and testing, including malformed --data handling and missing path variable errors.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch split/api-data-errors

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📊 Benchmark results

Comparing with 3c41e72

• Dependency count: 1,127 (no change)
• Package size: 379 MB ⬇️ 0.00% decrease vs. 3c41e72
• Number of ts-expect-error directives: 359 (no change)

[coderabbitai]

🧹 Nitpick comments (1)

tests/unit/commands/api/api.test.ts (1)

59-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test relies on real OpenAPI methods data rather than a mock.

This test depends on the actual getSite operation definition in methods containing site_id under parameters.path, since methods isn't mocked. This makes the test implicitly coupled to the real OpenAPI spec content — if the spec changes ordering/naming, this test could fail for reasons unrelated to the change under test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/commands/api/api.test.ts` around lines 59 - 65, The test is
implicitly coupled to the real OpenAPI spec because it depends on getSite and
methods containing site_id in parameters.path. Update the test setup to mock the
relevant operation metadata (or the methods lookup) so captureError exercises
the missing-path-variable behavior without relying on the live spec content.
Keep the assertions focused on the error formatting from captureError and the
getSite path-variable handling, not on the actual OpenAPI definition.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unit/commands/api/api.test.ts`:
- Around line 59-65: The test is implicitly coupled to the real OpenAPI spec
because it depends on getSite and methods containing site_id in parameters.path.
Update the test setup to mock the relevant operation metadata (or the methods
lookup) so captureError exercises the missing-path-variable behavior without
relying on the live spec content. Keep the assertions focused on the error
formatting from captureError and the getSite path-variable handling, not on the
actual OpenAPI definition.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 09e8d67a-3cfd-489d-8b8f-09378912381b

📥 Commits

Reviewing files that changed from the base of the PR and between 3c41e72 and d5f7f60.

📒 Files selected for processing (2)

• src/commands/api/api.ts
• tests/unit/commands/api/api.test.ts

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• netlify/blueprints (manual)