Report missing databases and roles on delete instead of empty output - #548
Open
andrelandgraf wants to merge 8 commits into
Open
andrelandgraf wants to merge 8 commits into
andrelandgraf wants to merge 8 commits into
Conversation
A typo currently looks like a successful delete: exit 0 and no bytes. Databases stay idempotent (exit 0); a missing role is usually a mistake so that command exits 1 after writing the same not-found payload.
roles delete of a missing name now exits 1, which is a behavior change for scripts that treated 204 as success.
…help. JSON consumers of databases delete cannot use the exit code to tell a 204 from a 200, so the payload needs a discriminant. Help now states the db-0 / role-1 contract.
…arameter. The db-0 / role-1 contract stays; it now lives in one helper instead of two copied handlers.
A 204 from either delete is not-found: table ERROR on stderr, JSON/YAML
{deleted:false} on stdout, exit 1 for both nouns.
Successful deletes go back to branchIdFromProps, so a br- id does not list branches before DELETE.
The not-found payload must not depend on listing branches after the 204. Use the name the user passed, or the id already resolved.
Other delete subcommands only name the noun. The new exit-1 behavior belongs in the PR, not in two help strings that imply the rest differ.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
neon databases delete <name>andneon roles delete <name>for a name that is already gone print nothing and exit 0. The API answers the DELETE with HTTP 204 and no body, and both commands only write output when a 200 body arrives, so the missing case falls through silently.A script that deletes and then checks
$?sees success. A person at the terminal sees an empty line and has to rundatabases listto learn whether anything happened.Diagnosis
On
origin/mainboth delete handlers end inif (data) { writer(props).end(...) }. With a 204 there is no record to print and no else branch, so the command returns normally and the process exits 0.The fix keys off
statusrather than the presence ofdata. Some clients parse an empty 204 body as{}, which is truthy, sostatus === 200 && data?.database(ordata?.role) is the condition for the success path. Everything else is treated as "the target was not there".Reporting has to differ by output mode because of how the CLI's error path works. Throwing an
Errormakes the top-level handler printERROR: <message>on stderr and exit 1, which is the right shape for table mode. In JSON and YAML mode the caller is parsing stdout, so the result goes through the normal writer as an object andprocess.exitCodeis set to 1 instead of throwing. Both handlers share this inpackages/cli/src/utils/missing_delete.ts.The branch in the message is built from values already in hand. If
--branchwas a name, the message uses that name. If it was abr-…id, or was omitted, the message uses the id that was resolved for the DELETE. There is no second list call after the 204.Interface
Table mode (default at a terminal):
ERROR:on stderr, empty stdout, exit 1.JSON: object on stdout, empty stderr, exit 1.
YAML emits the same two keys,
deleted: falseandmessage.roles deletebehaves identically withRolein the message.Which branch label appears in the message:
Unchanged for existing callers:
branchIdFromProps, prints the deleted record with the same fields, and exits 0.Delete a databaseandDelete a role, matching the other delete subcommands.Also in here
packages/cli/src/utils/missing_delete.ts:reportMissingDelete(mode-dependent reporting) andbranchNameForMissingDelete(name vs id selection using the existinglooksLikeBranchId). Both handlers call it so the two nouns cannot drift.DELETE.204.jsonmock files under the existing mock branch, fordatabases/nosuchdbandroles/nosuchrole, so the test fixture serves a 204 for those paths.neonminor. Exit code changes from 0 to 1 for a case that used to succeed silently.Verification
New tests, each run for both
databases deleteandroles delete:{ deleted: false, message }, stderr empty, exit 1ERROR: <message>, stdout empty, exit 1{ deleted: false, message }, stderr empty, exit 1Live run against a throwaway project: deleting a missing database and a missing role in table and JSON mode produced the outputs shown above and exited 1. The project was deleted afterwards.
Not covered by an automated test: the
br-…id and omitted--branchvariants of the message. The fixture passes a branch name; the id path is a direct branch ofbranchNameForMissingDeleteand was checked in the live run.For your attention
--if-exists. A flag to opt back into exit 0 for a missing target is a separate decision and is not in this PR.--yes/ confirmation prompt. Out of scope.databases deleteandroles deletereturn 204 for a missing name through this code path; other delete subcommands were not touched.