DEV Community

Cover image for What I learned building wfreq
Yash Kumar Saini
Yash Kumar Saini Subscriber

Posted on

What I learned building wfreq

#rust #cli #buildinpublic #discuss

TL;DR

How a word-frequency tool taught me to wrap errors instead of replacing them: from .expect() to thiserror, #[source] and per-file warnings.

wfreq running in the terminal

I built wfreq, a small Rust CLI that prints the most common words in .txt, .md, .pdf and .docx files. The counting took an afternoon. The error messages took the rest of the project.

This post is about the hardest part: making the tool tell the truth when something goes wrong.

Version 1: .expect() and a crash

The first main read the file like this:

let content = std::fs::read_to_string(&args.file).expect("File can not read");
Enter fullscreen mode Exit fullscreen mode

I pointed it at a PDF and got:

thread 'main' panicked at src/main.rs:30:57:
File can not read: Error { kind: InvalidData, message: "stream did not contain valid UTF-8" }
Enter fullscreen mode Exit fullscreen mode

Two problems. A panic is a crash with a backtrace hint, not a message for a user. And read_to_string only accepts valid UTF-8, so every PDF "can not be read", even though the file is right there.

Version 2: ? and an error message nobody asked for

The textbook fix is to return Result from main and use ?. I added a custom error enum with thiserror and nice messages:

#[derive(Debug, Error)]
pub enum CliError {
    #[error("File not found or cannot read: {0}")]
    FileReadError(PathBuf),
    // ...
}

fn main() -> Result<(), Box<dyn Error>> {
    let content = parser::extract_text(&args.file)?;
    // ...
}
Enter fullscreen mode Exit fullscreen mode

Then I ran it on a missing file:

$ wfreq nope.txt
Error: FileReadError("nope.txt")
Enter fullscreen mode Exit fullscreen mode

My message was nowhere. When main returns Err, Rust prints the error with Debug, not Display, so all the #[error("...")] text is ignored.

The fix is small: keep ? everywhere, but handle the error once, at the top.

fn main() {
    if let Err(e) = run() {
        eprintln!("error: {e}");   // Display: the #[error] text
        std::process::exit(1);
    }
}

fn run() -> Result<(), Box<dyn Error>> {
    // all the real work, using ? freely
}
Enter fullscreen mode Exit fullscreen mode

exit(1) lives in main, after run() has returned and dropped everything, so nothing is skipped. And eprintln! sends the message to stderr, so wfreq notes.txt > top.txt never puts an error into the output file.

Version 3: the lie

Now errors looked nice:

$ wfreq nope.txt
error: File not found or cannot read: nope.txt
Enter fullscreen mode Exit fullscreen mode

Then I tried a file that did exist but was not valid UTF-8. Same message: "File not found or cannot read". The file was right there. The real reason, invalid UTF-8, was gone.

The culprit was one character in the conversion:

std::fs::read_to_string(path)
    .map_err(|_| CliError::FileReadError(path.to_path_buf()))
Enter fullscreen mode Exit fullscreen mode

|_| throws the io::Error away. "Not found", "permission denied", "is a directory" and "not UTF-8" all became the same vague sentence.

The rule I took from this: when you convert an error, wrap it, don't replace it. Your error adds the context (which file), and the original error keeps the cause (why).

#[derive(Debug, Error)]
pub enum CliError {
    #[error("cannot read {}: {source}", .path.display())]
    FileRead {
        path: PathBuf,
        #[source]
        source: std::io::Error,
    },
    // ...
}

std::fs::read_to_string(path).map_err(|source| CliError::FileRead {
    path: path.to_path_buf(),
    source,
})
Enter fullscreen mode Exit fullscreen mode

Two details worth knowing:

  • PathBuf has no Display, because a path is not guaranteed to be valid text. So the message uses {} with .path.display() as an extra argument. The leading . refers to the variant's field.
  • #[source] makes thiserror implement Error::source(), so tools that print error chains can follow it to the io::Error.

