refactor: idiomatic rust

[olivermrose]

Attempting to see how far AI gets me...

---

EventSub Module Refactoring

Bug Fix: Double-spawn made error handling dead code

The original connect method spawned an inner tokio::spawn and always returned Ok(()), so the error check in mod.rs could never trigger. Removed the inner spawn so connect runs directly in the caller's spawned task, making the error path functional.

client.rs — Major Changes

1. Eliminated self: Arc<Self> pattern — process_stream, handle_text, handle_message, and reconnect all changed from self: Arc<Self> to &self. All interior mutation already uses Mutex, AtomicBool, and mpsc::UnboundedSender which work through shared references. This removed ~6 unnecessary Arc::clone calls.

2. Fixed unwrap()/expect() in production code:

• self.sender.send(payload).unwrap() → logs a warning if the receiver is dropped
• .expect("missing reconnect_url") → returns a proper Error::Generic

3. Replaced silent error swallowing in handle_text — Deserialization failures were silently dropped. Now logs a warn with the error before returning Ok(None).

4. Simplified confusing compare_exchange logic — The inverted compare_exchange(...).is_err() for checking "is initial connection" was replaced with self.reconnecting.swap(false, ...) and a clear if was_reconnecting / else branch.

5. Removed unnecessary allocation — Eliminated TWITCH_EVENTSUB_WS_URI.to_string() since connect_async accepts &str.

6. Made Keepalive match explicit — Replaced _ => () wildcard with Ws::Keepalive => () so the compiler catches any future variant additions.

7. Changed subscribe_all to accept &[(EventType, &serde_json::Value)] — Avoids requiring callers to allocate a Vec.

8. Changed unsubscribe to accept event: &str — Avoids requiring callers to clone/own the event string.

9. Simplified unsubscribe_all — Combined starts_with + strip_prefix + unwrap into a single filter_map with strip_prefix. Also used iterator chain with filter_map + flatten instead of a manual for loop for collecting results.

10. Made set_connected private — Only used internally.

11. Simplified subscription restoration — Replaced the nested if !map.is_empty() + for loop + manual Vec with a drain().filter_map().collect() iterator chain.

mod.rs — Changes

12. Fixed channel.send(message).unwrap() — Now handles the error gracefully by logging and breaking the loop when the frontend channel closes.

13. Connection errors are now logged — Changed is_err() to if let Err(err) with tracing::error!.

14. Removed Ok::<_, Error>(()) — No longer needed since the spawn block now returns () directly.

api.rs — Caller Updates

Updated both subscribe_all call sites to pass &events / &subs_ref (slice) instead of owned Vec.

---

IRC Module Refactoring

irc/mod.rs

Fixed channel.send(message).unwrap() — Now handles the error gracefully by logging a warning and breaking the forwarding loop when the frontend channel closes, instead of panicking.

irc/client/mod.rs

Replaced 3 unwrap() calls in connect, join, part — These would panic if the ClientLoopWorker stopped. Now they log descriptive errors and return gracefully. This prevents cascading panics when the IRC connection is in a degraded state.

irc/client/event_loop.rs

1. Used wrapping_add(1) instead of overflowing_add(1).0 — Semantically clearer; both are equivalent but wrapping_add directly communicates intent.

2. Fixed unwrap() on send in run_incoming_forward_task — If the client_loop_tx send fails, the loop now breaks cleanly instead of panicking. Also flattened the nested if let chain into let...else guards for better readability.

IRC message module — No changes

The message parsing code (commands.rs, prefix.rs, tags.rs, twitch.rs) is well-structured with comprehensive error types, clean extension trait patterns, and appropriate use of ? propagation. No refactoring needed.

---

7TV Module Refactoring

seventv/client.rs — Major Refactor

1. Bug fix: Removed double-spawn (same bug as eventsub) — connect spawned an inner tokio::spawn and always returned Ok(()), making the error handling in mod.rs dead code. Now connect runs the reconnection loop directly, and errors propagate to the caller.

2. Changed connect from self: Arc<Self> to &self — All internal mutation uses Mutex, AtomicBool, and mpsc::UnboundedSender which work through shared references. This eliminated the Arc::clone calls inside the method.

3. Removed unnecessary Arc wrappers on fields — session_id, subscriptions, and message_rx were Arc<Mutex<...>> but the struct itself lives behind Arc<SeventTvClient>. Simplified to just Mutex<...>.

4. Extracted handle_ws_message method — The deeply nested opcode handling was inlined in the WebSocket read loop. Extracted into a dedicated async fn handle_ws_message(&self, msg: WebSocketMessage) for clarity.

5. Fixed silent error swallowing — Deserialization failures (serde_json::from_str) were silently dropped with if let Ok(...). Now logs a warning on failure.

6. Fixed .split_once(':').unwrap() — When restoring subscriptions after a failed resume, a malformed key would panic. Now uses let Some(...) else { continue } with a warning log.

7. Simplified unsubscribe_all — Replaced starts_with + strip_prefix + unwrap with a single filter_map + strip_prefix.

seventv/mod.rs

1. Fixed dead error path — Changed Arc::clone(&client).connect().await.is_err() to client.connect().await with if let Err(err) to actually log and handle the error.

2. Fixed channel.send(message).unwrap() — Handles gracefully with a warning log and loop break.

[Copilot]

Pull request overview

Refactors the EventSub, 7TV, and IRC connection code to be more idiomatic Rust by removing unnecessary Arc patterns and unwrap() calls, while making error paths actually reachable (notably by removing double-spawns in websocket connect() implementations).

Changes:

• Fixes previously-dead error handling by removing inner tokio::spawn usage in websocket connect() paths and improving error logging.
• Replaces multiple unwrap()/expect() usages with structured error handling or warning logs across EventSub/7TV/IRC.
• Adjusts APIs to avoid unnecessary allocations (e.g., subscribe_all now takes a slice; unsubscribe now takes &str) and simplifies iterator-based collection logic.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src-tauri/src/seventv/mod.rs	Makes 7TV connection failures observable/logged and avoids panicking when frontend channel closes.
src-tauri/src/seventv/client.rs	Refactors 7TV client internals (removes extra Arc<Mutex<...>>, Arc<Self> method receivers) and improves error handling/logging.
src-tauri/src/irc/mod.rs	Avoids panic when IRC frontend channel closes by handling send errors.
src-tauri/src/irc/client/mod.rs	Replaces unwrap()s in IRC client commands with graceful handling/logging when the loop is stopped.
src-tauri/src/irc/client/event_loop.rs	Small refactor for clarity (wrapping_add) and avoids panic on forwarding-task send failure.
src-tauri/src/eventsub/mod.rs	Makes EventSub connection failures observable/logged and avoids panicking when frontend channel closes.
src-tauri/src/eventsub/client.rs	Removes double-spawn, switches methods to &self, improves deserialization/reconnect handling, and adjusts subscription APIs to slices/borrows.
src-tauri/src/api.rs	Updates call sites to match the new EventSubClient::subscribe_all(&[..]) signature.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.