refactor: address review feedback on TaskError migration — jargon, transparent errors, variant cleanup by Copilot · Pull Request #753 · dashpay/dash-evo-tool

Copilot · 2026-03-14T11:11:01Z

Follow-up to #739 addressing seven review findings: jargon in error messages, #[error(transparent)] forwarding internal error text to users, a semantically wrong variant, misrouted DashPay errors, a redundant variant, and stale CLAUDE.md docs.

Changes

`src/backend_task/error.rs`

Spv / GroveStark / JoinError: Replace #[error(transparent)] (which leaked e.g. "SPV lock poisoned" to users) with user-friendly messages. Spv gets a spv_user_message() helper for per-variant guidance:
```
#[error("{}", spv_user_message(.0))]
Spv(#[from] crate::spv::SpvError),
```
SdkInitializationFailed: Remove "SDK" jargon → "Could not connect to the Dash network. Please check your network settings and restart the application."
IdentitySaveError: Removed — duplicate of Database with the same rusqlite::Error source and no semantic distinction. All 9 callsites updated to TaskError::Database { source: e }.

`src/backend_task/contested_names/query_dpns_contested_resources.rs`

document_type_for_name("domain") failure was mapped to DataContractNotFound — wrong, the contract was found. Changed to ContractSchemaMismatch { detail: "DPNS contract missing 'domain' document type" }, consistent with the contested-index check already using that variant in the same function.

DashPay files (`profile.rs`, `contact_requests.rs`, `contact_info.rs`, `contacts.rs`, `auto_accept_handler.rs`)

All DpnsFetchError usages were DocumentQuery::new(...) failures — query construction errors with no relation to DPNS name lookup. Replaced with DashPayError::QueryCreation { query_target: "…", source } with descriptive targets ("DashPay profile", "DashPay contactRequest", "DashPay contactInfo", "DPNS domain").

`CLAUDE.md`

