pr reviews

eigerco · Jun 24, 2024 · 2a09295 · 2a09295
1 parent 48670ee
commit 2a09295
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 28 deletions.
diff --git a/node-wasm/src/node.rs b/node-wasm/src/node.rs
@@ -67,9 +67,7 @@ impl NodeDriver {
 
         // For chrome we default to running in a dedicated Worker because:
         // 1. Chrome Android does not support SharedWorkers at all
-        // 2. On desktop Chrome, if tab running lumina is reloaded, it fails to re-connect to
-        //    previous worker instance and doesn't create a new one, leaving it in non functioning
-        //    limbo
+        // 2. On desktop Chrome, restarting Lumina's worker causes all network connections to fail.
         let default_worker_type = if is_chrome().unwrap_or(false) {
             NodeWorkerKind::Dedicated
         } else {
@@ -183,7 +181,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let header = response.into_header().check_variant()?;
 
-        header.into_result()
+        header.into()
     }
 
     /// Request a header for the block with a given hash from the network.
@@ -192,7 +190,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let header = response.into_header().check_variant()?;
 
-        header.into_result()
+        header.into()
     }
 
     /// Request a header for the block with a given height from the network.
@@ -201,7 +199,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let header = response.into_header().check_variant()?;
 
-        header.into_result()
+        header.into()
     }
 
     /// Request headers in range (from, from + amount] from the network.
@@ -219,7 +217,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let headers = response.into_headers().check_variant()?;
 
-        headers.into_result()
+        headers.into()
     }
 
     /// Get current header syncing info.
@@ -246,7 +244,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let header = response.into_header().check_variant()?;
 
-        header.into_result()
+        header.into()
     }
 
     /// Get a synced header for the block with a given hash.
@@ -255,7 +253,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let header = response.into_header().check_variant()?;
 
-        header.into_result()
+        header.into()
     }
 
     /// Get a synced header for the block with a given height.
@@ -264,7 +262,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let header = response.into_header().check_variant()?;
 
-        header.into_result()
+        header.into()
     }
 
     /// Get synced headers from the given heights range.
@@ -288,7 +286,7 @@ impl NodeDriver {
         let response = self.client.exec(command).await?;
         let headers = response.into_headers().check_variant()?;
 
-        headers.into_result()
+        headers.into()
     }
 
     /// Get data sampling metadata of an already sampled height.

diff --git a/node-wasm/src/utils.rs b/node-wasm/src/utils.rs
@@ -12,7 +12,7 @@ use tracing_subscriber::fmt::time::UtcTime;
 use tracing_subscriber::prelude::*;
 use tracing_web::{performance_layer, MakeConsoleWriter};
 use wasm_bindgen::prelude::*;
-use web_sys::{Crypto, DedicatedWorkerGlobalScope, SharedWorker, SharedWorkerGlobalScope, Worker};
+use web_sys::{Crypto, DedicatedWorkerGlobalScope, Navigator, SharedWorker, SharedWorkerGlobalScope, Worker};
 
 use crate::error::{Context, Error, Result};
 
@@ -110,6 +110,12 @@ impl WorkerSelf for Worker {
     }
 }
 
+/// This type is useful in cases where we want to deal with de/serialising `Result<T, E>`, with
+/// [`serde_wasm_bindgen::preserve`] where `T` is a JavaScript object (which are not serializable by 
+/// Rust standards, but can be passed through unchanged via cast as they implement [`JsCast`]).
+///
+/// [`serde_wasm_bindgen::preserve`]: https://docs.rs/serde-wasm-bindgen/latest/serde_wasm_bindgen/preserve
+/// [`JsCast`]: https://docs.rs/wasm-bindgen/latest/wasm_bindgen/trait.JsCast.html
 #[derive(Serialize, Deserialize, Debug)]
 pub(crate) enum JsResult<T, E>
 where
@@ -121,17 +127,6 @@ where
     Err(E),
 }
 
-// once try_trait_v2 is stabilised, this can go
-impl<T, E> JsResult<T, E>
-where
-    T: JsCast + Debug,
-    E: Serialize + DeserializeOwned + Debug,
-{
-    pub fn into_result(self) -> Result<T, E> {
-        self.into()
-    }
-}
-
 impl<T, E> From<Result<T, E>> for JsResult<T, E>
 where
     T: JsCast + Debug,
@@ -160,19 +155,23 @@ where
 
 const CHROME_USER_AGENT_DETECTION_STR: &str = "Chrome/";
 
-// currently there's issue with SharedWorkers on Chrome, where restarting lumina's worker
+// Currently, there's an issue with SharedWorkers on Chrome where restarting Lumina's worker
 // causes all network connections to fail. Until that's resolved detect chrome and apply
 // a workaround.
 pub(crate) fn is_chrome() -> Result<bool, Error> {
-    js_sys::Reflect::get(&js_sys::global(), &JsValue::from_str("navigator"))
-        .context("failed to get `navigator` from global object")?
-        .dyn_into::<web_sys::Navigator>()
-        .context("`navigator` is not instanceof `Navigator`")?
+    get_navigator()?
         .user_agent()
         .context("could not get UserAgent from Navigator")
         .map(|user_agent| user_agent.contains(CHROME_USER_AGENT_DETECTION_STR))
 }
 
+pub(crate) fn get_navigator() -> Result<Navigator, Error> {
+    js_sys::Reflect::get(&js_sys::global(), &JsValue::from_str("navigator"))
+        .context("failed to get `navigator` from global object")?
+        .dyn_into::<Navigator>()
+        .context("`navigator` is not instanceof `Navigator`")
+}
+
 pub(crate) fn get_crypto() -> Result<Crypto, Error> {
     js_sys::Reflect::get(&js_sys::global(), &JsValue::from_str("crypto"))
         .context("failed to get `crypto` from global object")?