Code review for web application

1

i would like to know how can i make this code more fault tolerant
and how to make it more idiomatic rust

2

Well the first thing I notice is that you are not very consistent with your indentation and whitespace. For example, look at this:

let pool_size : usize = match env::var("THREAD_POOL_SIZE_FOR_EACH_ROUTE"){
    // value of 'THREAD_POOL_SIZE_FOR_EACH_ROUTE' is returned as string that needs to 
    // be parsed into usize ,in case of errors we are using default values
    Ok(var)=> match var.parse(){
        Ok(val) => val,
        Err(_err) => {
            println!("> Parse Error :{}'THREAD_POOL_SIZE_FOR_EACH_ROUTE' can only have unsigned integer Value",_err);
            println!("> using default value for THREAD_POOL_SIZE_FOR_EACH_ROUTE");
            2
        }
     },
    Err(_s)=>2
};

Sometimes you have spaces around =>, sometimes you have one space, but not the other. The second-to-last } is indented by one space too much. Your { do not have spaces before them. The comment too: You have the space before the comma instead of after. Fixing this gets us:

let pool_size: usize = match env::var("THREAD_POOL_SIZE_FOR_EACH_ROUTE") {
    // value of 'THREAD_POOL_SIZE_FOR_EACH_ROUTE' is returned as string that needs to 
    // be parsed into usize
    Ok(var) => match var.parse() {
        Ok(val) => val,
        Err(err) => {
            println!("> Parse Error :{}'THREAD_POOL_SIZE_FOR_EACH_ROUTE' can only have unsigned integer Value", err);
            println!("> using default value for THREAD_POOL_SIZE_FOR_EACH_ROUTE");
            2
        }
    },
    Err(_) => 2
};

I recommend trying out rustfmt. I wont comment further on indentation and spaces.

Note that Result and Option has convenience methods that lets you write this more succinctly:

let pool_size: usize = env::var("THREAD_POOL_SIZE_FOR_EACH_ROUTE")
    .map(|s| s.parse().expect("THREAD_POOL_SIZE_FOR_EACH_ROUTE given but not integer."))
    .unwrap_or(2);

The next thing I notice is that 0.0.0.0:7878 is not a port. The port is 7878. I wouldn't print an error message when using the default. I would probably just print Listening on blabla:port regardless of whether the default was chosen or not. Again, you can use convenience methods:

let port = env::var("RUST_SERVER_PORT").unwrap_or_else(|| "0.0.0.0:7878".to_string())

Here we are using the unwrap_or_else method, because to_string is a bit expensive, and there's no reason to call it if we don't need it.

You should also know the expect method, which is like unwrap, but with custom error message:

let listner  = TcpListener::bind(&port).expect("Binding failure");

Note that expect includes the error description in the error message, so you do not need to explicitly print that too.

Why are you using byte-strings here?

let home_route = String::from_utf8_lossy(b"/").to_string();
let route_2 = String::from_utf8_lossy(b"/r2").to_string();
let route_3 = String::from_utf8_lossy(b"/r3").to_string();

Just do:

let home_route = "/".to_string();
let route_2 = "/r2".to_string();
let route_3 = "/r3".to_string();

As for thread pools, I would not have a pool for every route. Just use a single pool for everything.

Now we get to the parts where you want to avoid panics:

let mut stream = match stream {
    Ok(val) => val,
    Err(_err) => {panic!("> Failed at Unwrapping Stream :{}",_err)}
};

I would recommend printing an error and using continue to just move on. Otherwise I can crash your web server by sending you something that makes you error.

Note that stream.write(b"Error 404") is not a valid http response. I haven't looked at the other files.

2 Likes
4

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.