Simple TCP client in Rust

Question

Here's a little command-line utility written in Rust that connects to a Denon AV receiver and sends a command, then receives the response. It's a simple TCP client that takes its command from the first argument.

extern crate encoding;

use std::env;
use std::io::prelude::*;
use std::net::TcpStream;
use encoding::{Encoding, EncoderTrap};
use encoding::all::ASCII;

const HOST: &'static str = "10.21.39.30:23";

fn main() {
    let command = match env::args().nth(1) {
        Some(cmd) => cmd,
        None => {
            let my_name = env::args().nth(0).unwrap();
            panic!("Usage: {} [command]", my_name)
        }
    };
    let mut command_bytes = ASCII.encode(&command, EncoderTrap::Strict).unwrap();
    command_bytes.push('\r' as u8);
    let mut stream = TcpStream::connect(HOST).unwrap();
    stream.write_all(&command_bytes).unwrap();
    let mut response = String::new();
    stream.read_to_string(&mut response).unwrap();
    println!("{}", response);
}

Is there a better way to handle this stuff? My questions are mostly:

• Is there an alternative to all of these unwrap() calls?
• It seems better (to me) to add the '\r' to the end of the command, making command_bytes mutable, than to call stream.write() another time. Think so?
• The response will be ASCII, so stream.read_to_string() seems appropriate. It also seems like a bad idea for a network client to read with no size limits. Any good way to deal with this?
• Any other structural stuff here that is glaringly bad? I'm starting to understand coding in Rust, I think, but haven't yet developed my own style. Early corrections would be greatly appreciated.

Answer 1

Your code looks pretty great to me, especially for the current size of it. As it grows to do more things, you will probably want to start restructuring things a bit, so I'm looking at from that direction. It also depends a bit on your audience — if this is just a tool for you, then unwrapping is just fine as you will understand what the error message means and how to fix it.

Is there an alternative to all of these unwrap() calls?

Absolutely. Check out The Rust Programming Language's section on error handling. In this case, it's probably simplest to use a Box<Error>, but in a larger program or a library you may want to investigate implementing your own error type. I almost put that version here, but it was so long that it would have been really inappropriate given the current size :-)

It seems better (to me) to add the '\r' to the end of the command, making command_bytes mutable, than to call stream.write() another time. Think so?

I don't think there's going to be a major performance difference between the two, or at least it could be made negligible. When you call write_all, it may actually make multiple write calls. In a way, you are implementing a simple BufWriter. I think it mostly comes down to semantics. In this case, I do like that you add the \r to the command packet, as it allows that logic to all be in one tidy spot.

It also seems like a bad idea for a network client to read with no size limits. Any good way to deal with this?

You can use take, which will limit the number of bytes read. The amount you use is the tricky part. Are you guaranteed to always get responses less than some size? Maybe the protocol says that you will always get 4 bytes that then tell you how many bytes follow? In the example below, I just assumed a maximum of 1 KiB.

---

Following the error handling guides, I extracted the majority of your logic to a new function that uses Result. That way, main simply has to decide what to do on success or failure. This also means that you get nicer error messages like this:

An error occurred: Unable to transmit: Network is unreachable (os error 51)

I also added a little bit of space between logically distinct parts of the method. They are a little small to be broken out for now, but they could easily lead to new methods in the future.

Some thoughts for the future:

1. It might be a good idea to parameterize the host value to the send_one_command method. You can then use the constant from within main.

2. Will you ever need to have multiple back-and-forth transmissions? If so, you may want to create a struct that holds the connection and keeps it open. Then you ask it to send a command.

3. The remaining unwrap in main is tricky. It seems very unlikely that the program name could be missing, so unwrap seems to be a good fit as it's a case you simply can't handle well. On the other hand, errors hiding other errors can be annoying to debug. You may want to use something like .unwrap_or_else(|| "unknown program name".into()).

extern crate encoding;

use std::{env, error};
use std::io::prelude::*;
use std::net::TcpStream;
use encoding::{Encoding, EncoderTrap};
use encoding::all::ASCII;

const HOST: &'static str = "10.21.39.30:23";

fn send_one_command(command: &str) -> Result<String, Box<error::Error + Send + Sync>> {
    let mut command_bytes = try!(ASCII.encode(command, EncoderTrap::Strict).map_err(|x| x.into_owned()));
    command_bytes.push('\r' as u8);

    let mut stream = try!(TcpStream::connect(HOST));
    try!(stream.write_all(&command_bytes));

    let mut response = String::new();
    let mut limited = stream.take(1024);
    try!(limited.read_to_string(&mut response));

    Ok(response)
}

fn main() {
    let command = match env::args().nth(1) {
        Some(cmd) => cmd,
        None => {
            let my_name = env::args().nth(0).unwrap();
            panic!("Usage: {} [command]", my_name)
        }
    };

    match send_one_command(&command) {
        Ok(response) => println!("{}", response),
        Err(err) => println!("An error occurred: {}", err),
    }
}