Slightly improve KV error messages to be clearer and more actionable#14436
Slightly improve KV error messages to be clearer and more actionable#14436dario-piotrowicz wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: f20f39b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
james-elicx
and removed request for
a team
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR rewords KV namespace/key error messages to be clearer and more actionable, updating the matching test snapshots.
One issue worth a look:
- The new message in
getIdFromSettingsmay give misleading guidance for the provisioning path (details inline).
| if (!existingKV || !("namespace_id" in existingKV)) { | ||
| throw new UserError( | ||
| `No namespace ID found for binding "${binding}". Add one to your wrangler config file or pass it via \`--namespace-id\`.`, | ||
| `No KV namespace ID found for binding "${binding}" in the deployed Worker settings for "${config.name}". Add an "id" to the "${binding}" entry in your config's "kv_namespaces" array, or use \`--namespace-id\` instead.`, |
There was a problem hiding this comment.
This branch is only reachable through the RESOURCES_PROVISION flow (called from lines 543 and 566), which by design handles bindings that have no id/preview_id in config and resolves the namespace ID from the deployed Worker's settings instead. In that situation, advising the user to "Add an id to the kv_namespaces array" contradicts the provisioning workflow they're relying on — they intentionally don't have an id yet, and the namespace simply isn't deployed.
Consider keeping the guidance focused on the actual cause (binding not found in the deployed Worker / not yet deployed) and the --namespace-id escape hatch, without pushing them to add an id:
| `No KV namespace ID found for binding "${binding}" in the deployed Worker settings for "${config.name}". Add an "id" to the "${binding}" entry in your config's "kv_namespaces" array, or use \`--namespace-id\` instead.`, | |
| `No KV namespace ID found for binding "${binding}" in the deployed Worker settings for "${config.name}". Deploy the Worker so the binding is provisioned, or use \`--namespace-id\` instead.`, |
|
I've reviewed PR #14436 and posted a review with one inline suggestion. Summary: The PR is a clean cosmetic improvement to KV error messages with matching test snapshot updates. The one actionable issue I flagged is that the new message in |
|
✅ All changesets look good Reviewed changesets:
All changesets avoid h1/h2/h3 headers and provide meaningful, user-facing descriptions. No analytics or experimental opt-in concerns identified. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
@cloudflare/autoconfig
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
2 participants
Error messages for KV namespace and key operations now consistently explain what went wrong, which flags or config fields to use, and what commands to run as alternatives. This covers namespace selection errors (delete, rename), binding resolution errors, config file issues, and preview namespace ambiguity.
A picture of a cute animal (not mandatory, but encouraged)