Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ log = { workspace = true }
moka = { version = "0.12.12", features = ["future"], optional = true }
portable-atomic = { workspace = true }
prost = { workspace = true }
qrcode = "0.14.1"
rand = { workspace = true }
scopeguard = "1.2"
serde = { workspace = true }
Expand Down
15 changes: 15 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use chrono::{Local, Utc};
use log::{error, info};
use qrcode::{QrCode, render::unicode};
use std::sync::Arc;
use wacore::proto_helpers::MessageExt;
use wacore::types::events::Event;
Expand Down Expand Up @@ -31,6 +32,7 @@ fn main() {
eprintln!("Phone number provided: {}", phone);
if let Some(ref code) = custom_code {
eprintln!("Custom pair code: {}", code);
print_qr_to_terminal(&code);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
eprintln!("Will use pair code authentication (concurrent with QR)");
}
Expand Down Expand Up @@ -90,6 +92,7 @@ fn main() {
timeout.as_secs()
);
info!("\n{}\n", code);
print_qr_to_terminal(&code);
info!("----------------------------------------");
Comment on lines +94 to 95

@coderabbitai coderabbitai Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check the file size to determine how to read it
wc -l src/main.rs

Repository: jlucaso1/whatsapp-rust

Length of output: 81


🏁 Script executed:

# Read the relevant sections - around lines 94-95 (callsite)
sed -n '85,110p' src/main.rs

Repository: jlucaso1/whatsapp-rust

Length of output: 1504


🏁 Script executed:

# Read the function definition around lines 267-272
sed -n '260,280p' src/main.rs

Repository: jlucaso1/whatsapp-rust

Length of output: 422


🏁 Script executed:

# Also check for any other references to print_qr_to_terminal
rg 'print_qr_to_terminal' -n src/main.rs

Repository: jlucaso1/whatsapp-rust

Length of output: 223


🏁 Script executed:

# Check if log/logging is available in the file
head -30 src/main.rs | grep -E "use log|use tracing"

Repository: jlucaso1/whatsapp-rust

Length of output: 89


🏁 Script executed:

# Check imports and see what logging is used in the file
rg "log::|info!|warn!" src/main.rs | head -10

Repository: jlucaso1/whatsapp-rust

Length of output: 679


Offload QR render/print from async event handling and surface QR generation failures.

print_qr_to_terminal runs CPU-intensive QR code rendering and blocking println! synchronously, but it's called from async event handlers (lines 94 and 105). This blocks Tokio worker threads. Additionally, QrCode::new errors are silently ignored.

Wrap the function body in tokio::task::spawn_blocking and make the function async, as per the coding guideline: "All I/O uses Tokio; wrap blocking I/O and heavy CPU work in tokio::task::spawn_blocking".

Suggested change
-                        print_qr_to_terminal(&code);
+                        print_qr_to_terminal(&code).await;
@@
-                        print_qr_to_terminal(&code);
+                        print_qr_to_terminal(&code).await;
@@
-pub fn print_qr_to_terminal(txt: &str) {
-    if let Ok(code) = QrCode::new(txt) {
-        // Render QR code as UTF-8 characters for the terminal
-        let qr_string = code.render::<unicode::Dense1x2>().build();
-    
-        // Print to terminal
-        println!("{}", qr_string);
-    }
+pub async fn print_qr_to_terminal(txt: &str) {
+    let payload = txt.to_owned();
+    let _ = tokio::task::spawn_blocking(move || {
+        match QrCode::new(&payload) {
+            Ok(code) => {
+                let qr_string = code.render::<unicode::Dense1x2>().build();
+                println!("{}", qr_string);
+            }
+            Err(e) => {
+                log::warn!("Failed to generate QR code for terminal output: {}", e);
+            }
+        }
+    })
+    .await;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` around lines 94 - 95, The sync function print_qr_to_terminal
currently does CPU-heavy QR rendering and blocking println! calls (and ignores
QrCode::new errors) while being called from async handlers; change
print_qr_to_terminal to async and move its blocking work into
tokio::task::spawn_blocking so the async runtime isn’t blocked, propagate or log
QrCode::new errors instead of ignoring them, and await the spawned task from
callers at the existing call sites where print_qr_to_terminal(&code) is invoked.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, if it should be implemented. Need to ask the maintainers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abhi3700, that's a reasonable point to raise. Here's a summary of both sides to help frame the discussion with maintainers:


Why the fix matters (the concern is valid):

  • print_qr_to_terminal performs CPU work (QrCode::new + rendering) and a blocking println!, all inside an async event handler closure. Per the project's own coding guideline, blocking work in async contexts should be offloaded via tokio::task::spawn_blocking.
  • If the Tokio runtime is configured with a small thread pool (or is under load), blocking a worker thread—even briefly—can delay other async tasks.

Why the impact is low in practice:

  • This is a one-shot operation: the QR code is displayed once during authentication and never again. It's not a hot path.
  • QR rendering for a small payload is typically sub-millisecond, so the actual blockage is negligible for a dev/CLI tool.
  • The PR description image confirms this is a developer-facing CLI workflow, not a production service.

Recommendation for maintainers:
The fix aligns with the project's stated guideline and is good async hygiene. Whether to require it depends on how strictly they want to enforce that guideline for non-critical code paths. A middle ground would be to at least log the QrCode::new error (currently silently swallowed) even if spawn_blocking is deferred — that part has no downside.

Feel free to share this context with the maintainers when asking for their preference!


🧠 Learnings used
Learnt from: CR
Repo: jlucaso1/whatsapp-rust PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T17:48:51.780Z
Learning: Applies to **/*.rs : All I/O uses Tokio; wrap blocking I/O (`ureq`) and heavy CPU work in `tokio::task::spawn_blocking`

}
Event::PairingCode { code, timeout } => {
Expand All @@ -100,6 +103,7 @@ fn main() {
info!("> Link with phone number instead");
info!("");
info!(" >>> {} <<<", code);
print_qr_to_terminal(&code);
info!("");
info!("========================================");
}
Expand Down Expand Up @@ -258,3 +262,14 @@ fn parse_arg(args: &[String], long: &str, short: &str) -> Option<String> {
}
None
}

/// Show QR code on terminal as image.
pub fn print_qr_to_terminal(txt: &str) {
let code = QrCode::new(txt).unwrap();

// Render QR code as UTF-8 characters for the terminal
let qr_string = code.render::<unicode::Dense1x2>().build();

// Print to terminal
println!("{}", qr_string);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading