Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
last quality sweep
Browse files Browse the repository at this point in the history
  • Loading branch information
drahnr committed Feb 25, 2022
1 parent 92a6379 commit 99e6900
Show file tree
Hide file tree
Showing 13 changed files with 42 additions and 56 deletions.
13 changes: 4 additions & 9 deletions node/core/dispute-coordinator/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ pub type JfyiResult<T> = std::result::Result<T, JfyiError>;
#[allow(missing_docs)]
#[fatality::fatality(splitable)]
pub enum Error {
/// Errors coming from runtime::Runtime.
#[fatal]
#[error("Error while accessing runtime information {0}")]
RuntimeApi(#[from] runtime::FatalError),

/// We received a legacy `SubystemError::Context` error which is considered fatal.
#[fatal]
#[error("SubsystemError::Context error: {0}")]
Expand Down Expand Up @@ -74,6 +69,10 @@ pub enum Error {
#[error("Chain API dropped response channel sender")]
ChainApiSenderDropped,

#[fatal(forward)]
#[error("Error while accessing runtime information {0}")]
Runtime(#[from] runtime::Error),

#[error(transparent)]
ChainApi(#[from] ChainApiError),

Expand All @@ -96,10 +95,6 @@ pub enum Error {
#[error("Sessions unavailable in `RollingSessionWindow`: {0}")]
RollingSessionWindow(#[from] SessionsUnavailable),

/// Errors coming from runtime::Runtime.
#[error("Error while accessing runtime information: {0}")]
Runtime(#[from] runtime::JfyiError),

#[error(transparent)]
QueueError(#[from] participation::QueueError),
}
Expand Down
10 changes: 1 addition & 9 deletions node/core/dispute-coordinator/src/real/ordering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use crate::{
error::{FatalError, FatalResult, Result},
LOG_TARGET,
};
use fatality::Split;

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -230,14 +229,7 @@ impl OrderingProvider {
for (block_num, block_hash) in block_numbers.zip(block_hashes) {
// Get included events:
let included = get_candidate_events(sender, block_hash)
.await
.map_err(|err| -> crate::error::Error {
match err.split() {
// will be remedied as soon as `split` is comatibe with `defer` in `fatality`.
Ok(jfyi) => crate::error::JfyiError::Runtime(jfyi).into(),
Err(fatal) => crate::error::FatalError::RuntimeApi(fatal).into(),
}
})?
.await?
.into_iter()
.filter_map(|ev| match ev {
CandidateEvent::CandidateIncluded(receipt, _, _, _) => Some(receipt),
Expand Down
7 changes: 3 additions & 4 deletions node/network/availability-distribution/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,9 @@ pub type Result<T> = std::result::Result<T, Error>;
/// We basically always want to try and continue on error. This utility function is meant to
/// consume top-level errors by simply logging them
pub fn log_error(result: Result<()>, ctx: &'static str) -> std::result::Result<(), FatalError> {
match result.into_nested() {
Err(fatal) => Err(fatal),
Ok(Ok(())) => Ok(()),
Ok(Err(jfyi)) => {
match result.into_nested()? {
Ok(()) => Ok(()),
Err(jfyi) => {
match jfyi {
JfyiError::UnexpectedPoV |
JfyiError::InvalidValidatorIndex |
Expand Down
12 changes: 6 additions & 6 deletions node/network/availability-distribution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use polkadot_subsystem::{

/// Error and [`Result`] type for this subsystem.
mod error;
use error::{log_error, Error, Result};
use error::{log_error, FatalError, Result};

use polkadot_node_subsystem_util::runtime::RuntimeInfo;

Expand Down Expand Up @@ -95,7 +95,7 @@ impl AvailabilityDistributionSubsystem {
}

/// Start processing work as passed on from the Overseer.
async fn run<Context>(self, mut ctx: Context) -> std::result::Result<(), Error>
async fn run<Context>(self, mut ctx: Context) -> std::result::Result<(), FatalError>
where
Context: SubsystemContext<Message = AvailabilityDistributionMessage>,
Context: overseer::SubsystemContext<Message = AvailabilityDistributionMessage>,
Expand All @@ -111,13 +111,13 @@ impl AvailabilityDistributionSubsystem {
"pov-receiver",
run_pov_receiver(sender.clone(), pov_req_receiver, metrics.clone()).boxed(),
)
.map_err(Error::SpawnTask)?;
.map_err(FatalError::SpawnTask)?;

ctx.spawn(
"chunk-receiver",
run_chunk_receiver(sender, chunk_req_receiver, metrics.clone()).boxed(),
)
.map_err(Error::SpawnTask)?;
.map_err(FatalError::SpawnTask)?;
}

loop {
Expand All @@ -132,9 +132,9 @@ impl AvailabilityDistributionSubsystem {
// Handle task messages sending:
let message = match action {
Either::Left(subsystem_msg) =>
subsystem_msg.map_err(|e| Error::IncomingMessageChannel(e))?,
subsystem_msg.map_err(|e| FatalError::IncomingMessageChannel(e))?,
Either::Right(from_task) => {
let from_task = from_task.ok_or(Error::RequesterExhausted)?;
let from_task = from_task.ok_or(FatalError::RequesterExhausted)?;
ctx.send_message(from_task).await;
continue
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use polkadot_subsystem::{
};

use crate::{
error::{Error, Result},
error::{Error, FatalError, JfyiError, Result},
metrics::{FAILED, NOT_FOUND, SUCCEEDED},
Metrics, LOG_TARGET,
};
Expand All @@ -56,7 +56,7 @@ where
let authority_id = info
.discovery_keys
.get(from_validator.0 as usize)
.ok_or(Error::InvalidValidatorIndex)?
.ok_or(JfyiError::InvalidValidatorIndex)?
.clone();
let (req, pending_response) = OutgoingRequest::new(
Recipient::Authority(authority_id.clone()),
Expand All @@ -77,7 +77,7 @@ where
"pov-fetcher",
fetch_pov_job(pov_hash, authority_id, pending_response.boxed(), span, tx, metrics).boxed(),
)
.map_err(|e| Error::SpawnTask(e))?;
.map_err(|e| FatalError::SpawnTask(e))?;
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use polkadot_subsystem::{
};

use crate::{
error::{Error, Result},
error::{FatalError, Result},
metrics::{Metrics, FAILED, SUCCEEDED},
requester::session_cache::{BadValidators, SessionInfo},
LOG_TARGET,
Expand Down Expand Up @@ -185,7 +185,7 @@ impl FetchTask {
let (handle, kill) = oneshot::channel();

ctx.spawn("chunk-fetcher", running.run(kill).boxed())
.map_err(|e| Error::SpawnTask(e))?;
.map_err(|e| FatalError::SpawnTask(e))?;

Ok(FetchTask { live_in, state: FetchedState::Started(handle) })
} else {
Expand Down
8 changes: 5 additions & 3 deletions node/network/availability-distribution/src/requester/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ use polkadot_subsystem::{
ActivatedLeaf, ActiveLeavesUpdate, LeafStatus, SubsystemContext,
};

use super::{Metrics, Result, LOG_TARGET};
use crate::error::Error;
use super::{FatalError, Metrics, Result, LOG_TARGET};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -324,6 +323,9 @@ where
})
.await;

let ancestors = rx.await.map_err(Error::ChainApiSenderDropped)?.map_err(Error::ChainApi)?;
let ancestors = rx
.await
.map_err(FatalError::ChainApiSenderDropped)?
.map_err(FatalError::ChainApi)?;
Ok(ancestors)
}
12 changes: 6 additions & 6 deletions node/network/availability-distribution/src/responder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use polkadot_primitives::v1::{CandidateHash, ValidatorIndex};
use polkadot_subsystem::{jaeger, messages::AvailabilityStoreMessage, SubsystemSender};

use crate::{
error::{Error, Result},
error::{JfyiError, Result},
metrics::{Metrics, FAILED, NOT_FOUND, SUCCEEDED},
LOG_TARGET,
};
Expand Down Expand Up @@ -170,7 +170,7 @@ where
},
};

req.send_response(response).map_err(|_| Error::SendResponse)?;
req.send_response(response).map_err(|_| JfyiError::SendResponse)?;
Ok(result)
}

Expand Down Expand Up @@ -206,7 +206,7 @@ where
Some(chunk) => v1::ChunkFetchingResponse::Chunk(chunk.into()),
};

req.send_response(response).map_err(|_| Error::SendResponse)?;
req.send_response(response).map_err(|_| JfyiError::SendResponse)?;
Ok(result)
}

Expand All @@ -215,7 +215,7 @@ async fn query_chunk<Sender>(
sender: &mut Sender,
candidate_hash: CandidateHash,
validator_index: ValidatorIndex,
) -> Result<Option<ErasureChunk>>
) -> std::result::Result<Option<ErasureChunk>, JfyiError>
where
Sender: SubsystemSender,
{
Expand All @@ -234,7 +234,7 @@ where
error = ?e,
"Error retrieving chunk",
);
Error::QueryChunkResponseChannel(e)
JfyiError::QueryChunkResponseChannel(e)
})?;
Ok(result)
}
Expand All @@ -252,6 +252,6 @@ where
.send_message(AvailabilityStoreMessage::QueryAvailableData(candidate_hash, tx).into())
.await;

let result = rx.await.map_err(|e| Error::QueryAvailableDataResponseChannel(e))?;
let result = rx.await.map_err(JfyiError::QueryAvailableDataResponseChannel)?;
Ok(result)
}
7 changes: 3 additions & 4 deletions node/network/availability-recovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,8 +993,8 @@ impl AvailabilityRecoverySubsystem {
}
}
in_req = recv_req => {
match in_req.into_nested() {
Ok(Ok(req)) => {
match in_req.into_nested().map_err(|fatal| SubsystemError::with_origin("availability-recovery", fatal))? {
Ok(req) => {
match query_full_data(&mut ctx, req.payload.candidate_hash).await {
Ok(res) => {
let _ = req.send_response(res.into());
Expand All @@ -1010,8 +1010,7 @@ impl AvailabilityRecoverySubsystem {
}
}
}
Err(fatal) => return Err(SubsystemError::with_origin("availability-recovery", fatal)),
Ok(Err(jfyi)) => {
Err(jfyi) => {
tracing::debug!(
target: LOG_TARGET,
error = ?jfyi,
Expand Down
7 changes: 3 additions & 4 deletions node/network/collator-protocol/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ pub enum Error {
/// We basically always want to try and continue on error. This utility function is meant to
/// consume top-level errors by simply logging them.
pub fn log_error(result: Result<()>, ctx: &'static str) -> std::result::Result<(), FatalError> {
match result.into_nested() {
Ok(Ok(())) => Ok(()),
Err(fatal) => Err(fatal),
Ok(Err(jfyi)) => {
match result.into_nested()? {
Ok(()) => Ok(()),
Err(jfyi) => {
tracing::warn!(target: LOG_TARGET, error = ?jfyi, ctx);
Ok(())
},
Expand Down
2 changes: 1 addition & 1 deletion node/network/protocol/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ sc-authority-discovery = { git = "https://github.com/paritytech/substrate", bran
strum = { version = "0.24", features = ["derive"] }
futures = "0.3.21"
thiserror = "1.0.30"
fatality = "0.0.6"
fatality = "0.0.6"
8 changes: 4 additions & 4 deletions node/network/protocol/src/request_response/incoming/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ where
fn try_from_raw(
raw: sc_network::config::IncomingRequest,
reputation_changes: Vec<UnifiedReputationChange>,
) -> Result<Self> {
) -> std::result::Result<Self, JfyiError> {
let sc_network::config::IncomingRequest { payload, peer, pending_response } = raw;
let payload = match Req::decode(&mut payload.as_ref()) {
Ok(payload) => payload,
Expand All @@ -98,9 +98,9 @@ where
};

if let Err(_) = pending_response.send(response) {
return Err(Error::DecodingErrorNoReputationChange(peer, err))
return Err(JfyiError::DecodingErrorNoReputationChange(peer, err))
}
return Err(Error::DecodingError(peer, err))
return Err(JfyiError::DecodingError(peer, err))
},
};
Ok(Self::new(peer, payload, pending_response))
Expand Down Expand Up @@ -224,7 +224,7 @@ where
F: FnOnce() -> Vec<UnifiedReputationChange>,
{
let req = match self.raw.next().await {
None => return Err(Error::RequestChannelExhausted.into()),
None => return Err(FatalError::RequestChannelExhausted.into()),
Some(raw) => IncomingRequest::<Req>::try_from_raw(raw, reputation_changes())?,
};
Ok(req)
Expand Down
2 changes: 1 addition & 1 deletion node/subsystem-util/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ parity-scale-codec = { version = "3.0.0", default-features = false, features = [
pin-project = "1.0.9"
rand = "0.8.5"
thiserror = "1.0.30"
fatality = "0.0.6"
tracing = "0.1.31"
derive_more = "0.99.17"
lru = "0.7.2"
Expand All @@ -25,7 +26,6 @@ polkadot-primitives = { path = "../../primitives" }
polkadot-node-primitives = { path = "../primitives" }
polkadot-overseer = { path = "../overseer" }
metered-channel = { path = "../metered-channel" }
fatality = "0.0.6"

sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand Down

0 comments on commit 99e6900

Please sign in to comment.