# digit v0.2.0 Polish Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Improve digit's error handling and connection robustness: validate query input at parse time, unify timeout error messages, and try all DNS-resolved addresses in parallel.
**Architecture:** Three independent changes to existing modules. Query validation changes the `parse` return type (breaking API change), requiring caller updates. Timeout and parallel DNS changes are internal to `protocol.rs`. No new files, no new dependencies.
**Tech Stack:** Rust 2021, thiserror, std::sync::mpsc, std::thread.
---
### Task 1: Query Validation -- Change API and Add Tests (Red)
**Files:**
- Modify: `src/query.rs`
- Modify: `src/main.rs`
- Modify: `src/protocol.rs` (test module only)
- Modify: `tests/integration.rs`
This task changes the `parse` return type to `Result`, updates all callers so the project compiles, and adds new failing tests for invalid inputs. The validation logic itself is NOT implemented yet -- `parse` returns `Ok(...)` for all inputs, so the new error-case tests will fail.
- [ ] **Step 1: Add QueryError and change parse signature**
In `src/query.rs`, add the `QueryError` type after the `DEFAULT_PORT` constant (line 19):
```rust
/// Errors that can occur when parsing a finger query string.
#[derive(Debug, Clone, PartialEq, thiserror::Error)]
pub enum QueryError {
/// One or more hostname segments in the query are empty.
#[error("invalid query: empty hostname in '{input}'")]
EmptyHostname { input: String },
}
```
Change the `parse` method signature from:
```rust
pub fn parse(input: Option<&str>, long: bool, port: u16) -> Query {
```
to:
```rust
pub fn parse(input: Option<&str>, long: bool, port: u16) -> Result<Query, QueryError> {
```
Change every `return Query {` and trailing `Query {` in the `parse` method body to `return Ok(Query {` and `Ok(Query {` respectively. There are three return points:
1. The empty input early return (line 35): change `return Query {` to `return Ok(Query {`
2. The no-`@` early return (line 48): change `return Query {` to `return Ok(Query {`
3. The final return (line 65): change `Query {` to `Ok(Query {`
And close each with `})` instead of `}`.
- [ ] **Step 2: Update existing unit tests to unwrap**
In `src/query.rs` test module, update every `Query::parse(...)` call to `Query::parse(...).unwrap()`. There are 12 tests with parse calls. Each line like:
```rust
let q = Query::parse(Some("user@host"), false, 79);
```
becomes:
```rust
let q = Query::parse(Some("user@host"), false, 79).unwrap();
```
The tests that need this change are: `parse_user_at_host`, `parse_at_host_lists_users`, `parse_user_only_defaults_to_localhost`, `parse_empty_string_defaults_to_localhost`, `parse_none_defaults_to_localhost`, `parse_forwarding_chain`, `parse_forwarding_chain_no_user`, `parse_long_flag_preserved`, `parse_custom_port_preserved`, `target_host_returns_last_host`, `target_host_single_host`, `parse_three_host_chain`.
- [ ] **Step 3: Add new failing tests for invalid inputs**
Add these tests to the `#[cfg(test)] mod tests` block in `src/query.rs`:
```rust
#[test]
fn parse_trailing_at_is_error() {
let result = Query::parse(Some("user@"), false, 79);
assert_eq!(
result,
Err(QueryError::EmptyHostname {
input: "user@".to_string()
})
);
}
#[test]
fn parse_bare_at_is_error() {
let result = Query::parse(Some("@"), false, 79);
assert_eq!(
result,
Err(QueryError::EmptyHostname {
input: "@".to_string()
})
);
}
#[test]
fn parse_trailing_at_in_chain_is_error() {
let result = Query::parse(Some("user@host@"), false, 79);
assert_eq!(
result,
Err(QueryError::EmptyHostname {
input: "user@host@".to_string()
})
);
}
#[test]
fn parse_double_at_is_error() {
let result = Query::parse(Some("user@@host"), false, 79);
assert_eq!(
result,
Err(QueryError::EmptyHostname {
input: "user@@host".to_string()
})
);
}
#[test]
fn parse_at_host_trailing_at_is_error() {
let result = Query::parse(Some("@host@"), false, 79);
assert_eq!(
result,
Err(QueryError::EmptyHostname {
input: "@host@".to_string()
})
);
}
```
- [ ] **Step 4: Update main.rs to handle Result**
In `src/main.rs`, change line 38 from:
```rust
let query = Query::parse(cli.query.as_deref(), cli.long, cli.port);
```
to:
```rust
let query = match Query::parse(cli.query.as_deref(), cli.long, cli.port) {
Ok(q) => q,
Err(e) => {
eprintln!("digit: {}", e);
return ExitCode::FAILURE;
}
};
```
Also update the import on line 7 from:
```rust
use digit::query::{Query, DEFAULT_PORT};
```
to:
```rust
use digit::query::{Query, QueryError, DEFAULT_PORT};
```
(Note: `QueryError` may not be directly referenced by name after the match -- if clippy warns about unused import, remove it. The match uses `Err(e)` which works without naming the type.)
- [ ] **Step 5: Update protocol.rs tests to unwrap**
In `src/protocol.rs` test module, update every `Query::parse(...)` call to `Query::parse(...).unwrap()`. There are 10 tests with parse calls. Each line like:
```rust
let q = Query::parse(Some("user@host"), false, 79);
```
becomes:
```rust
let q = Query::parse(Some("user@host"), false, 79).unwrap();
```
- [ ] **Step 6: Update integration tests to expect**
In `tests/integration.rs`, update every `Query::parse(...)` call to `Query::parse(...).expect("valid query")`. There are 5 tests with parse calls. Each line like:
```rust
let q = Query::parse(Some(&format!("user@127.0.0.1")), false, port);
```
becomes:
```rust
let q = Query::parse(Some(&format!("user@127.0.0.1")), false, port).expect("valid query");
```
- [ ] **Step 7: Verify new tests fail, old tests pass**
Run: `cargo test --lib query`
Expected: The 12 original tests PASS (they unwrap Ok values). The 5 new tests FAIL because `parse` returns `Ok(...)` for all inputs -- the `assert_eq!` against `Err(...)` fails.
- [ ] **Step 8: Commit**
```bash
git add src/query.rs src/main.rs src/protocol.rs tests/integration.rs
git commit -m "test: add query validation error tests and update parse API to Result (red)"
```
---
### Task 2: Query Validation -- Implementation (Green)
**Files:**
- Modify: `src/query.rs`
- [ ] **Step 1: Add validation to parse**
In `src/query.rs`, in the `parse` method, after the hosts vector is built (line 63 currently: `let hosts: Vec<String> = parts[1].split('@').map(|s| s.to_string()).collect();`), add validation before the final `Ok(Query {`:
```rust
let hosts: Vec<String> = parts[1].split('@').map(|s| s.to_string()).collect();
// Validate: no empty hostnames.
if hosts.iter().any(|h| h.is_empty()) {
return Err(QueryError::EmptyHostname {
input: input.to_string(),
});
}
```
- [ ] **Step 2: Run all tests**
Run: `cargo test`
Expected: All tests pass -- the 12 original query tests, 5 new error tests, 10 protocol tests, and 5 integration tests (32 total).
- [ ] **Step 3: Commit**
```bash
git add src/query.rs
git commit -m "feat: validate query input rejects empty hostnames (green)"
```
---
### Task 3: Timeout Consistency -- Test (Red)
**Files:**
- Modify: `tests/integration.rs`
- [ ] **Step 1: Add a read-timeout integration test**
Add this test to `tests/integration.rs`:
```rust
#[test]
fn read_timeout_reports_timed_out() {
// Server accepts connection and reads the query, but never responds.
let listener = TcpListener::bind("127.0.0.1:0").expect("bind to ephemeral port");
let port = listener.local_addr().unwrap().port();
let handle = thread::spawn(move || {
let (mut stream, _) = listener.accept().expect("accept connection");
stream.set_read_timeout(Some(Duration::from_secs(5))).ok();
// Read the query so the client's write succeeds.
let mut buf = [0u8; 1024];
let _ = stream.read(&mut buf);
// Sleep longer than the client's timeout, then drop.
thread::sleep(Duration::from_secs(5));
});
let q =
Query::parse(Some(&format!("user@127.0.0.1")), false, port).expect("valid query");
let result = finger(&q, Duration::from_secs(1));
assert!(result.is_err());
let err = result.unwrap_err();
let msg = format!("{}", err);
assert!(
msg.contains("timed out"),
"expected 'timed out' but got: {}",
msg
);
handle.join().expect("server thread");
}
```
- [ ] **Step 2: Run the test to verify it fails**
Run: `cargo test --test integration read_timeout_reports_timed_out`
Expected: FAIL -- the error message contains "failed to read response" instead of "timed out".
- [ ] **Step 3: Commit**
```bash
git add tests/integration.rs
git commit -m "test: add read timeout integration test (red)"
```
---
### Task 4: Timeout Consistency -- Implementation (Green)
**Files:**
- Modify: `src/protocol.rs`
- [ ] **Step 1: Add TimedOut check to write_all error mapper**
In `src/protocol.rs`, change the `write_all` error mapper (currently around line 123-125) from:
```rust
stream
.write_all(query_string.as_bytes())
.map_err(|e| FingerError::SendFailed { source: e })?;
```
to:
```rust
stream
.write_all(query_string.as_bytes())
.map_err(|e| {
if e.kind() == io::ErrorKind::TimedOut {
FingerError::Timeout {
host: host.to_string(),
port: query.port,
}
} else {
FingerError::SendFailed { source: e }
}
})?;
```
- [ ] **Step 2: Add TimedOut check to read_to_end error mapper**
Change the `read_to_end` error mapper (currently around line 129-131) from:
```rust
stream
.read_to_end(&mut buf)
.map_err(|e| FingerError::ReadFailed { source: e })?;
```
to:
```rust
stream
.read_to_end(&mut buf)
.map_err(|e| {
if e.kind() == io::ErrorKind::TimedOut {
FingerError::Timeout {
host: host.to_string(),
port: query.port,
}
} else {
FingerError::ReadFailed { source: e }
}
})?;
```
- [ ] **Step 3: Run all tests**
Run: `cargo test`
Expected: All tests pass, including the new `read_timeout_reports_timed_out` test.
- [ ] **Step 4: Commit**
```bash
git add src/protocol.rs
git commit -m "fix: map post-connection timeouts to FingerError::Timeout (green)"
```
---
### Task 5: Parallel DNS Connection
**Files:**
- Modify: `src/protocol.rs`
- [ ] **Step 1: Add thread and mpsc imports**
At the top of `src/protocol.rs`, add to the existing imports:
```rust
use std::sync::mpsc;
use std::thread;
```
The full import block should now be:
```rust
use std::io::{self, Read, Write};
use std::net::{TcpStream, ToSocketAddrs};
use std::sync::mpsc;
use std::thread;
use std::time::Duration;
```
- [ ] **Step 2: Extract a connect_to_addr helper**
Add this function before the `finger` function:
```rust
/// Attempt to connect to a single socket address with a timeout.
/// Returns the mapped error with host/port context on failure.
fn connect_to_addr(
addr: std::net::SocketAddr,
host: &str,
port: u16,
timeout: Duration,
) -> Result<TcpStream, FingerError> {
TcpStream::connect_timeout(&addr, timeout).map_err(|e| {
if e.kind() == io::ErrorKind::TimedOut {
FingerError::Timeout {
host: host.to_string(),
port,
}
} else {
FingerError::ConnectionFailed {
host: host.to_string(),
port,
source: e,
}
}
})
}
```
- [ ] **Step 3: Rewrite the connection section of finger()**
Replace the entire connection section of the `finger` function -- from the `// Resolve hostname` comment through the `stream.set_write_timeout` line (currently lines 88-119) -- with:
```rust
// Resolve hostname to socket addresses.
let addrs: Vec<std::net::SocketAddr> = addr_str
.to_socket_addrs()
.map_err(|e| FingerError::DnsResolution {
host: host.to_string(),
source: e,
})?
.collect();
if addrs.is_empty() {
return Err(FingerError::DnsResolution {
host: host.to_string(),
source: io::Error::new(io::ErrorKind::NotFound, "no addresses found"),
});
}
// Connect: single address uses direct call, multiple addresses race in parallel.
let mut stream = if addrs.len() == 1 {
connect_to_addr(addrs[0], host, query.port, timeout)?
} else {
let (tx, rx) = mpsc::channel();
let addr_count = addrs.len();
for addr in addrs {
let tx = tx.clone();
let host = host.to_string();
let port = query.port;
thread::spawn(move || {
let result = TcpStream::connect_timeout(&addr, timeout);
let _ = tx.send(result);
// Ignore send error -- receiver may have moved on.
});
}
// Drop our copy so the channel closes when all threads finish.
drop(tx);
let mut last_err = None;
for _ in 0..addr_count {
match rx.recv() {
Ok(Ok(s)) => {
// First successful connection wins.
// Set timeouts and return.
s.set_read_timeout(Some(timeout)).ok();
s.set_write_timeout(Some(timeout)).ok();
// Send the query.
let query_string = build_query_string(query);
let mut stream = s;
stream
.write_all(query_string.as_bytes())
.map_err(|e| {
if e.kind() == io::ErrorKind::TimedOut {
FingerError::Timeout {
host: host.to_string(),
port: query.port,
}
} else {
FingerError::SendFailed { source: e }
}
})?;
// Read the full response.
let mut buf = Vec::new();
stream
.read_to_end(&mut buf)
.map_err(|e| {
if e.kind() == io::ErrorKind::TimedOut {
FingerError::Timeout {
host: host.to_string(),
port: query.port,
}
} else {
FingerError::ReadFailed { source: e }
}
})?;
return Ok(String::from_utf8_lossy(&buf).into_owned());
}
Ok(Err(e)) => {
last_err = Some(e);
}
Err(_) => break, // Channel closed, all threads done.
}
}
// All addresses failed.
let e = last_err.unwrap_or_else(|| {
io::Error::new(io::ErrorKind::ConnectionRefused, "all addresses failed")
});
if e.kind() == io::ErrorKind::TimedOut {
return Err(FingerError::Timeout {
host: host.to_string(),
port: query.port,
});
} else {
return Err(FingerError::ConnectionFailed {
host: host.to_string(),
port: query.port,
source: e,
});
}
};
// Set read/write timeouts on the connected socket.
stream.set_read_timeout(Some(timeout)).ok();
stream.set_write_timeout(Some(timeout)).ok();
```
Note: The rest of the function (send query, read response, return) remains unchanged for the single-address path. The multi-address path handles send/read/return inline because it takes ownership of the winning stream inside the match arm.
- [ ] **Step 4: Run all tests**
Run: `cargo test`
Expected: All tests pass (single-address path exercised by all existing tests).
- [ ] **Step 5: Run clippy**
Run: `cargo clippy -- -D warnings`
Expected: No warnings.
- [ ] **Step 6: Commit**
```bash
git add src/protocol.rs
git commit -m "feat: try all DNS-resolved addresses in parallel"
```
---
### Task 6: Version Bump and Final Checks
**Files:**
- Modify: `Cargo.toml`
- [ ] **Step 1: Bump version to 0.2.0**
In `Cargo.toml`, change:
```toml
version = "0.1.0"
```
to:
```toml
version = "0.2.0"
```
- [ ] **Step 2: Run full test suite**
Run: `cargo test`
Expected: All tests pass.
- [ ] **Step 3: Run clippy**
Run: `cargo clippy -- -D warnings`
Expected: No warnings.
- [ ] **Step 4: Check formatting**
Run: `cargo fmt -- --check`
Expected: No formatting issues. If any, run `cargo fmt` to fix.
- [ ] **Step 5: Commit**
```bash
git add Cargo.toml
git commit -m "chore: bump version to 0.2.0"
```