Skip to content

Commit 5217c9f

Browse files
Copilotlklimek
andcommitted
refactor: store Box<SdkError> as typed source in TaskError variants instead of debug string
Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>
1 parent 8210e4f commit 5217c9f

2 files changed

Lines changed: 54 additions & 31 deletions

File tree

src/backend_task/error.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//! Parses known error patterns into typed variants automatically.
77
88
use dash_sdk::dashcore_rpc;
9+
use dash_sdk::Error as SdkError;
910
use thiserror::Error;
1011

1112
/// Dash Core RPC error code: wallet file not specified (multi-wallet node).
@@ -64,15 +65,17 @@ pub enum TaskError {
6465
/// Duplicate identity public key — the key data already exists on the platform.
6566
#[error("This public key is already registered on the platform. Try a different key.")]
6667
DuplicateIdentityPublicKey {
67-
/// Raw `SdkError` debug representation for diagnostic logging.
68-
source_error: String,
68+
/// The original SDK error returned by the broadcast API.
69+
#[source]
70+
source_error: Box<SdkError>,
6971
},
7072

7173
/// Duplicate identity public key ID — the key hash is already taken platform-wide.
7274
#[error("This key hash is already registered on the platform. Try a different key.")]
7375
DuplicateIdentityPublicKeyId {
74-
/// Raw `SdkError` debug representation for diagnostic logging.
75-
source_error: String,
76+
/// The original SDK error returned by the broadcast API.
77+
#[source]
78+
source_error: Box<SdkError>,
7679
},
7780

7881
/// Identity public key conflicts with an existing key's unique contract bounds.
@@ -81,8 +84,9 @@ pub enum TaskError {
8184
)]
8285
IdentityPublicKeyContractBoundsConflict {
8386
contract_id: String,
84-
/// Raw `SdkError` debug representation for diagnostic logging.
85-
source_error: String,
87+
/// The original SDK error returned by the broadcast API.
88+
#[source]
89+
source_error: Box<SdkError>,
8690
},
8791
}
8892

src/backend_task/identity/add_key_to_identity.rs

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,36 +28,55 @@ use dash_sdk::platform::{Fetch, Identity};
2828
/// Matches known consensus error variants from two SDK error paths
2929
/// (`StateTransitionBroadcastError` and `Protocol/ConsensusError`),
3030
/// falling back to `TaskError::Generic` for unrecognised errors.
31-
fn broadcast_error(error: &SdkError) -> TaskError {
32-
let source_error = format!("{:?}", error);
31+
/// The original `SdkError` is preserved as a typed field on each variant.
32+
fn broadcast_error(error: SdkError) -> TaskError {
33+
// Classify the error while borrowing; the borrow is dropped before `error`
34+
// is moved into the returned variant.
35+
enum ConsensusKind {
36+
DuplicateKey,
37+
DuplicateKeyId,
38+
ContractBoundsConflict(String),
39+
}
3340

34-
let consensus_error = match error {
35-
SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(),
36-
SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()),
37-
_ => None,
38-
};
41+
let kind: Option<ConsensusKind> = {
42+
let consensus_error = match &error {
43+
SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(),
44+
SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()),
45+
_ => None,
46+
};
3947

40-
if let Some(ce) = consensus_error {
41-
match ce {
48+
consensus_error.and_then(|ce| match ce {
4249
ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyStateError(_)) => {
43-
return TaskError::DuplicateIdentityPublicKey { source_error };
50+
Some(ConsensusKind::DuplicateKey)
4451
}
4552
ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyIdStateError(_)) => {
46-
return TaskError::DuplicateIdentityPublicKeyId { source_error };
53+
Some(ConsensusKind::DuplicateKeyId)
4754
}
4855
ConsensusError::StateError(
4956
StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e),
50-
) => {
51-
return TaskError::IdentityPublicKeyContractBoundsConflict {
52-
contract_id: e.contract_id().to_string(Encoding::Base58),
53-
source_error,
54-
};
57+
) => Some(ConsensusKind::ContractBoundsConflict(
58+
e.contract_id().to_string(Encoding::Base58),
59+
)),
60+
_ => None,
61+
})
62+
};
63+
64+
let boxed = Box::new(error);
65+
match kind {
66+
Some(ConsensusKind::DuplicateKey) => {
67+
TaskError::DuplicateIdentityPublicKey { source_error: boxed }
68+
}
69+
Some(ConsensusKind::DuplicateKeyId) => {
70+
TaskError::DuplicateIdentityPublicKeyId { source_error: boxed }
71+
}
72+
Some(ConsensusKind::ContractBoundsConflict(contract_id)) => {
73+
TaskError::IdentityPublicKeyContractBoundsConflict {
74+
contract_id,
75+
source_error: boxed,
5576
}
56-
_ => {}
5777
}
78+
None => TaskError::Generic(format!("Broadcasting error: {boxed}")),
5879
}
59-
60-
TaskError::Generic(format!("Broadcasting error: {}", error))
6180
}
6281

6382
impl AppContext {
@@ -112,7 +131,7 @@ impl AppContext {
112131
let result = state_transition
113132
.broadcast_and_wait(sdk, None)
114133
.await
115-
.map_err(|ref e| broadcast_error(e))?;
134+
.map_err(broadcast_error)?;
116135

117136
// Log and handle the proof result
118137
tracing::info!("AddKeyToIdentity proof result: {}", result);
@@ -184,15 +203,15 @@ mod tests {
184203
let consensus =
185204
ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2]));
186205
let sdk_err = SdkError::from(consensus);
187-
let err = broadcast_error(&sdk_err);
206+
let err = broadcast_error(sdk_err);
188207
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. }));
189208
}
190209

191210
#[test]
192211
fn test_duplicate_public_key_id_error() {
193212
let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3]));
194213
let sdk_err = SdkError::from(consensus);
195-
let err = broadcast_error(&sdk_err);
214+
let err = broadcast_error(sdk_err);
196215
assert!(matches!(err, TaskError::DuplicateIdentityPublicKeyId { .. }));
197216
}
198217

@@ -210,7 +229,7 @@ mod tests {
210229
),
211230
);
212231
let sdk_err = SdkError::from(consensus);
213-
let err = broadcast_error(&sdk_err);
232+
let err = broadcast_error(sdk_err);
214233
let expected_contract_id = contract_id.to_string(Encoding::Base58);
215234
assert!(
216235
matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id, .. } if *contract_id == expected_contract_id)
@@ -228,14 +247,14 @@ mod tests {
228247
cause: Some(consensus),
229248
};
230249
let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err);
231-
let err = broadcast_error(&sdk_err);
250+
let err = broadcast_error(sdk_err);
232251
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. }));
233252
}
234253

235254
#[test]
236255
fn test_unknown_sdk_error_falls_back() {
237256
let sdk_err = SdkError::Generic("connection timeout".to_string());
238-
let err = broadcast_error(&sdk_err);
257+
let err = broadcast_error(sdk_err);
239258
assert!(matches!(err, TaskError::Generic(ref s) if s.contains("connection timeout")));
240259
}
241260
}

0 commit comments

Comments
 (0)