diff --git a/src/encoding/keycode.rs b/src/encoding/keycode.rs index 2cc34c69..309f537a 100644 --- a/src/encoding/keycode.rs +++ b/src/encoding/keycode.rs @@ -47,8 +47,8 @@ use de::IntoDeserializer; use serde::{de, ser}; +use crate::errdata; use crate::error::{Error, Result}; -use crate::{errassert, errdata}; // Serializes a key to a binary KeyCode representation. pub fn serialize(key: &T) -> Result> { @@ -365,7 +365,7 @@ impl<'de, 'a> serde::Deserializer<'de> for &'a mut Deserializer<'de> { type Error = Error; fn deserialize_any>(self, _: V) -> Result { - errassert!("must provide type, KeyCode is not self-describing") + panic!("must provide type, KeyCode is not self-describing") } fn deserialize_bool>(self, visitor: V) -> Result { diff --git a/src/error.rs b/src/error.rs index 5260ae47..4f2e5730 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,9 +6,6 @@ pub enum Error { /// The operation was aborted and must be retried. This typically happens /// with e.g. Raft leader changes. Abort, - /// An assertion failure. Should never happen. - /// TODO: include backtrace. - Assert(String), /// Invalid data, typically decoding errors or unexpected internal values. InvalidData(String), /// Invalid user input, typically parser or query errors. @@ -28,7 +25,6 @@ impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { Error::Abort => write!(f, "operation aborted"), - Error::Assert(msg) => write!(f, "assertion failed: {msg}"), Error::InvalidData(msg) => write!(f, "invalid data: {msg}"), Error::InvalidInput(msg) => write!(f, "invalid input: {msg}"), Error::IO(msg) => write!(f, "io error: {msg}"), @@ -48,9 +44,6 @@ impl Error { match self { // Aborts don't happen during application, only leader changes. Error::Abort => true, - // Assertion failures typically indicate invariant violations or - // data corruption, so we should assume it's unsafe to continue. - Error::Assert(_) => false, // Possible data corruption local to this node. Error::InvalidData(_) => false, // Input errors are (likely) deterministic. We could employ command @@ -66,12 +59,6 @@ impl Error { } } -/// Constructs an Error::Assert via format!() and into(). -#[macro_export] -macro_rules! errassert { - ($($args:tt)*) => { $crate::error::Error::Assert(format!($($args)*)).into() }; -} - /// Constructs an Error::InvalidData via format!() and into(). #[macro_export] macro_rules! errdata { @@ -84,15 +71,6 @@ macro_rules! errinput { ($($args:tt)*) => { $crate::error::Error::InvalidInput(format!($($args)*)).into() }; } -/// Returns an Error::Assert if the given condition is false. -/// TODO: use this instead of assert! where appropriate. -#[macro_export] -macro_rules! asserterr { - ($cond:expr, $($args:tt)*) => { - if !$cond { return errassert!($($args)*) } - }; -} - /// Result returning Error. pub type Result = std::result::Result; @@ -152,7 +130,7 @@ impl From> for Error { impl From for Error { fn from(err: hdrhistogram::CreationError) -> Self { - Error::Assert(err.to_string()) + panic!("{err}") } } @@ -170,13 +148,13 @@ impl From for Error { impl From for Error { fn from(err: log::SetLoggerError) -> Self { - Error::Assert(err.to_string()) + panic!("{err}") } } impl From for Error { fn from(err: regex::Error) -> Self { - Error::Assert(err.to_string()) + panic!("{err}") } } diff --git a/src/raft/log.rs b/src/raft/log.rs index c29187a4..da205259 100644 --- a/src/raft/log.rs +++ b/src/raft/log.rs @@ -2,7 +2,6 @@ use super::{NodeID, Term}; use crate::encoding::{self, bincode, Key as _, Value as _}; use crate::error::Result; use crate::storage; -use crate::{asserterr, errassert}; use serde::{Deserialize, Serialize}; @@ -148,12 +147,12 @@ impl Log { /// term does not regress, and that we only vote for one node in a term. pub fn set_term(&mut self, term: Term, vote: Option) -> Result<()> { match self.get_term()? { - (t, _) if term < t => return errassert!("term regression {t} → {term}"), + (t, _) if term < t => panic!("term regression {t} → {term}"), (t, _) if term > t => {} // below, term == t - (0, _) => return errassert!("can't set term 0"), + (0, _) => panic!("can't set term 0"), (t, v) if t == term && v == vote => return Ok(()), (_, None) => {} - (_, v) if vote != v => return errassert!("can't change vote {v:?} → {vote:?}"), + (_, v) if vote != v => panic!("can't change vote {v:?} → {vote:?}"), (_, _) => {} }; self.engine.set(&Key::TermVote.encode()?, bincode::serialize(&(term, vote))?)?; @@ -166,9 +165,9 @@ impl Log { /// The term must be equal to or greater than the previous entry. pub fn append(&mut self, term: Term, command: Option>) -> Result { match self.get(self.last_index)? { - Some(e) if term < e.term => return errassert!("term regression {} → {term}", e.term), - None if self.last_index > 0 => return errassert!("log gap at {}", self.last_index), - None if term == 0 => return errassert!("can't append entry with term 0"), + Some(e) if term < e.term => panic!("term regression {} → {term}", e.term), + None if self.last_index > 0 => panic!("log gap at {}", self.last_index), + None if term == 0 => panic!("can't append entry with term 0"), Some(_) | None => {} } // We could omit the index in the encoded value, since it's also stored @@ -186,11 +185,11 @@ impl Log { pub fn commit(&mut self, index: Index) -> Result { let term = match self.get(index)? { Some(e) if e.index < self.commit_index => { - return errassert!("commit index regression {} → {}", self.commit_index, e.index); + panic!("commit index regression {} → {}", self.commit_index, e.index); } Some(e) if e.index == self.commit_index => return Ok(index), Some(e) => e.term, - None => return errassert!("commit index {index} does not exist"), + None => panic!("commit index {index} does not exist"), }; self.engine.set(&Key::CommitIndex.encode()?, bincode::serialize(&(index, term))?)?; // NB: the commit index doesn't need to be fsynced, since the entries @@ -251,24 +250,24 @@ impl Log { // Check that the entries are well-formed. if first.index == 0 || first.term == 0 { - return errassert!("spliced entry has index or term 0"); + panic!("spliced entry has index or term 0"); } if !entries.windows(2).all(|w| w[0].index + 1 == w[1].index) { - return errassert!("spliced entries are not contiguous"); + panic!("spliced entries are not contiguous"); } if !entries.windows(2).all(|w| w[0].term <= w[1].term) { - return errassert!("spliced entries have term regression"); + panic!("spliced entries have term regression"); } // Check that the entries connect to the existing log (if any), and that the // term doesn't regress. match self.get(first.index - 1)? { Some(base) if first.term < base.term => { - return errassert!("splice term regression {} → {}", base.term, first.term) + panic!("splice term regression {} → {}", base.term, first.term) } Some(_) => {} None if first.index == 1 => {} - None => return errassert!("first index {} must touch existing log", first.index), + None => panic!("first index {} must touch existing log", first.index), } // Skip entries that are already in the log. @@ -276,11 +275,11 @@ impl Log { let mut scan = self.scan(first.index..=last.index)?; while let Some(entry) = scan.next().transpose()? { // [0] is ok, because the scan has the same size as entries. - asserterr!(entry.index == entries[0].index, "index mismatch at {entry:?}"); + assert!(entry.index == entries[0].index, "index mismatch at {entry:?}"); if entry.term != entries[0].term { break; } - asserterr!(entry.command == entries[0].command, "command mismatch at {entry:?}"); + assert!(entry.command == entries[0].command, "command mismatch at {entry:?}"); entries = &entries[1..]; } drop(scan); @@ -293,7 +292,7 @@ impl Log { // Write the entries that weren't already in the log, and remove the // tail of the old log if any. We can't write below the commit index, // since these entries must be immutable. - asserterr!(first.index > self.commit_index, "spliced entries below commit index"); + assert!(first.index > self.commit_index, "spliced entries below commit index"); for entry in entries { self.engine.set(&Key::Entry(entry.index).encode()?, entry.encode()?)?; diff --git a/src/raft/node.rs b/src/raft/node.rs index cd5291d2..a3e6401d 100644 --- a/src/raft/node.rs +++ b/src/raft/node.rs @@ -1228,7 +1228,6 @@ fn quorum_value(mut values: Vec) -> T { mod tests { use super::*; use crate::encoding::{self, bincode, Value as _}; - use crate::errassert; use crate::raft::{ Entry, Request, RequestID, Response, ELECTION_TIMEOUT_RANGE, HEARTBEAT_INTERVAL, MAX_APPEND_ENTRIES, @@ -1478,7 +1477,7 @@ mod tests { Ok(node.into()) } Node::Follower(node) => Ok(node.into_candidate()?.into()), - Node::Leader(node) => errassert!("{} is a leader", node.id), + Node::Leader(node) => panic!("{} is a leader", node.id), }; for id in ids.iter().copied() { self.transition(id, campaign, output)?; diff --git a/src/raft/testscripts/log/append b/src/raft/testscripts/log/append index a2513540..d7f2d255 100644 --- a/src/raft/testscripts/log/append +++ b/src/raft/testscripts/log/append @@ -1,7 +1,7 @@ # Appending an entry with term 0 fails, even on an empty log. !append 0 foo --- -Error: assertion failed: can't append entry with term 0 +Panic: can't append entry with term 0 # Appending to an empty log works. The term doesn't have to be 1, and doesn't # have to match get_term(). The entry is written to the engine and flushed @@ -43,8 +43,8 @@ append → 4@5 None !append 4 old !append 0 --- -Error: assertion failed: term regression 5 → 4 -Error: assertion failed: term regression 5 → 0 +Panic: term regression 5 → 4 +Panic: term regression 5 → 0 # Dump the final status and data. status diff --git a/src/raft/testscripts/log/commit b/src/raft/testscripts/log/commit index 2db081a1..59b2c8f1 100644 --- a/src/raft/testscripts/log/commit +++ b/src/raft/testscripts/log/commit @@ -1,7 +1,7 @@ # Committing fails on an empty engine. !commit 1 --- -Error: assertion failed: commit index 1 does not exist +Panic: commit index 1 does not exist # Add some entries. splice 1@1= 2@1=foo 3@2=bar @@ -11,7 +11,7 @@ splice → 3@2 bar # Committing entry 0 fails. !commit 0 --- -Error: assertion failed: commit index 0 does not exist +Panic: commit index 0 does not exist # Committing entry 1 works, and updates the commit index. # @@ -50,14 +50,14 @@ last=3@2 commit=3@2 !commit 2 status --- -Error: assertion failed: commit index regression 3 → 2 +Panic: commit index regression 3 → 2 last=3@2 commit=3@2 # Committing non-existant indexes error. !commit 4 status --- -Error: assertion failed: commit index 4 does not exist +Panic: commit index 4 does not exist last=3@2 commit=3@2 # Dump the raw values. diff --git a/src/raft/testscripts/log/splice b/src/raft/testscripts/log/splice index f60a737e..c6466679 100644 --- a/src/raft/testscripts/log/splice +++ b/src/raft/testscripts/log/splice @@ -1,12 +1,12 @@ # Splicing at index 0 should fail. !splice 0@1=foo --- -Error: assertion failed: spliced entry has index or term 0 +Panic: spliced entry has index or term 0 # Splicing at index 2 should fail (creates gap). !splice 2@1=foo --- -Error: assertion failed: first index 2 must touch existing log +Panic: first index 2 must touch existing log # Splicing entries at start should work, both with and without commands, and # starting at a term after 1. They should be written to the engine and flushed @@ -36,27 +36,27 @@ last=2@2 commit=0@0 # Splicing multiple duplicate entries should fail. !splice 3@2= 3@2= --- -Error: assertion failed: spliced entries are not contiguous +Panic: spliced entries are not contiguous # Splicing entries with a gap should fail. !splice 3@2= 5@2= --- -Error: assertion failed: spliced entries are not contiguous +Panic: spliced entries are not contiguous # Splicing entries with a term regression should fail. !splice 3@2= 4@1= --- -Error: assertion failed: spliced entries have term regression +Panic: spliced entries have term regression # Splicing entries with a gap from the base should fail. !splice 4@2= --- -Error: assertion failed: first index 4 must touch existing log +Panic: first index 4 must touch existing log # Splicing with a term regression from the base should fail. !splice 3@1= --- -Error: assertion failed: splice term regression 2 → 1 +Panic: splice term regression 2 → 1 # Fully overlapping entries is a noop. splice 1@2= 2@2=command oplog=true @@ -86,7 +86,7 @@ splice → 2@2 command !splice 2@2=foo scan --- -Error: assertion failed: command mismatch at Entry { index: 2, term: 2, command: Some([99, 111, 109, 109, 97, 110, 100]) } +Panic: command mismatch at Entry { index: 2, term: 2, command: Some([99, 111, 109, 109, 97, 110, 100]) } 1@2 None 2@2 command @@ -180,8 +180,8 @@ last=4@6 commit=2@5 !splice 2@7= !splice 1@7= --- -Error: assertion failed: spliced entries below commit index -Error: assertion failed: spliced entries below commit index +Panic: spliced entries below commit index +Panic: spliced entries below commit index # Dump the raw data. dump diff --git a/src/raft/testscripts/log/term b/src/raft/testscripts/log/term index 14d8f0ae..af69eca6 100644 --- a/src/raft/testscripts/log/term +++ b/src/raft/testscripts/log/term @@ -6,7 +6,7 @@ term=0 vote=None # Storing a 0 term errors. !set_term 0 --- -Error: assertion failed: can't set term 0 +Panic: can't set term 0 # set_term stores a term and empty vote, writing it to the engine # and flushing it to durable storage. @@ -46,17 +46,17 @@ term=9 vote=1 # Regressing the term errors. !set_term 8 --- -Error: assertion failed: term regression 9 → 8 +Panic: term regression 9 → 8 # Clearing the vote errors. !set_term 9 --- -Error: assertion failed: can't change vote Some(1) → None +Panic: can't change vote Some(1) → None # Changing the vote errors. !set_term 9 2 --- -Error: assertion failed: can't change vote Some(1) → Some(2) +Panic: can't change vote Some(1) → Some(2) # The above errors should not have changed the term/vote. get_term diff --git a/src/sql/plan/planner.rs b/src/sql/plan/planner.rs index 438c3c11..957a294a 100644 --- a/src/sql/plan/planner.rs +++ b/src/sql/plan/planner.rs @@ -2,8 +2,8 @@ use super::super::parser::ast; use super::super::schema::{Catalog, Column, Table}; use super::super::types::{Expression, Value}; use super::{Aggregate, Direction, Node, Plan}; +use crate::errinput; use crate::error::Result; -use crate::{errassert, errinput}; use std::collections::{HashMap, HashSet}; use std::mem::replace; @@ -29,10 +29,10 @@ impl<'a, C: Catalog> Planner<'a, C> { Ok(match statement { // Transaction control and explain statements should have been handled by session. ast::Statement::Begin { .. } | ast::Statement::Commit | ast::Statement::Rollback => { - return errassert!("unexpected transaction statement {statement:?}") + panic!("unexpected transaction statement {statement:?}") } - ast::Statement::Explain(_) => return errassert!("unexpected explain statement"), + ast::Statement::Explain(_) => panic!("unexpected explain statement"), // DDL statements (schema changes). ast::Statement::CreateTable { name, columns } => Node::CreateTable { @@ -698,7 +698,7 @@ impl Scope { /// Adds a table to the scope. fn add_table(&mut self, label: String, table: Table) -> Result<()> { if self.constant { - return errassert!("can't modify constant scope"); + panic!("can't modify constant scope"); } if self.tables.contains_key(&label) { return errinput!("duplicate table name {label}"); @@ -729,7 +729,7 @@ impl Scope { /// Merges two scopes, by appending the given scope to self. fn merge(&mut self, scope: Scope) -> Result<()> { if self.constant { - return errassert!("can't modify constant scope"); + panic!("can't modify constant scope"); } for (label, table) in scope.tables { if self.tables.contains_key(&label) { @@ -775,7 +775,7 @@ impl Scope { /// and returns a new scope for the projection. fn project(&mut self, projection: &[(Expression, Option)]) -> Result<()> { if self.constant { - return errassert!("can't modify constant scope"); + panic!("can't modify constant scope"); } let mut new = Self::new(); new.tables = self.tables.clone();