Removed the false From<String> claim. Replaced with: "Domain errors (DashPayError, SpvError, etc.) are wired as #[from] variants for automatic conversion via ?. When adding new backend error types, add a dedicated TaskError variant rather than converting to String."

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…aveError, DpnsFetchError, CLAUDE.md Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>

) * docs: promote i18n-ready strings to general coding rule (#660) Move the i18n guideline out of the error-messages section into general rules, broadening its scope to all user-facing strings (labels, tooltips, messages). Remove direct Fluent reference, fix placeholder syntax to Rust-style `{ name }`, and renumber remaining error rules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: add typed error variants for migration (#660) Add new TaskError variants (LockPoisoned, Database, CoreRpc, P2P), new domain error types (P2PError, WalletError::KeyDerivation, WalletError::Sighash, DashPayError::QueryCreation, DashPayError::CryptoKeyParsing, DashPayError::PrivateKeyResolution, ConfigError::SaveError), and replace the transparent Sqlite variant with a user-friendly Database variant. Drop Clone/PartialEq from DashPayError to support #[source] fields and fix the two downstream callsites that depended on them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: replace string errors with typed errors in spv/config/infra (#660) - core_p2p_handler: eliminate ReadMessageError(String), use P2PError variants throughout internal methods; keep public API returning String for callers outside scope - config: replace ConfigError::LoadError with SaveError in save() for all I/O operations (writeln, sync_all, persist) - backend_task/mod: add map_err(TaskError::from) bridges for wallet task functions that still return String - model/wallet: fix KeyDerivation source type mismatch (key_wallet::Error != bip32::Error) by reverting to e.to_string() at derivation_path() callsites Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: replace string errors with typed TaskError in model/database/context (#660) - Use WalletError::AddressError in utxos.rs for dashcore address errors - Use TaskError::from(e) for RPC errors in broadcast_raw_transaction Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: replace string errors with typed TaskError in backend_task/ (#660) Migrate callsites in src/backend_task/ from Result<T, String> to Result<T, TaskError>: - broadcast_state_transition: map SDK error via TaskError::from - contract: direct ? for rusqlite, TaskError::from for SDK errors - document: all 7 document operations migrated - mod.rs: run_wallet_task return type updated to TaskError - register_contract: sender error → InternalSendError, SDK → TaskError::from - update_data_contract: sender error → InternalSendError, format strings → Generic - wallet/*: lock poison → Generic, SDK errors → TaskError::from, ? for rusqlite Build, clippy, and fmt all pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: replace string errors with typed TaskError in ui/ (#660) Migrate rusqlite error callsites in src/ui/ to typed TaskError variants: - `IdentityTokenInfo::try_from_*` functions in tokens_screen/structs.rs now return `Result<Self, TaskError>` — DB errors become `TaskError::Database` via `?` instead of `.map_err(|e| e.to_string())`. Callers use `MessageBanner::set_global` with the error which accepts `impl Display`, so no caller changes were needed. - `save_private_info` in contact_profile_viewer.rs now returns `Result<(), TaskError>` — the rusqlite error is wrapped explicitly as `TaskError::Database { source: e }` for clear error semantics. All other `map_err(|e| e.to_string())` callsites in src/ui/ were reviewed and left unchanged — they are in UI-local validation functions (clipboard, form validation, address generation) that do not feed into the backend task error system. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove useless TaskError::from conversions in wallet task dispatch (#660) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: fix all unresolved review comments — proof errors, asset lock, format! migrations (#740) * Initial plan * refactor: add typed TaskError variants, eliminate Generic from backend_task files Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: eliminate Result<T,String> from context/transaction_processing, add typed TaskError variants Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: migrate context/wallet_lifecycle.rs from Result<T,String> to Result<T,TaskError> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: add source_error to ProofError, add log_drive_proof_error helper, migrate token tasks to TaskError Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: replace TaskError::Internal with granular variants, migrate identity tasks to TaskError Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * style: apply cargo +nightly fmt to fix CI formatting failures Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: fix all unresolved review comments (Claude + lklimek) Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: remove TaskError::Generic and TaskError::Other, replace with specialized variants (#744) * Initial plan * refactor: remove TaskError::Generic and TaskError::Other, add specialized variants - Remove Generic(String) and Other(Box<dyn Error>) variants from TaskError - Add SerializationError, IdentifierParsingError, IdentityCreationError, InvalidPrivateKey, DpnsFetchError, AssetLockNotValidForWallet, AssetLockInstantLockExpiredNotChainlocked, PlatformFetchError variants - Update From<String> bridge to map to UserInput instead of Generic - Replace TaskError::Generic("Identity not found") with IdentityNotFoundLocally - Replace TaskError::Generic("Contract not found") with DataContractNotFound - Replace TaskError::Generic(e.to_string()) for bincode with SerializationError - Fix instant-lock fallback to use Debug repr instead of Display for string matching - Fix poisoned lock spinning in transaction_processing.rs - Fix silently dropped wallet locks in load_identity.rs and load_identity_by_dpns_name.rs - Change contract_token_db::remove_wallet to return TaskError instead of String - Use typed variants in register_identity.rs and top_up_identity.rs - Use ? operator for lock errors in core/mod.rs Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: address remaining PR 739 review comments - Fix wallet removal atomicity: acquire write lock before DB removal - Fix proof-log write failures: warn instead of silently dropping - Fix domain jargon in AssetLock* error messages (no "asset lock", "instant lock") - Fix token metadata update atomicity: use upsert instead of remove+insert Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: address code review comments - improve error messages and add TODO notes Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: replace UserInput catch-all with granular DashCoreStartError, OperationNotAvailableOnNetwork, and LegacyError variants Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: improve DashCoreStartError message and simplify OperationNotAvailableOnNetwork display Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: address PR #739 review comments — structural error matching, lock poisoning cleanup, parameterized RequestType - Parameterize `log_drive_proof_error` with `RequestType` instead of hardcoding `BroadcastStateTransition` (26 callsites across 15 files) - Replace all `map_err` to `LockPoisoned` with `?` operator using blanket `From<PoisonError<T>>` impl; add doc comment explaining the intentional design - Change `CreditCalculationOverflow` variant to carry raw data fields (`amount`, `credits_per_duff`) instead of pre-formatted text - Add `TaskError::AssetLockInstantLockProofInvalid` variant with structural SDK error matching, replacing fragile `format!("{:?}").contains(...)` in register_identity and top_up_identity - Replace `StaleNode` string matching with structural `Error::StaleNode(_)` match in contested names queries - Add `is_unique_constraint_violation()` helper for SQLite error matching in import_mnemonic_screen - Fix wrong error variant: `DocumentNotFound` → `DataContractNotFound` in update_token_config - Add TODO for `BroadcastError(String)` pending `ScreenLike::display_message` trait redesign Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: address remaining PR #739 review comments — lock poisoning, error interpolation, StaleNode matching - Remove all remaining explicit `map_err` to `LockPoisoned` across 10 files, using `?` with blanket `From<PoisonError<T>>` impl - Fix `context/mod.rs` sites returning `Result<_, String>` to use `TaskError::from(e).to_string()` instead of discarding resource info - Add `StaleNode` structural matching to `query_dpns_vote_contenders.rs` for consistency with `query_dpns_contested_resources.rs` - Interpolate `operation` and `allowed_networks` fields in `OperationNotAvailableOnNetwork` error message - Narrow `is_unique_constraint_violation` to check `extended_code: 2067` (SQLITE_CONSTRAINT_UNIQUE) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: address final PR #739 review comments — jargon removal, dual signal fix - Replace protocol jargon ("asset lock", "instant lock proof", "chain lock proof") with user-friendly language in three TaskError Display messages per CLAUDE.md error guidelines - Fix dual success+error signal in register_contract.rs: return Ok(ContractSavedAfterProofError) instead of sending success via channel then returning Err(ProofError) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: add 6 new TaskError variants for LegacyError removal Add PlatformInfoFetchError, EncryptionError, WalletDatabasePersistError, AvatarProcessingError, MasterKeyNotFound, and TokenQueryError variants to prepare for removing From<String> and LegacyError. Also fix test assertion for updated jargon-free error message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: migrate system/grovestark/mnlist/platform_info to TaskError Replace Result<_, String> with Result<_, TaskError> in four domain dispatcher modules, eliminating reliance on From<String> for TaskError in these paths. - system_task: use ? on rusqlite errors (via #[from]) - grovestark: use ? on GroveSTARKError (via #[from]) - mnlist: replace .unwrap() on RwLock with proper error handling - platform_info: wrap SDK errors in TaskError::SdkError, use TaskError::Generic for document property extraction errors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: remove From<String> for TaskError and LegacyError variant Remove the `impl From<String> for TaskError` bridge and `TaskError::LegacyError(String)` variant. All 45 files that relied on string-to-TaskError conversion now use typed variants. Migration patterns applied: - RwLock/Mutex `.map_err(|e| e.to_string())?` → bare `?` (From<PoisonError>) - rusqlite `.map_err(|e| e.to_string())?` → bare `?` (Database #[from]) - dashcore_rpc errors → bare `?` (From<dashcore_rpc::Error>) - SDK errors → bare `?` (From<SdkError>) - String literals → typed variants (UserInput, WalletNotFound, etc.) - Model/SPV boundary → .map_err(TaskError::UserInput) at callsite New variants used: PlatformInfoFetchError, EncryptionError, WalletDatabasePersistError, AvatarProcessingError, MasterKeyNotFound, TokenQueryError. 45 files changed, 859 insertions(+), 1000 deletions(-) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: address 16 review findings across error handling and conventions Error design (error.rs): - Remove dead AvatarProcessingError variant (CODE-004) - Add ContractSchemaMismatch variant for contested index (CODE-006) - Add WithdrawalDocumentParsingError variant for withdrawal parsing (CODE-007) - Route 9 platform_info.rs callsites through From<SdkError> classification (CODE-001) Correctness fixes: - Fix dual signal in update_data_contract: return Ok instead of send+Err (CODE-002) - Return Ok(None) instead of Err(QueryReturnedNoRows) in contract_token_db (CODE-003) P2P module: - Public API returns P2PError instead of String, enabling #[from] conversion (CODE-008) - Add ProtocolError variant for protocol violations instead of synthetic io::Error (CODE-009) Error message quality: - Rewrite ~20 DashPayError Display messages for Everyday User audience (PROJ-001) - Rewrite ConfigError::NoValidConfigs and WalletError::AddressError Display (PROJ-004) - Update CLAUDE.md rule 7: remove stale Generic references (PROJ-002) - Clarify i18n placeholder style in CLAUDE.md (PROJ-005) Mechanical improvements: - Replace explicit TaskError::Database{source:e} with ? via #[from] (CODE-010) - Replace ok_or_else with ok_or for unit-like TaskError variants (CODE-011) - Replace .unwrap() with .unwrap_or_else(|e| e.into_inner()) in identity_db (CODE-017) - Simplify lock poisoning double-conversion in context/mod.rs (CODE-018) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: replace `UserInput(String)` with typed `TaskError` variants (#751) * Initial plan * refactor: replace all UserInput usages with typed TaskError variants Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: improve error message clarity (reviewer feedback) Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: merge RPC/context variants, split NotTokenDistributionRecipient, use #[source] for typed errors Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: address review feedback on TaskError migration — jargon, transparent errors, variant cleanup (#753) * Initial plan * refactor: address all review feedback — transparent errors, IdentitySaveError, DpnsFetchError, CLAUDE.md Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * fix: resolve CI formatting failures (#754) * Initial plan * fix: run cargo +nightly fmt --all to fix CI formatting failures Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * fix: address 3 PR review findings — error chain, transitional note, vote errors - DashPayError::SdkError: preserve error chain with #[source] Box<dash_sdk::Error> instead of stringifying to reason: String - CLAUDE.md rule 7: add transitional note acknowledging detail: String pattern - vote_on_dpns_name: route SDK errors through TaskError for user-friendly messages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address review round 4 — typed errors, dead code, lock safety - vote_on_dpns_name: use ContractSchemaMismatch instead of fake SdkError::Generic - core/mod.rs: migrate 7 payment functions from Result<_, String> to Result<_, TaskError> - load_identity_from_wallet: replace .unwrap() with ? on wallet RwLock operations - Remove dead UiError type (src/ui/error.rs) — zero usages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: strengthen error variant policy — never use String fields for user messages CLAUDE.md rule 7: replace transitional note with strict rule — error variants must not contain String fields that hold user-facing messages. Use #[source] with typed errors, or fieldless variants. Replace existing String-field variants when encountered. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>

Initial plan

7bded54

Copilot AI assigned Copilot and lklimek Mar 14, 2026

Copilot AI mentioned this pull request Mar 14, 2026

refactor: migrate from Result<T, String> to typed TaskError (#660) #739

Merged

5 tasks

Copilot started work on behalf of lklimek March 14, 2026 11:11 View session

refactor: address all review feedback — transparent errors, IdentityS…

c6eaa77

…aveError, DpnsFetchError, CLAUDE.md Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] [WIP] Address feedback on error handling and messaging in TaskError migration~~ refactor: address review feedback on TaskError migration — jargon, transparent errors, variant cleanup Mar 14, 2026

Copilot AI requested a review from lklimek March 14, 2026 11:31

Copilot finished work on behalf of lklimek March 14, 2026 11:31

lklimek marked this pull request as ready for review March 14, 2026 13:22

lklimek merged commit b27c1c5 into task/660-i18n-ready-strings-general-rule Mar 14, 2026
1 of 2 checks passed

lklimek deleted the copilot/sub-pr-739 branch March 14, 2026 13:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: address review feedback on TaskError migration — jargon, transparent errors, variant cleanup#753

refactor: address review feedback on TaskError migration — jargon, transparent errors, variant cleanup#753
lklimek merged 2 commits into
task/660-i18n-ready-strings-general-rulefrom
copilot/sub-pr-739

Copilot AI commented Mar 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Copilot AI commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

src/backend_task/error.rs

src/backend_task/contested_names/query_dpns_contested_resources.rs

DashPay files (profile.rs, contact_requests.rs, contact_info.rs, contacts.rs, auto_accept_handler.rs)

CLAUDE.md

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Mar 14, 2026 •

edited

Loading

`src/backend_task/error.rs`

`src/backend_task/contested_names/query_dpns_contested_resources.rs`

DashPay files (`profile.rs`, `contact_requests.rs`, `contact_info.rs`, `contacts.rs`, `auto_accept_handler.rs`)

`CLAUDE.md`