Now the same file says what is actually wrong:

error: cannot read bad.txt: stream did not contain valid UTF-8
error: cannot read nope.txt: No such file or directory (os error 2)
Enter fullscreen mode Exit fullscreen mode

Version 4: the bug that compiled

When I added PDF and DOCX support, the parser picked a function by file extension. One version looked like this:

match ext.as_str() {
    "txt" | "md" => {
        txt::parse(file_path);
        Ok("Parser success".to_string())
    }
    "docx" => docx::parse(file_path),
    // ...
}
Enter fullscreen mode Exit fullscreen mode

This type-checks (once a missing use std::fs; was added). Every arm returns Result<String, CliError>. But the ; after txt::parse(file_path) throws away the parsed text and any error. Every .txt file now "contained" the two words parser and success, and a broken file reported success.

The compiler can't catch this: the types are right, the values are wrong. A test can:

#[test]
fn reads_text_from_stdin() {
    let out = run_with_stdin(&["-", "-n", "1"], "to be or not to be");
    assert_eq!(stdout(&out), "Path: -\n 1. be              2\n");
}
Enter fullscreen mode Exit fullscreen mode

Each arm should just be the parser call, so its result (text or error) is returned as-is:

"txt" | "md" => txt::parse(file_path),
"pdf" => pdf::parse(file_path),
"docx" => docx::parse(file_path),
Enter fullscreen mode Exit fullscreen mode

Version 5: one bad file in a folder

Then the tool learned to read folders. A folder can hold a broken PDF next to twenty good text files. Should one bad file stop the whole run?

I decided:

  • One file given: any problem is an error, exit code 1.
  • Several files or a folder: a bad file is a warning on stderr and is skipped.
  • Every file failed: that is an error too, because there is nothing to count. 
for path in &files {
    match parser::extract_text(path) {
        Ok(text) => merge_counts(&mut counts, count_words(&text, args.case_sensitive)),
        Err(e) if files.len() > 1 => eprintln!("warning: {e} (skipped)"),
        Err(e) => return Err(e.into()),
    }
}
Enter fullscreen mode Exit fullscreen mode

This only works because of version 3. The warning is useful because the error already carries the file and the reason:

warning: cannot parse docs/old.pdf: PDF error: couldn't parse input: invalid file header (skipped)
Enter fullscreen mode Exit fullscreen mode

Without the path in the error, that line would say "cannot parse: invalid file header", and you would have to guess which of the twenty files it means.

What I'd tell myself at the start

  1. .expect() is for "this cannot happen", not for user input.
  2. Returning Result from main prints Debug. Use a run() function and print the error yourself.
  3. map_err(|_| ...) deletes information you can't get back. Wrap the error with #[source] instead.
  4. Put the file path in the error, not in the caller. You will need it as soon as there is more than one file.
  5. A match arm with a ; and a hard-coded Ok(...) compiles fine. Write an end-to-end test that checks real output.

The integration tests in tests/integration.rs check every error message mentioned here, so none of these bugs can come back quietly.

full crash → crates.io story: https://medium.com/@yash_k_saini/from-a-crash-to-crates-io-shipping-a-rust-cli-tool-f6ed59c16a9f
repo: https://github.com/yashksaini-coder/wfreq
crate: https://crates.io/crates/wfreq

Top comments (1)

Collapse
 
sinarezaei profile image
Sina Rezaei •

The per-file warning approach is a good call. I’d be curious how you’d handle this if wfreq were running in CI over thousands of documents: say one PDF is corrupted halfway through the batch, but the rest are valid. Keeping the original error chain makes debugging much easier, but I’d also want the final exit status to distinguish “some files were skipped” from “the whole run failed.” Otherwise, a pipeline could publish incomplete frequency data and still report success. That’s where error handling starts becoming part of the CLI’s contract, not just nicer error messages.