Skip to content

Commit 05e781a

Browse files
Copilotlklimek
andauthored
refactor: preserve typed SdkError in TaskError variants for duplicate-key errors (#731)
* Initial plan * fix: log raw SdkError in broadcast_error to preserve technical context Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: embed raw SdkError as source_error field in typed TaskError variants Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: store Box<SdkError> as typed source in TaskError variants instead of debug string Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * style: fix rustfmt formatting issues in error.rs and add_key_to_identity.rs 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>
1 parent ec5e11d commit 05e781a

2 files changed

Lines changed: 69 additions & 30 deletions

File tree

src/backend_task/error.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//! `From<String>` → backwards compatible with existing `Result<T, String>` code.
66
//! Parses known error patterns into typed variants automatically.
77
8+
use dash_sdk::Error as SdkError;
89
use dash_sdk::dashcore_rpc;
910
use thiserror::Error;
1011

@@ -63,17 +64,30 @@ pub enum TaskError {
6364

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.")]
66-
DuplicateIdentityPublicKey,
67+
DuplicateIdentityPublicKey {
68+
/// The original SDK error returned by the broadcast API.
69+
#[source]
70+
source_error: Box<SdkError>,
71+
},
6772

6873
/// Duplicate identity public key ID — the key hash is already taken platform-wide.
6974
#[error("This key hash is already registered on the platform. Try a different key.")]
70-
DuplicateIdentityPublicKeyId,
75+
DuplicateIdentityPublicKeyId {
76+
/// The original SDK error returned by the broadcast API.
77+
#[source]
78+
source_error: Box<SdkError>,
79+
},
7180

7281
/// Identity public key conflicts with an existing key's unique contract bounds.
7382
#[error(
7483
"This key conflicts with an existing key bound to contract {contract_id}. Use a different key or purpose."
7584
)]
76-
IdentityPublicKeyContractBoundsConflict { contract_id: String },
85+
IdentityPublicKeyContractBoundsConflict {
86+
contract_id: String,
87+
/// The original SDK error returned by the broadcast API.
88+
#[source]
89+
source_error: Box<SdkError>,
90+
},
7791
}
7892

7993
impl From<String> for TaskError {

src/backend_task/identity/add_key_to_identity.rs

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,33 +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 consensus_error = match error {
33-
SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(),
34-
SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()),
35-
_ => None,
36-
};
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+
}
40+
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+
};
3747

38-
if let Some(ce) = consensus_error {
39-
match ce {
48+
consensus_error.and_then(|ce| match ce {
4049
ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyStateError(_)) => {
41-
return TaskError::DuplicateIdentityPublicKey;
50+
Some(ConsensusKind::DuplicateKey)
4251
}
4352
ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyIdStateError(_)) => {
44-
return TaskError::DuplicateIdentityPublicKeyId;
53+
Some(ConsensusKind::DuplicateKeyId)
4554
}
4655
ConsensusError::StateError(
4756
StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e),
48-
) => {
49-
return TaskError::IdentityPublicKeyContractBoundsConflict {
50-
contract_id: e.contract_id().to_string(Encoding::Base58),
51-
};
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) => TaskError::DuplicateIdentityPublicKey {
67+
source_error: boxed,
68+
},
69+
Some(ConsensusKind::DuplicateKeyId) => TaskError::DuplicateIdentityPublicKeyId {
70+
source_error: boxed,
71+
},
72+
Some(ConsensusKind::ContractBoundsConflict(contract_id)) => {
73+
TaskError::IdentityPublicKeyContractBoundsConflict {
74+
contract_id,
75+
source_error: boxed,
5276
}
53-
_ => {}
5477
}
78+
None => TaskError::Generic(format!("Broadcasting error: {boxed}")),
5579
}
56-
57-
TaskError::Generic(format!("Broadcasting error: {}", error))
5880
}
5981

6082
impl AppContext {
@@ -109,7 +131,7 @@ impl AppContext {
109131
let result = state_transition
110132
.broadcast_and_wait(sdk, None)
111133
.await
112-
.map_err(|ref e| broadcast_error(e))?;
134+
.map_err(broadcast_error)?;
113135

114136
// Log and handle the proof result
115137
tracing::info!("AddKeyToIdentity proof result: {}", result);
@@ -181,16 +203,19 @@ mod tests {
181203
let consensus =
182204
ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2]));
183205
let sdk_err = SdkError::from(consensus);
184-
let err = broadcast_error(&sdk_err);
185-
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey));
206+
let err = broadcast_error(sdk_err);
207+
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. }));
186208
}
187209

188210
#[test]
189211
fn test_duplicate_public_key_id_error() {
190212
let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3]));
191213
let sdk_err = SdkError::from(consensus);
192-
let err = broadcast_error(&sdk_err);
193-
assert!(matches!(err, TaskError::DuplicateIdentityPublicKeyId));
214+
let err = broadcast_error(sdk_err);
215+
assert!(matches!(
216+
err,
217+
TaskError::DuplicateIdentityPublicKeyId { .. }
218+
));
194219
}
195220

196221
#[test]
@@ -207,10 +232,10 @@ mod tests {
207232
),
208233
);
209234
let sdk_err = SdkError::from(consensus);
210-
let err = broadcast_error(&sdk_err);
235+
let err = broadcast_error(sdk_err);
211236
let expected_contract_id = contract_id.to_string(Encoding::Base58);
212237
assert!(
213-
matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id } if *contract_id == expected_contract_id)
238+
matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id, .. } if *contract_id == expected_contract_id)
214239
);
215240
}
216241

@@ -225,14 +250,14 @@ mod tests {
225250
cause: Some(consensus),
226251
};
227252
let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err);
228-
let err = broadcast_error(&sdk_err);
229-
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey));
253+
let err = broadcast_error(sdk_err);
254+
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. }));
230255
}
231256

232257
#[test]
233258
fn test_unknown_sdk_error_falls_back() {
234259
let sdk_err = SdkError::Generic("connection timeout".to_string());
235-
let err = broadcast_error(&sdk_err);
260+
let err = broadcast_error(sdk_err);
236261
assert!(matches!(err, TaskError::Generic(ref s) if s.contains("connection timeout")));
237262
}
238263
}

0 commit comments

Comments
 (0)