ClickHouse / ClickHouse/clickhousectl

Emit structured error envelopes for every Cloud failure in JSON mode

Open
#825 0 comments 0 reactions 0 assignees View on GitHub
bug cloud
Dominant language
Rust
Stars
74
Forks
5
Avg merge
2d 11h
Merged PRs (30d)
196

Description

## Validated scope and current-PR plan — 2026-09-11

This section records the QA review and the current decision to finalize existing PRs without opening new PRs. It supersedes the proposed routing/fix suggestions in the original report below; the original observations are retained. Central plan: #757.

### Disposition

Deferred from this existing-PR finalization pass. Keep the issue open; no new PR is requested now. This is not a resolution, a severity downgrade, or a waiver of release acceptance.

### Validation and corrections

Confirmed with a mock 400: explicit JSON still prints prose. [mod.rs:112](https://github.com/ClickHouse/clickhousectl/blob/d2c37f814fff53fe68d636e34103e2d0550bf84d/crates/clickhousectl/src/cloud/mod.rs#L112) drops structured context for generic errors. Preserve auth exit 4 and cancellation exit 3 explicitly; blindly converting everything to the existing CloudDetailed variant is not sufficient. Keep separate from [#771](https://github.com/ClickHouse/clickhousectl/pull/771)’s service-delete conflict classification.

---

## Original QA report

Found in the 2026-09-11 QA sweep of the combined PR stack at [`d2c37f81`](https://github.com/ClickHouse/clickhousectl/commit/d2c37f814fff53fe68d636e34103e2d0550bf84d), top PR #823, installed release build (package version still 0.4.2). Reproduced behaviour only; no implementation included.

Severity: high; cross-cutting. Every cloud scope in the sweep hit this.

Only errors carrying a `CloudErrorDetail` render as `{"error":{…}}` in JSON mode. Every other failure — API 4xx/5xx, client-side validation, wrong `--org-id`, SQL errors from `service query`, the no-credentials exit-4 message — prints `Error: ` to stderr with empty stdout, under an explicit `--json` and under agent auto-detection alike. A machine consumer has to string-match English.

```sh
clickhousectl cloud service scaling-schedule get "$SVC" --json; echo $?
# stderr: Error: NOT_FOUND: no autoscaling schedule found for service … (exit 1, no JSON)
clickhousectl cloud service get 11111111-2222-3333-4444-555555555555 --json
# {"error":{"code":"resource_not_found", …}} (JSON — the only class that does)
clickhousectl cloud clickpipe context get "$SVC" --json 1>/tmp/out 2>/tmp/err; wc -c /tmp/out # 0
```

Local JSON mode already solves this: `local::output::print_error` wraps *every* `Error` through one exhaustive `LocalErrorOutput::from_error` match. Cloud has no equivalent because `cloud_error_to_top_level` collapses a `CloudError` without details into `Error::Cloud(String)`, discarding the `kind` and `failure: Option` classification the error already carries.

## Proposed design

- `cloud_error_to_top_level` always produces `Error::CloudDetailed`. `code` comes from closed vocabularies only: the existing `CloudErrorCode` when a detail is present; otherwise derived from `FailureKind` (`http_4xx`, `http_5xx`, `rate_limited`, `transport`, `timeout`, `sql_error`, `service_stopped`, `io`, `other`) plus `auth_required` (exit 4) and `cancelled` (exit 3). All `&'static str` from enums, consistent with the telemetry invariant.
- `message` stays today's prose, so human output is byte-identical and `main.rs` no longer needs a mode-specific special case.
- Envelope on stderr, stdout empty on failure — matching the local contract and the existing `cloud::output::print_error`. Document that in the README JSON section. Clap usage errors keep clap's format, as locally.

Acceptance: wiremock subprocess tests asserting an `{"error":{"code","message"}}` envelope for each failure class (401/403, 404, 400, 429, 5xx, transport, client-side validation), exit codes unchanged, human output unchanged.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with crates/clickhousectl/src/cloud/mod.rs at the referenced line and trace cloud_error_to_top_level, then compare its behavior with local::output::print_error and the main.rs handling. Review the README JSON section and existing wiremock subprocess tests; done means every listed failure class has the specified stderr envelope, unchanged exit codes, and unchanged human output.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, cli, cloud, documentation, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.