From cb58e761caa3db4a33cfe4a2a462584deddddfb0 Mon Sep 17 00:00:00 2001 From: AnthonyMichaelTDM <68485672+AnthonyMichaelTDM@users.noreply.github.com> Date: Thu, 2 May 2024 23:10:17 -0700 Subject: [PATCH 1/4] fix(dev): don't panic in `is_connected` if we couldn't get a connection to the db --- create-rust-app/src/dev/controller.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/create-rust-app/src/dev/controller.rs b/create-rust-app/src/dev/controller.rs index 9c9af2f1..bebe369f 100644 --- a/create-rust-app/src/dev/controller.rs +++ b/create-rust-app/src/dev/controller.rs @@ -43,12 +43,11 @@ pub fn query_db(db: &Database, body: &MySqlQuery) -> Result bool { - let mut db = db.pool.clone().get().unwrap(); + let Ok(mut db) = db.pool.clone().get() else { + return false; + }; let is_connected = sql_query("SELECT 1;").execute(&mut db); is_connected.is_err() } From 5077bee7582e951d53bb1ec738440d5844aa5455 Mon Sep 17 00:00:00 2001 From: AnthonyMichaelTDM <68485672+AnthonyMichaelTDM@users.noreply.github.com> Date: Thu, 2 May 2024 23:18:08 -0700 Subject: [PATCH 2/4] perf: assigning the result of `Clone::clone()` may be inefficient https://rust-lang.github.io/rust-clippy/master/index.html#/assigning_clones --- create-rust-app/src/dev/dev_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/create-rust-app/src/dev/dev_server.rs b/create-rust-app/src/dev/dev_server.rs index 5646db5d..e18a7e79 100644 --- a/create-rust-app/src/dev/dev_server.rs +++ b/create-rust-app/src/dev/dev_server.rs @@ -246,7 +246,7 @@ async fn handle_socket(stream: WebSocket, state: Arc) { } DevServerEvent::CompileMessages(messages) => { let mut s = state2.dev.lock().unwrap(); - s.compiler_messages = messages.clone(); + s.compiler_messages.clone_from(&messages); } DevServerEvent::SHUTDOWN => { let mut s = state2.dev.lock().unwrap(); From eb51c59f7be36628fb482b492e4f5b7590cea19d Mon Sep 17 00:00:00 2001 From: AnthonyMichaelTDM <68485672+AnthonyMichaelTDM@users.noreply.github.com> Date: Thu, 2 May 2024 23:22:03 -0700 Subject: [PATCH 3/4] refactor(dev): migrate_db fn to return result instead of ( success, optional error_message ) tuple. We aren't in Go, we have better error handling available so should take advantage of it TODO: There are more cases in this function where it may be better to propagate errors rather than panicing --- create-rust-app/src/dev/controller.rs | 12 ++++++------ create-rust-app/src/dev/dev_server.rs | 6 +++++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/create-rust-app/src/dev/controller.rs b/create-rust-app/src/dev/controller.rs index bebe369f..21560316 100644 --- a/create-rust-app/src/dev/controller.rs +++ b/create-rust-app/src/dev/controller.rs @@ -1,4 +1,5 @@ use crate::Database; +use anyhow::{bail, Result}; use diesel::{ migration::{Migration, MigrationSource}, query_dsl::RunQueryDsl, @@ -135,9 +136,8 @@ pub fn needs_migration(db: &Database) -> bool { /// * cannot find the migrations directory /// * cannot run the migrations /// -/// TODO: return a Result instead of a tuple (bool, Option), this is Rust, not Go -#[must_use] -pub fn migrate_db(db: &Database) -> (bool, /* error message: */ Option) { +/// TODO: Propagate more of these errors instead of panicking +pub fn migrate_db(db: &Database) -> Result<()> { let mut db = db.pool.clone().get().unwrap(); let source = FileBasedMigrations::find_migrations_directory().unwrap(); @@ -145,15 +145,15 @@ pub fn migrate_db(db: &Database) -> (bool, /* error message: */ Option) MigrationHarness::has_pending_migration(&mut db, source.clone()).unwrap(); if !has_pending_migrations { - return (true, None); + return Ok(()); } let op = MigrationHarness::run_pending_migrations(&mut db, source); match op { - Ok(_) => (true, None), + Ok(_) => Ok(()), Err(err) => { println!("{err:#?}"); - (false, Some(err.to_string())) + bail!(err) } } } diff --git a/create-rust-app/src/dev/dev_server.rs b/create-rust-app/src/dev/dev_server.rs index e18a7e79..5cdc354a 100644 --- a/create-rust-app/src/dev/dev_server.rs +++ b/create-rust-app/src/dev/dev_server.rs @@ -300,7 +300,11 @@ async fn handle_socket(stream: WebSocket, state: Arc) { println!("📝 Could not open file `{file_name}`"); }); } else if t.eq_ignore_ascii_case("migrate") { - let (success, error_message) = controller::migrate_db(&state3.db); + let (success, error_message) = + match controller::migrate_db(&state3.db) { + Ok(_) => (true, None), + Err(e) => (false, Some(e.to_string())), + }; state3 .tx From ef930cdff722df42f9a70cc70011246fca0ff9f0 Mon Sep 17 00:00:00 2001 From: AnthonyMichaelTDM <68485672+AnthonyMichaelTDM@users.noreply.github.com> Date: Thu, 2 May 2024 23:26:18 -0700 Subject: [PATCH 4/4] refactor(dev): listen_for_signals, rename variables and refactor error handling https://rust-lang.github.io/rust-clippy/master/index.html#/never_loop the loop only actually runs once, so an `if let` is a much more clear expression. --- create-rust-app/src/dev/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/create-rust-app/src/dev/mod.rs b/create-rust-app/src/dev/mod.rs index 4378fa79..4559210f 100644 --- a/create-rust-app/src/dev/mod.rs +++ b/create-rust-app/src/dev/mod.rs @@ -305,11 +305,14 @@ fn check_exit(state: &DevState) { async fn listen_for_signals(signal_tx: tokio::sync::broadcast::Sender) { let (event_sender, event_receiver) = priority::bounded::(1024); - let (er_s, mut er_r) = tokio::sync::mpsc::channel(64); + let (error_sender, mut error_receiver) = tokio::sync::mpsc::channel(64); // panic on errors tokio::spawn(async move { - while let Some(error) = er_r.recv().await { + // we panic on the first error we receive, so we only need to recv once + // if, in the future, we change the error handling to be more robust (not panic), we'll need to loop here + // like `while let Some(error) = error_receiver.recv().await { ... }` + if let Some(error) = error_receiver.recv().await { panic!( "Error handling process signal:\n==============================\n{:#?}", error @@ -328,5 +331,5 @@ async fn listen_for_signals(signal_tx: tokio::sync::broadcast::Sender