docs(speckit): human-readable wallet error messages spec (#179)#292
Conversation
Draft spec for issue #179 — surface short, user-friendly error messages for common wallet failures instead of raw backend/transport errors. Server-side approach: add GuardianError::user_message(), surfaced additively on the HTTP error envelope and gRPC Status details; Rust/TS clients consume it and classify codeless transport failures (pattern from 0xMiden/wallet #273).
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #292 +/- ##
==========================================
- Coverage 76.88% 76.57% -0.31%
==========================================
Files 153 154 +1
Lines 27373 27662 +289
==========================================
+ Hits 21045 21182 +137
- Misses 6328 6480 +152 Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new Speckit feature specification for surfacing short, sanitized, end-user-safe error messages to wallet clients, while keeping existing machine-readable error codes and status mappings stable and additive across HTTP and gRPC.
Changes:
- Introduces a draft spec for adding a
user_messagealongside existingcode/errorfields. - Defines user stories, acceptance scenarios, and contract requirements for HTTP + gRPC parity and client consumption.
- Proposes a starter copy catalog and outlines connectivity vs server-authored messaging responsibilities.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| When Guardian hits an internal fault (storage error, signing error, configuration error, degraded data), the wallet user must see a single safe, generic message — never a file path, a raw RPC string, a stack trace, or a database detail. The diagnostic detail remains available to operators/logs via the unchanged `error` field, but it is never the message the wallet presents. | ||
|
|
||
| **Why this priority**: This is both a UX requirement and a security/disclosure requirement (Constitution Principle IV: stable boundary errors). Raw internal text leaking to an untrusted wallet client is a disclosure risk that exists today. | ||
|
|
||
| **Independent Test**: Trigger each internal variant (`StorageError`, `SigningError`, `ConfigurationError`, `DataUnavailable`, `NetworkError`) and assert the `user_message` equals the generic safe string, contains no path/URL/hash/raw-error fragment, while the `error` field still carries the diagnostic detail for logging. | ||
|
|
||
| **Acceptance Scenarios**: | ||
|
|
||
| 1. **Given** the storage backend fails, **When** any wallet request errors, **Then** `user_message` is the generic "Something went wrong on Guardian's side. Please try again." and the diagnostic text appears only in `error`. | ||
| 2. **Given** an upstream RPC returns a raw transport error, **When** Guardian maps it to `RpcUnavailable`, **Then** `user_message` is a connectivity-style sentence with the raw upstream text stripped, while `error` retains the raw text. | ||
| 3. **Given** any internal variant, **When** the `user_message` is scanned, **Then** it matches none of the disallowed patterns (hex commitments/IDs, file paths, `http(s)://` URLs, "error:" fragments from upstream). |
|
|
||
| - **FR-001**: `GuardianError` MUST expose a `user_message()` accessor returning a short, plain-language, end-user-safe string for **every** variant. The set of `code()` values and their HTTP/gRPC status mappings MUST remain unchanged by this feature. | ||
| - **FR-002**: `user_message()` output MUST be safe-by-construction: it MUST NOT contain account IDs, commitments, nonces, signer IDs, file paths, URLs, raw upstream/RPC error text, or any value interpolated from the variant's payload fields. (Contrast with `Display`, which may.) | ||
| - **FR-003**: Internal and non-user-actionable variants (`StorageError`, `SigningError`, `ConfigurationError`, `DataUnavailable`, and the internal portion of `NetworkError`/`RpcUnavailable`) MUST map to a single shared generic message (e.g. "Something went wrong on Guardian's side. Please try again."). User-actionable variants MUST map to a category-appropriate message (see Key Entities catalog). |
zeljkoX
left a comment
zeljkoX
left a comment
There was a problem hiding this comment.
Since Guardian is still early-phase with no external consumers locked to the current contract, I think we should treat this as a breaking change rather than add user_message additively. The additive path keeps two human-ish strings whose names don't say which is safe to show, plus a flat envelope that grows an optional field per variant. A clean reshape is cheap now.
Proposed wire shape — one object, identical on the HTTP body and the gRPC Status.details:
{
"code": "authorization_failed",
"message": "You're not an authorized signer for this account.",
"meta": { "retryable": false }
}
- code — stable machine code (existing vocabulary, unchanged). Branch key + i18n key.
- message — user-safe sentence, always present and safe to display. Only code is stable; wording can change.
- meta — structured machine data; holds the fields that are top-level optionals today (retryable, retry_after_secs, missing_permissions, paused_at, paused_reason). Omitted when empty.
The diagnostic string stays off the wire — server logs only. The Display text (account IDs, commitments, postgres://…, raw RPC errors) is exactly what we don't want to hand an untrusted wallet. Logging it instead of returning it removes the disclosure risk by construction. This also frees us to name the field plainly message (the old diagnostic error field is gone). We also drop success (the HTTP status is the discriminator; gRPC never had it).
Examples:
{ "code": "rate_limit_exceeded", "message": "Too many requests — please try again shortly.", "meta": { "retryable": true, "retry_after_secs": 30 } }
{ "code": "storage_error", "message": "Something went wrong on Guardian's side. Please try again.", "meta": { "retryable": true } }
gRPC: Status.code unchanged, Status.message becomes the user-safe message, and Status.details carries the same {code, message, meta} object. Details is still needed because the ~16 canonical gRPC codes can't recover our 37 stable code values (e.g. authorization_failed / signer_not_authorized both map to PERMISSION_DENIED).
Adopt @zeljkoX's breaking reshape to a single {code, message, meta} object, identical on the HTTP body and gRPC Status.details. Drop the diagnostic `error` field (logs only) and `success`; rename user_message -> message; fold top-level optionals into meta. Also split server-mapped connectivity faults (network_error, rpc_unavailable, rpc_validation_failed) from pure internal faults, resolving the Copilot review's FR-003/User-Story-2 inconsistency.
|
@zeljkoX Thanks for the feedback, pushed an update to reshape the spec accordingly:
Three things still open in the spec if you have a preference:
|
|
LGTM |
What
Draft spec only (no implementation yet) for issue #179 — surface short, user-friendly error messages for common wallet failures instead of raw backend/transport errors.
Adds
speckit/features/009-human-readable-errors/spec.mdfollowing the house spec-kit format.Refs #179.
Approach (proposed)
Server-side single source of truth:
GuardianError::user_message()— a short, sanitized sentence per variant (no account IDs / commitments / paths / raw upstream text).user_messageon the HTTP error envelope and the gRPCStatusdetails (generalizing the existingGUARDIAN_ACCOUNT_PAUSEDdetails pattern).code,error, and all status mappings stay frozen.connectivity-classify.ts).code; server ships English defaults.Anchored to the constitution: HTTP/gRPC parity (II), bottom-up propagation to Rust+TS clients (I), additive/stable boundary errors (IV). Bonus: stops the current raw
errorstrings (which embed IDs/commitments/raw RPC text) from leaking to wallet clients.Open questions for reviewers (@MCarlomagno @zeljkoX)
user_messagevshintvsdisplay_message.codefor internal errors, or collapse to oneinternal_error.The 2026-06-18 clarifications in the spec are marked proposed — pending confirmation.
Not included
Implementation,
plan.md/data-model.md/contracts/. Those follow once direction is confirmed (perAGENTS.md§4 contract-change workflow: proto + OpenAPI regen → Rust client → TS clients → multisig SDK → examples).Summary by CodeRabbit