Take advantage of a new std::io::Error::other (#10198)

Now that std::io::Error::other has stabilised, use it to simplify bunch of code.
near · Nov 17, 2023 · 87cca9a · 87cca9a
1 parent ed3dc7e
commit 87cca9a
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 65 deletions.
diff --git a/core/store/src/db/colddb.rs b/core/store/src/db/colddb.rs
@@ -29,7 +29,7 @@ impl ColdDB {
     // Checks if the column is is the cold db and returns an error if not.
     fn check_is_in_colddb(col: DBCol) -> std::io::Result<()> {
         if !col.is_in_colddb() {
-            return Err(std::io::Error::new(std::io::ErrorKind::Other, Self::err_msg(col)));
+            return Err(std::io::Error::other(Self::err_msg(col)));
         }
         Ok(())
     }

diff --git a/core/store/src/db/rocksdb.rs b/core/store/src/db/rocksdb.rs
@@ -114,7 +114,7 @@ impl RocksDB {
         columns: &[DBCol],
     ) -> io::Result<Self> {
         let counter = instance_tracker::InstanceTracker::try_new(store_config.max_open_files)
-            .map_err(other_error)?;
+            .map_err(io::Error::other)?;
         let (db, db_opt) = Self::open_db(path, store_config, mode, temp, columns)?;
         let cf_handles = Self::get_cf_handles(&db, columns);
         Ok(Self { db, db_opt, cf_handles, _instance_tracker: counter })
@@ -144,7 +144,7 @@ impl RocksDB {
         } else {
             DB::open_cf_descriptors(&options, path, cf_descriptors)
         }
-        .map_err(into_other)?;
+        .map_err(io::Error::other)?;
         if cfg!(feature = "single_thread_rocksdb") {
             // These have to be set after open db
             let mut env = Env::new().unwrap();
@@ -200,7 +200,7 @@ impl RocksDB {
         } else if cfg!(debug_assertions) {
             panic!("The database instance isn’t setup to access {col}");
         } else {
-            Err(other_error(format!("{col}: no such column")))
+            Err(io::Error::other(format!("{col}: no such column")))
         }
     }
 
@@ -269,7 +269,7 @@ impl<'a> Iterator for RocksDBIterator<'a> {
     type Item = io::Result<(Box<[u8]>, Box<[u8]>)>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        Some(self.0.next()?.map_err(into_other))
+        Some(self.0.next()?.map_err(io::Error::other))
     }
 }
 
@@ -314,7 +314,7 @@ impl Database for RocksDB {
         let result = self
             .db
             .get_pinned_cf_opt(self.cf_handle(col)?, key, &read_options)
-            .map_err(into_other)?
+            .map_err(io::Error::other)?
             .map(DBSlice::from_rocksdb_slice);
         timer.observe_duration();
         Ok(result)
@@ -366,7 +366,7 @@ impl Database for RocksDB {
                 }
                 DBOp::DeleteAll { col } => {
                     let cf_handle = self.cf_handle(col)?;
-                    let range = self.get_cf_key_range(cf_handle).map_err(into_other)?;
+                    let range = self.get_cf_key_range(cf_handle).map_err(io::Error::other)?;
                     if let Some(range) = range {
                         batch.delete_range_cf(cf_handle, range.start(), range.end());
                         // delete_range_cf deletes ["begin_key", "end_key"), so need one more delete
@@ -378,7 +378,7 @@ impl Database for RocksDB {
                 }
             }
         }
-        self.db.write(batch).map_err(into_other)
+        self.db.write(batch).map_err(io::Error::other)
     }
 
     fn compact(&self) -> io::Result<()> {
@@ -392,7 +392,7 @@ impl Database for RocksDB {
         // Need to iterator over all CFs because the normal `flush()` only
         // flushes the default column family.
         for col in DBCol::iter() {
-            self.db.flush_cf(self.cf_handle(col)?).map_err(into_other)?;
+            self.db.flush_cf(self.cf_handle(col)?).map_err(io::Error::other)?;
         }
         Ok(())
     }
@@ -640,14 +640,6 @@ fn parse_statistics(
     Ok(())
 }
 
-fn other_error(msg: String) -> io::Error {
-    io::Error::new(io::ErrorKind::Other, msg)
-}
-
-fn into_other(error: rocksdb::Error) -> io::Error {
-    io::Error::new(io::ErrorKind::Other, error.into_string())
-}
-
 /// Returns name of a RocksDB column family corresponding to given column.
 ///
 /// Historically we used `col##` names (with `##` being index of the column).

diff --git a/core/store/src/db/rocksdb/instance_tracker.rs b/core/store/src/db/rocksdb/instance_tracker.rs
@@ -205,17 +205,13 @@ impl NoFile for RealNoFile {
 
 #[test]
 fn test_ensure_max_open_files_limit() {
-    fn other_error(msg: &str) -> std::io::Error {
-        super::other_error(msg.to_string())
-    }
-
     /// Mock implementation of NoFile interface.
     struct MockNoFile<'a>(&'a mut (u64, u64));
 
     impl<'a> NoFile for MockNoFile<'a> {
         fn get(&self) -> std::io::Result<(u64, u64)> {
             if self.0 .0 == 666 {
-                Err(other_error("error"))
+                Err(std::io::ErrorKind::Other.into())
             } else {
                 Ok(*self.0)
             }
@@ -224,7 +220,7 @@ fn test_ensure_max_open_files_limit() {
         fn set(&mut self, soft: u64, hard: u64) -> std::io::Result<()> {
             let (old_soft, old_hard) = self.get().unwrap();
             if old_hard == 666000 {
-                Err(other_error("error"))
+                Err(std::io::ErrorKind::Other.into())
             } else {
                 assert!(soft != old_soft, "Pointless call to set");
                 *self.0 = (soft, hard);

diff --git a/core/store/src/db/rocksdb/snapshot.rs b/core/store/src/db/rocksdb/snapshot.rs
@@ -51,7 +51,7 @@ impl std::convert::From<io::Error> for SnapshotError {
 
 impl std::convert::From<::rocksdb::Error> for SnapshotError {
     fn from(err: ::rocksdb::Error) -> Self {
-        super::into_other(err).into()
+        io::Error::other(err).into()
     }
 }
 
@@ -94,7 +94,7 @@ impl Snapshot {
         }
 
         let db = super::RocksDB::open(db_path, config, crate::Mode::ReadWriteExisting, temp)?;
-        let cp = Checkpoint::new(&db.db).map_err(super::into_other)?;
+        let cp = Checkpoint::new(&db.db).map_err(io::Error::other)?;
         cp.create_checkpoint(&snapshot_path)?;
 
         Ok(Self(Some(snapshot_path)))

diff --git a/core/store/src/flat/store_helper.rs b/core/store/src/flat/store_helper.rs
@@ -139,17 +139,13 @@ pub fn encode_flat_state_db_key(shard_uid: ShardUId, key: &[u8]) -> Vec<u8> {
 
 pub fn decode_flat_state_db_key(key: &[u8]) -> io::Result<(ShardUId, Vec<u8>)> {
     if key.len() < 8 {
-        return Err(io::Error::new(
-            io::ErrorKind::Other,
-            format!("expected FlatState key length to be at least 8: {key:?}"),
-        ));
+        return Err(io::Error::other(format!(
+            "expected FlatState key length to be at least 8: {key:?}"
+        )));
     }
     let (shard_uid_bytes, trie_key) = key.split_at(8);
     let shard_uid = shard_uid_bytes.try_into().map_err(|err| {
-        io::Error::new(
-            io::ErrorKind::Other,
-            format!("failed to decode shard_uid as part of FlatState key: {err}"),
-        )
+        io::Error::other(format!("failed to decode shard_uid as part of FlatState key: {err}"))
     })?;
     Ok((shard_uid, trie_key.to_vec()))
 }

diff --git a/core/store/src/metadata.rs b/core/store/src/metadata.rs
@@ -103,7 +103,7 @@ fn read<T: std::str::FromStr>(
 
     match result {
         Some(value) => Ok(value),
-        None => Err(other_error(format!("missing {what}; {msg}"))),
+        None => Err(std::io::Error::other(format!("missing {what}; {msg}"))),
     }
 }
 
@@ -118,19 +118,12 @@ fn maybe_read<T: std::str::FromStr>(
     key: &[u8],
 ) -> std::io::Result<Option<T>> {
     let msg = "it’s not a neard database or database is corrupted";
-    if let Some(bytes) = db.get_raw_bytes(crate::DBCol::DbVersion, key)? {
-        let value = std::str::from_utf8(&bytes)
-            .map_err(|_err| format!("invalid {what}: {bytes:?}; {msg}"))
-            .map_err(other_error)?;
-        let value = T::from_str(value)
-            .map_err(|_err| format!("invalid {what}: ‘{value}’; {msg}"))
-            .map_err(other_error)?;
-        Ok(Some(value))
-    } else {
-        Ok(None)
-    }
-}
-
-fn other_error(msg: String) -> std::io::Error {
-    std::io::Error::new(std::io::ErrorKind::Other, msg)
+    db.get_raw_bytes(crate::DBCol::DbVersion, key)?
+        .map(|bytes| {
+            let value = std::str::from_utf8(&bytes)
+                .map_err(|_err| format!("invalid {what}: {bytes:?}; {msg}"))?;
+            T::from_str(value).map_err(|_err| format!("invalid {what}: ‘{value}’; {msg}"))
+        })
+        .transpose()
+        .map_err(std::io::Error::other)
 }
diff --git a/core/store/src/opener.rs b/core/store/src/opener.rs
@@ -524,7 +524,7 @@ impl<'a> DBOpener<'a> {
         let metadata = DbMetadata::read(&db)?;
         if want_version != metadata.version {
             let msg = format!("unexpected DbVersion {}; expected {want_version}", metadata.version);
-            Err(std::io::Error::new(std::io::ErrorKind::Other, msg))
+            Err(std::io::Error::other(msg))
         } else {
             Ok((db, metadata))
         }

diff --git a/runtime/near-test-contracts/build.rs b/runtime/near-test-contracts/build.rs
@@ -1,14 +1,11 @@
-use std::path::Path;
-use std::path::PathBuf;
 use std::process::Command;
-use std::{env, fs, io, process};
 
 type Error = Box<dyn std::error::Error>;
 
 fn main() {
     if let Err(err) = try_main() {
         eprintln!("{}", err);
-        process::exit(1);
+        std::process::exit(1);
     }
 }
 
@@ -39,14 +36,14 @@ fn build_contract(dir: &str, args: &[&str], output: &str) -> Result<(), Error> {
 
     let src =
         target_dir.join(format!("wasm32-unknown-unknown/release/{}.wasm", dir.replace('-', "_")));
-    fs::copy(&src, format!("./res/{}.wasm", output))
+    std::fs::copy(&src, format!("./res/{}.wasm", output))
         .map_err(|err| format!("failed to copy `{}`: {}", src.display(), err))?;
     println!("cargo:rerun-if-changed=./{}/src/lib.rs", dir);
     println!("cargo:rerun-if-changed=./{}/Cargo.toml", dir);
     Ok(())
 }
 
-fn cargo_build_cmd(target_dir: &Path) -> Command {
+fn cargo_build_cmd(target_dir: &std::path::Path) -> Command {
     let mut res = Command::new("cargo");
 
     res.env_remove("CARGO_BUILD_RUSTFLAGS");
@@ -61,15 +58,18 @@ fn cargo_build_cmd(target_dir: &Path) -> Command {
 }
 
 fn check_status(mut cmd: Command) -> Result<(), Error> {
-    let status = cmd.status().map_err(|err| {
-        io::Error::new(io::ErrorKind::Other, format!("command `{:?}` failed to run: {}", cmd, err))
-    })?;
-    if !status.success() {
-        return Err(format!("command `{:?}` exited with non-zero status: {:?}", cmd, status).into());
-    }
-    Ok(())
+    cmd.status()
+        .map_err(|err| format!("command `{cmd:?}` failed to run: {err}"))
+        .and_then(|status| {
+            if status.success() {
+                Ok(())
+            } else {
+                Err(format!("command `{cmd:?}` exited with non-zero status: {status:?}"))
+            }
+        })
+        .map_err(Error::from)
 }
 
-fn out_dir() -> PathBuf {
-    env::var("OUT_DIR").unwrap().into()
+fn out_dir() -> std::path::PathBuf {
+    std::env::var("OUT_DIR").unwrap().into()
 }