fix(dapi): broadcast errors displayed as consensus error #2841
Conversation
WalkthroughTwo changes improve error handling: a new test case validates that publishing a data contract with non-contiguous document property positions produces the expected error, and error message derivation in the platform service now attempts to extract consensus error details when the primary message is unavailable. Changes
Sequence Diagram(s)sequenceDiagram
participant Code as Platform Service
participant Status as TenderdashStatus
participant Mapping as Error Mapping
Code->>Mapping: Convert TenderdashStatus to<br/>StateTransitionBroadcastError
Mapping->>Status: Check err.message
alt message present
Mapping->>Mapping: Use err.message
else message absent
Mapping->>Status: Attempt deserialize<br/>err.consensus_error
alt deserialization succeeds
Mapping->>Mapping: Use ConsensusError<br/>string form
else deserialization fails
Mapping->>Mapping: Use "Unknown error"
end
end
Mapping->>Code: Return StateTransitionBroadcastError<br/>with resolved message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "be96ad7836570ad60a0a65dd339955431fc3a11dec61a81a97285097d2c9a564"
)Xcode manual integration:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-dapi/src/services/platform_service/error_mapping.rs (1)
142-156: Approve the fix, but suggest extracting duplicated logic.The error message derivation logic correctly implements the PR objective of extracting consensus error messages. However, this logic is duplicated between this
Fromimplementation and thegrpc_message()method (lines 81-96).Consider extracting the message derivation logic into a helper method on
TenderdashStatus:impl TenderdashStatus { + /// Derive the error message from available sources. + fn derive_message(&self) -> String { + if let Some(message) = &self.message { + return message.clone(); + } + + if let Some(consensus_error_bytes) = &self.consensus_error + && let Ok(consensus_error) = + ConsensusError::deserialize_from_bytes(consensus_error_bytes).inspect_err(|e| { + tracing::debug!("Failed to deserialize consensus error: {}", e); + }) + { + return consensus_error.to_string(); + } + + format!("Unknown error with code {}", self.code) + } + /// Derive an end-user message, preferring explicit message over consensus error details. fn grpc_message(&self) -> String { - if let Some(message) = &self.message { - return message.clone(); - } - - if let Some(consensus_error_bytes) = &self.consensus_error - && let Ok(consensus_error) = - ConsensusError::deserialize_from_bytes(consensus_error_bytes).inspect_err(|e| { - tracing::debug!("Failed to deserialize consensus error: {}", e); - }) - { - return consensus_error.to_string(); - } - - format!("Unknown error with code {}", self.code) + self.derive_message() }Then update the
Fromimplementation:impl From<TenderdashStatus> for StateTransitionBroadcastError { fn from(err: TenderdashStatus) -> Self { - let message = if let Some(msg) = err.message { - msg - } else { - // try to extract from consensus error - if let Some(consensus_error_bytes) = &err.consensus_error - && let Ok(consensus_error) = - ConsensusError::deserialize_from_bytes(consensus_error_bytes).inspect_err(|e| { - tracing::debug!("Failed to deserialize consensus error: {}", e); - }) - { - consensus_error.to_string() - } else { - "Unknown error".to_string() - } - }; + let message = err.derive_message(); StateTransitionBroadcastError { code: err.code.clamp(0, u32::MAX as i64) as u32, message, data: err.consensus_error.clone().unwrap_or_default(), } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/platform-test-suite/test/functional/platform/DataContract.spec.js(1 hunks)packages/rs-dapi/src/services/platform_service/error_mapping.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
packages/platform-test-suite/**
📄 CodeRabbit inference engine (AGENTS.md)
packages/platform-test-suite/**: Keep end-to-end tests and helpers in packages/platform-test-suite
Keep all E2E tests exclusively in packages/platform-test-suite
Files:
packages/platform-test-suite/test/functional/platform/DataContract.spec.js
packages/**/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/**/*.{js,ts,jsx,tsx}: Adhere to ESLint with Airbnb/TypeScript configs for JS/TS code
Use camelCase for JS/TS variables and functions
Use PascalCase for JS/TS classes
Prefer kebab-case filenames within JS packages
Files:
packages/platform-test-suite/test/functional/platform/DataContract.spec.js
🧠 Learnings (6)
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
📚 Learning: 2024-11-20T10:01:50.837Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs:94-94
Timestamp: 2024-11-20T10:01:50.837Z
Learning: In `packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs`, when converting with `PublicKey::try_from`, it's acceptable to use `.expect()` to handle potential conversion errors.
Applied to files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
PR: dashpay/platform#2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
PR: dashpay/platform#2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/rs-dapi/src/services/platform_service/error_mapping.rs
📚 Learning: 2025-06-18T03:44:14.385Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2673
File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs:1164-1197
Timestamp: 2025-06-18T03:44:14.385Z
Learning: QuantumExplorer determined that a CodeRabbit suggestion about fixing signable_bytes calculation in identity update tests with contract-bound keys was incorrect - the code flow is working as intended without the suggested changes.
Applied to files:
packages/platform-test-suite/test/functional/platform/DataContract.spec.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (1)
packages/platform-test-suite/test/functional/platform/DataContract.spec.js (1)
62-85: LGTM! Test validates the error message extraction fix.The test properly validates that consensus errors are now correctly exposed when a data contract has non-contiguous document property positions. The test structure follows established patterns and appropriately uses
skipValidationto bypass client-side validation and trigger server-side validation.
|
I would just target 2.2. |
lklimek
dismissed
QuantumExplorer’s stale review
The base branch was changed.
Issue being fixed or feature implemented
rs-dapi returns
Unknown errorwhen state transition failsWhat was done?
Extract error message from consensus error received from Tenderdash.
How Has This Been Tested?
local devnet
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit