Skip to content

Fix: cloud service delete surfaces NOT_FOUND instead of exiting 0 - #618

Open
sdairs wants to merge 1 commit into
fix/609-local-init-json-pathsfrom
fix/601-service-delete-not-found
Open

Fix: cloud service delete surfaces NOT_FOUND instead of exiting 0#618
sdairs wants to merge 1 commit into
fix/609-local-init-json-pathsfrom
fix/601-service-delete-not-found

Conversation

@sdairs

@sdairs sdairs commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

What

clickhousectl cloud service delete <nonexistent-id> printed null and exited 0 instead of failing. CloudClient::delete_service_if_exists swallowed any 404 from the delete endpoint into Ok(None), and the handler rendered that as null (with --json) or "Service X is already absent" (human output) with exit code 0.

Why

Exit 0 on a delete of a nonexistent service misleads scripts/agents into believing the delete succeeded. cloud service get on the same missing ID already fails correctly with Error: NOT_FOUND: ... and exit 1 — delete should behave the same way.

Fix

Replaced delete_service_if_exists (which special-cased 404 by additionally checking the organization still existed, then swallowing the service-not-found into Ok(None)) with a strict delete_service that maps errors via convert_error_for_organization exactly like get_service does, so a 404 propagates as a normal CloudError (exit code 1, NOT_FOUND: ... message, with the same organization-scoping note get/list already add for a bare NOT_FOUND).

Tests

  • Updated forced_service_delete_treats_an_absent_key_and_service_as_success (renamed to forced_service_delete_surfaces_not_found_for_an_absent_service) to assert exit 1 / NOT_FOUND error and that local query-key cleanup and the org-scoped key deletion are not attempted after a failed delete.
  • Updated service_delete_does_not_treat_a_missing_organization_as_an_absent_service to reflect that delete no longer performs an extra organization-existence probe before failing.
  • Added service_delete_preserves_a_detailed_not_found_error, mirroring the existing service_get_preserves_a_detailed_not_found_error coverage, to pin that a detailed API-provided NOT_FOUND message passes through verbatim.

Verified locally: cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, cargo test -p clickhousectl, and cargo test -p clickhouse-cloud-api --test spec_coverage_test (unaffected crate, sanity only).

Fixes #601

Note: this PR is part of a stacked chain and targets fix/609-local-init-json-paths as its base branch, not main.

🤖 Generated with Claude Code

@sdairs
sdairs force-pushed the fix/601-service-delete-not-found branch from e6683fb to c593e95 Compare August 28, 2026 14:46
@sdairs sdairs linked an issue Aug 28, 2026 that may be closed by this pull request
@sdairs sdairs linked an issue Aug 28, 2026 that may be closed by this pull request
@sdairs
sdairs force-pushed the fix/601-service-delete-not-found branch from c593e95 to 9562a91 Compare August 28, 2026 19:42
…exiting 0

`instance_delete` silently swallowed a 404 into `Ok(None)`, which
`service_delete` rendered as `null`/exit 0 (or "already absent" in
human output) instead of surfacing the NOT_FOUND error, misleading
scripts into thinking a delete of an already-deleted service
succeeded. `CloudClient::delete_service` now propagates the 404 the
same way `get_service` does, so `service delete` on a nonexistent ID
fails with `Error: NOT_FOUND: ...` and exit code 1, consistent with
`service get`.

Fixes #601

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sdairs
sdairs force-pushed the fix/601-service-delete-not-found branch from 9562a91 to c7bd2f7 Compare September 1, 2026 20:36

@iskakaushik iskakaushik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cloud service delete on a nonexistent service prints null and exits 0

2 participants