Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7f4291d
docs: promote i18n-ready strings to general coding rule (#660)
lklimek Mar 12, 2026
b934df5
refactor: add typed error variants for migration (#660)
lklimek Mar 12, 2026
aab8178
refactor: replace string errors with typed errors in spv/config/infra…
lklimek Mar 12, 2026
73704b7
refactor: replace string errors with typed TaskError in model/databas…
lklimek Mar 12, 2026
1e98cef
refactor: replace string errors with typed TaskError in backend_task/…
lklimek Mar 12, 2026
21bd380
refactor: replace string errors with typed TaskError in ui/ (#660)
lklimek Mar 12, 2026
5e5ea87
fix: remove useless TaskError::from conversions in wallet task dispat…
lklimek Mar 12, 2026
6184482
Merge branch 'v1.0-dev' into task/660-i18n-ready-strings-general-rule
lklimek Mar 12, 2026
d466668
Merge branch 'v1.0-dev' into task/660-i18n-ready-strings-general-rule
lklimek Mar 12, 2026
a5c577d
refactor: fix all unresolved review comments — proof errors, asset lo…
Copilot Mar 12, 2026
d44ade4
refactor: remove TaskError::Generic and TaskError::Other, replace wit…
Copilot Mar 13, 2026
0b635e5
refactor: address PR #739 review comments — structural error matching…
lklimek Mar 13, 2026
bd55bee
refactor: address remaining PR #739 review comments — lock poisoning,…
lklimek Mar 13, 2026
1d62eb7
Merge branch 'v1.0-dev' into task/660-i18n-ready-strings-general-rule
lklimek Mar 13, 2026
22b0a45
refactor: address final PR #739 review comments — jargon removal, dua…
lklimek Mar 13, 2026
cb9af82
refactor: add 6 new TaskError variants for LegacyError removal
lklimek Mar 13, 2026
4f61876
refactor: migrate system/grovestark/mnlist/platform_info to TaskError
lklimek Mar 13, 2026
751117c
refactor: remove From<String> for TaskError and LegacyError variant
lklimek Mar 13, 2026
2f58f08
refactor: address 16 review findings across error handling and conven…
lklimek Mar 13, 2026
9c9ea5a
Merge branch 'v1.0-dev' into task/660-i18n-ready-strings-general-rule
lklimek Mar 13, 2026
ab31046
refactor: replace `UserInput(String)` with typed `TaskError` variants…
Copilot Mar 14, 2026
b27c1c5
refactor: address review feedback on TaskError migration — jargon, tr…
Copilot Mar 14, 2026
de44eeb
fix: resolve CI formatting failures (#754)
Copilot Mar 14, 2026
bf40da6
fix: address 3 PR review findings — error chain, transitional note, v…
lklimek Mar 15, 2026
b5017b8
fix: address review round 4 — typed errors, dead code, lock safety
lklimek Mar 15, 2026
1ef019d
docs: strengthen error variant policy — never use String fields for u…
lklimek Mar 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions CLAUDE.md
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ scripts/safe-cargo.sh +nightly fmt --all

* When a method takes `&AppContext` (or `Option<&AppContext>`), place it as the first parameter after `self`.
* Screen constructors handle errors internally via `MessageBanner` and return `Self` with degraded state. Keep `create_screen()` clean — no error handling at callsites.
* **i18n-ready strings**: All user-facing strings (labels, messages, tooltips, errors) must be simple, complete sentences. Avoid concatenating fragments, positional assumptions, or grammar that breaks in other languages. Each string should be extractable as a single translation unit with named placeholders for dynamic values (`{ seconds }`, `{ name }`) and no logic in the text itself.

### Error messages

Expand All @@ -69,11 +70,10 @@ User-facing error messages (shown in `MessageBanner` via `Display`) must follow
1. **Audience**: Write for the Everyday User persona (`docs/personas/everyday-user.md`). No jargon — no "consensus error", "nonce", "state transition", "SDK", "RPC", or error codes.
2. **Structure**: *What happened* + *what to do*. Every message must include a concrete action the user can take themselves: retry, wait, try a different approach. Never redirect to "contact support" — users must be able to self-resolve.
3. **Tone**: Calm, direct, brief. Not apologetic ("Sorry!"), not alarming ("Something went wrong!"), not vague ("An error occurred").
4. **Technical details**: Never in the message itself — no raw error strings, stack traces, SDK internals, or error codes. Attach via `BannerHandle::with_details(e)` — the `Debug` repr goes to the collapsible details panel and logs. Never refer users to "details" or "details panel" — these are not visible in basic mode. Exception: Base58 identifiers (see rule 7) are not technical details — they are user-meaningful handles.
5. **i18n-ready**: Write messages as simple, complete sentences without interpolation tricks. Avoid concatenating fragments, positional assumptions, or grammar that breaks in other languages. Messages should be straightforward to extract into [Fluent](https://projectfluent.org/) `.ftl` files later — one message ID per string, placeholders only for dynamic values (`{ $seconds }`, `{ $name }`), no logic in the text itself.
6. **Reference implementation**: `sdk_error_user_message()` in `src/backend_task/error.rs` demonstrates the pattern for SDK errors. New `TaskError` variants should follow the same style.
7. **Base58 IDs are allowed in messages**: Contract IDs, identity IDs, document IDs, and similar Base58-encoded identifiers may appear in user-facing messages when they help the user identify which object is involved (e.g., *"This key conflicts with an existing key bound to contract `Abc123…`."*). They are not jargon — they are opaque-but-copyable handles the user can look up.
8. **Prefer granular `TaskError` variants over `Generic`**: When mapping errors to add context, add a dedicated `TaskError` variant with a `#[source]` field rather than converting to `TaskError::Generic(format!(...))`. Granular variants preserve the error chain, enable structural matching by callers, and make `Display` / `Debug` separation explicit. `TaskError::Generic` is a last resort for one-off strings with no upstream error to preserve. For `#[source]` fields in SDK-originated error variants, use `Box<SdkError>` — convert upstream types (e.g. `ProtocolError`) via `SdkError::Protocol(e)`. Use the concrete domain type directly for non-SDK errors (e.g. `rusqlite::Error`). Omit `#[source]` entirely when the upstream error carries no useful diagnostic information (e.g. a channel `SendError`).
4. **Technical details**: Never in the message itself — no raw error strings, stack traces, SDK internals, or error codes. Attach via `BannerHandle::with_details(e)` — the `Debug` repr goes to the collapsible details panel and logs. Never refer users to "details" or "details panel" — these are not visible in basic mode. Exception: Base58 identifiers (see rule 6) are not technical details — they are user-meaningful handles.
5. **Reference implementation**: `sdk_error_user_message()` in `src/backend_task/error.rs` demonstrates the pattern for SDK errors. New `TaskError` variants should follow the same style.
6. **Base58 IDs are allowed in messages**: Contract IDs, identity IDs, document IDs, and similar Base58-encoded identifiers may appear in user-facing messages when they help the user identify which object is involved (e.g., *"This key conflicts with an existing key bound to contract `Abc123…`."*). They are not jargon — they are opaque-but-copyable handles the user can look up.
7. **Prefer granular `TaskError` variants over `Generic`**: When mapping errors to add context, add a dedicated `TaskError` variant with a `#[source]` field rather than converting to `TaskError::Generic(format!(...))`. Granular variants preserve the error chain, enable structural matching by callers, and make `Display` / `Debug` separation explicit. `TaskError::Generic` is a last resort for one-off strings with no upstream error to preserve. For `#[source]` fields in SDK-originated error variants, use `Box<SdkError>` — convert upstream types (e.g. `ProtocolError`) via `SdkError::Protocol(e)`. Use the concrete domain type directly for non-SDK errors (e.g. `rusqlite::Error`). Omit `#[source]` entirely when the upstream error carries no useful diagnostic information (e.g. a channel `SendError`).

## Architecture Overview

Expand Down
12 changes: 7 additions & 5 deletions src/backend_task/broadcast_state_transition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use dash_sdk::{
platform::transition::broadcast::BroadcastStateTransition,
};

use crate::backend_task::error::TaskError;
use crate::context::AppContext;

use super::BackendTaskSuccessResult;
Expand All @@ -12,10 +13,11 @@ impl AppContext {
&self,
state_transition: StateTransition,
sdk: &Sdk,
) -> Result<BackendTaskSuccessResult, String> {
match state_transition.broadcast(sdk, None).await {
Ok(_) => Ok(BackendTaskSuccessResult::BroadcastedStateTransition),
Err(e) => Err(format!("Error broadcasting state transition: {}", e)),
}
) -> Result<BackendTaskSuccessResult, TaskError> {
state_transition
.broadcast(sdk, None)
.await
.map(|_| BackendTaskSuccessResult::BroadcastedStateTransition)
.map_err(TaskError::from)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ impl AppContext {
return Err(format!("Contested resource query failed: {}", e));
}
}
if e.to_string().contains("try another server")
// TODO: Replace the "contract not found" string match with a
// structural SDK variant when one is available.
if matches!(e, dash_sdk::Error::StaleNode(_))
|| e.to_string().contains(
"contract not found when querying from value with contract info",
)
Comment thread
lklimek marked this conversation as resolved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,10 @@ impl AppContext {
})
.map_err(|e| e.to_string())?
}
let error_str = e.to_string();
if error_str.contains("try another server")
|| error_str.contains(
// TODO: Replace the "contract not found" string match with a
// structural SDK variant when one is available.
if matches!(e, dash_sdk::Error::StaleNode(_))
|| e.to_string().contains(
"contract not found when querying from value with contract info",
Comment thread
lklimek marked this conversation as resolved.
)
{
Expand Down
4 changes: 3 additions & 1 deletion src/backend_task/contested_names/query_ending_times.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ impl AppContext {
})
.map_err(|e| e.to_string())?
}
if e.to_string().contains("try another server")
// TODO: Replace the "contract not found" string match with a
// structural SDK variant when one is available.
if matches!(e, dash_sdk::Error::StaleNode(_))
|| e.to_string().contains(
"contract not found when querying from value with contract info",
)
Expand Down
42 changes: 18 additions & 24 deletions src/backend_task/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,32 +44,28 @@ impl AppContext {
task: ContractTask,
sdk: &Sdk,
sender: crate::utils::egui_mpsc::SenderAsync<TaskResult>,
) -> Result<BackendTaskSuccessResult, String> {
) -> Result<BackendTaskSuccessResult, crate::backend_task::error::TaskError> {
match task {
ContractTask::FetchContracts(identifiers) => {
match DataContract::fetch_many(sdk, identifiers).await {
Ok(data_contracts) => {
let mut results = vec![];
for data_contract in data_contracts {
if let Some(contract) = &data_contract.1 {
self.db
.insert_contract_if_not_exists(
contract,
None,
NoTokensShouldBeAdded,
self,
)
.map_err(|e| {
format!("Error inserting contract into the database: {}", e)
})?;
self.db.insert_contract_if_not_exists(
contract,
None,
NoTokensShouldBeAdded,
self,
)?;
results.push(Some(contract.clone()));
} else {
results.push(None);
}
}
Ok(BackendTaskSuccessResult::FetchedContracts(results))
}
Err(e) => Err(format!("Error fetching contracts: {}", e)),
Err(e) => Err(crate::backend_task::error::TaskError::from(e)),
}
}
ContractTask::FetchContractsWithDescriptions(identifiers) => {
Expand All @@ -96,7 +92,7 @@ impl AppContext {
};
let document_option = Document::fetch(sdk, document_query)
.await
.map_err(|e| format!("Error fetching description: {}", e))?;
.map_err(crate::backend_task::error::TaskError::from)?;

let mut token_infos = vec![];
for token in contract.tokens() {
Expand Down Expand Up @@ -152,7 +148,7 @@ impl AppContext {
}
Ok(BackendTaskSuccessResult::ContractsWithDescriptions(results))
}
Err(e) => Err(format!("Error fetching contracts: {}", e)),
Err(e) => Err(crate::backend_task::error::TaskError::from(e)),
}
}
ContractTask::FetchActiveGroupActions(contract, identity) => {
Expand All @@ -176,7 +172,7 @@ impl AppContext {

let group_actions = GroupAction::fetch_many(sdk, query)
.await
.map_err(|e| format!("Error fetching group actions: {}", e))?;
.map_err(crate::backend_task::error::TaskError::from)?;

for group_action in group_actions {
if let Some(action) = &group_action.1 {
Expand Down Expand Up @@ -213,16 +209,14 @@ impl AppContext {
ContractTask::RemoveContract(identifier) => self
.remove_contract(&identifier)
.map(|_| BackendTaskSuccessResult::RemovedContract)
.map_err(|e| format!("Error removing contract: {}", e)),
.map_err(crate::backend_task::error::TaskError::from),
ContractTask::SaveDataContract(data_contract, alias, insert_tokens_too) => {
self.db
.insert_contract_if_not_exists(
&data_contract,
alias.as_deref(),
insert_tokens_too,
self,
)
.map_err(|e| format!("Error inserting contract into the database: {}", e))?;
self.db.insert_contract_if_not_exists(
&data_contract,
alias.as_deref(),
insert_tokens_too,
self,
)?;
Ok(BackendTaskSuccessResult::SavedContract)
}
}
Expand Down
22 changes: 9 additions & 13 deletions src/backend_task/core/mod.rs
Comment thread
lklimek marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[CODE-003] MEDIUM — get_best_chain_lock (line 419) still returns Result<ChainLock, String> with jargon error messages

This method wasn't touched in this PR, but it's a notable gap in the migration: it returns Result<ChainLock, String> with developer-facing format! messages containing "core cookie path" (line 430) and "Failed to get best chain lock" (line 453) — jargon that can bubble up to users through the detail: String path in TaskError variants.

Suggestion: Convert to return Result<ChainLock, TaskError> with appropriate variants. Suitable for a follow-up PR.

🤖 Claudius the Magnificent · Review round 5

Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl AppContext {
}
CoreTask::RefreshSingleKeyWalletInfo(wallet) => {
let (key_hash, address) = {
let g = wallet.read().map_err(|e| TaskError::from(e.to_string()))?;
let g = wallet.read().map_err(TaskError::from)?;
(g.key_hash, g.address.clone())
};
let wallet_for_retry = wallet.clone();
Expand All @@ -275,7 +275,7 @@ impl AppContext {
}
CoreTask::StartDashQT(network, custom_dash_qt, overwrite_dash_conf) => self
.start_dash_qt(network, custom_dash_qt, overwrite_dash_conf)
.map_err(|e| TaskError::from(e.to_string()))
.map_err(|e| TaskError::DashCoreStartError { source: e })
.map(|_| BackendTaskSuccessResult::None),
CoreTask::CreateRegistrationAssetLock(wallet, amount, identity_index) => {
let (seed_hash, first_addr) = Self::core_wallet_first_address(&wallet)?;
Expand Down Expand Up @@ -303,7 +303,7 @@ impl AppContext {
}
CoreTask::SendSingleKeyWalletPayment { wallet, request } => {
let (key_hash, address) = {
let g = wallet.read().map_err(|e| TaskError::from(e.to_string()))?;
let g = wallet.read().map_err(TaskError::from)?;
(g.key_hash, g.address.clone())
};
let result = self
Expand All @@ -325,17 +325,16 @@ impl AppContext {
wallet,
} => {
if !matches!(self.network, Network::Regtest | Network::Devnet) {
return Err(TaskError::from(
"Mining is only available on Regtest and Devnet".to_string(),
));
return Err(TaskError::OperationNotAvailableOnNetwork {
operation: "Mining",
allowed_networks: "Regtest and Devnet",
});
}
let ctx = self.clone();
let mined = tokio::task::spawn_blocking(move || {
ctx.core_client
.read()
.map_err(|e| {
TaskError::from(format!("Core client lock was poisoned: {}", e))
})?
.map_err(TaskError::from)?
.generate_to_address(block_count, &address)
.map_err(TaskError::from)
})
Expand All @@ -346,10 +345,7 @@ impl AppContext {
// Refresh wallet balances via RPC so the UI reflects the new coins
let refresh_ctx = self.clone();
tokio::task::spawn_blocking(move || refresh_ctx.refresh_wallet_info(wallet))
.await?
.map_err(|e| {
TaskError::from(format!("Error refreshing wallet after mining: {}", e))
})?;
.await??;

Ok(BackendTaskSuccessResult::MineBlocksSuccess(mined_count))
}
Expand Down
30 changes: 29 additions & 1 deletion src/backend_task/dashpay/errors.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use dash_sdk::dpp::dashcore::secp256k1;
use dash_sdk::platform::Identifier;
use thiserror::Error;

/// Comprehensive error types for DashPay operations
#[derive(Error, Debug, Clone, PartialEq)]
#[derive(Error, Debug)]
pub enum DashPayError {
// Contact Request Errors
#[error("Identity not found: {identity_id}")]
Expand Down Expand Up @@ -121,6 +122,33 @@ pub enum DashPayError {

#[error("Rate limit exceeded for operation: {operation}")]
RateLimited { operation: String },

/// Failed to build a document query (schema / configuration error).
#[error("Could not prepare the data request. Please retry or update the application.")]
QueryCreation {
/// Description of what query was being built (e.g., "contact requests", "DPNS domain").
query_target: &'static str,
#[source]
source: Box<dash_sdk::Error>,
},

/// Failed to parse a cryptographic key (secp256k1).
#[error("Could not read a cryptographic key. The data may be corrupted.")]
CryptoKeyParsing {
#[from]
source: secp256k1::Error,
},

/// Failed to resolve a private key from the identity's key store.
#[error(
"Could not find the required private key in your wallet. Try refreshing your identities."
)]
PrivateKeyResolution {
/// Human-readable key purpose (e.g. "ENCRYPTION", "AUTHENTICATION").
key_purpose: String,
/// Details about why the lookup failed.
reason: String,
},
}

impl DashPayError {
Expand Down
Loading
Loading