Trying to implement proper error handling, fix an error, and return a temp file

help
1

Hey all,

So with the things I've learned so far regarding .unwrap() and env::current_dir(), I've been attempting to rewrite one of my functions. You can see in the comment at the bottom of the function what my old code was.

fn write_arti_bin(){
    let arti_bin = include_bytes!("C:\\Code\\spiderweb\\src\\bins\\arti.exe");
    let exe_file = NamedTempFile::with_suffix_in("exe", "./");
    
    if exe_file.is_ok(){
        let arti_bin_result = fs::write(exe_file.unwrap().path(), arti_bin);

        if arti_bin_result.is_ok(){
            println!("Arti Binary was written sucessfully.");
            println!("Path: {:?}",exe_file.unwrap().path());
        }
        else {
            println!("Arti binary failed to write. Error: {:#?}", arti_bin_result.err());
        }
    }else {
        println!("Failed to create temprorary file. Error: {:#?}", exe_file.err())
    }

    /*
    let filename= "arti.exe";
    let cur_dir = env::current_dir().unwrap();
    let full_path = PathBuf::from(cur_dir).join(filename);

    fs::write(full_path, arti_bin).expect("Unable to write file.");
    */
}

So I have a few questions:

  1. Is it appropriate to use .unwrap() in this line? My thought is that we've already gotten through declaring the temp file by checking it in the if statement right? Is there a better way of accessing the NamedTempFile part of the Result<NamedTempFile,Error> without unwrap()?
[...]
if exe_file.is_ok(){
    let arti_bin_result = fs::write(exe_file.unwrap().path(), arti_bin);
[...]
  1. I'm getting an error attempting to write out the path of the tempfile with println!().
println!("Path: {:?}",exe_file.unwrap().path());

This is erroring with: Use of moved value: exe_file. Is that because I already "used" the exe_file object in the if statement? I tried making the println a reference to exe_file but I still received the moved value error.

  1. Lastly, is there a better way of handling the errors instead of using a bunch of if statements? I think the answer to this one might be to have this function return a Result<(),Error> to the calling line in main.rs and use ? to bubble up the errors but I'm not sure. Actually, now that I think about it, I'll need to return the NamedTempFile too right?

I have another function that happens after this write function that then builds a command to execute this binary...so I would need to return my temp file somehow so that function can act on it right?

2

You are unwrapping the same result multiple times. You want to do it just once.
There's only i/o errors in there, so it's easy to just resurn an io::Result and maybe better. However, there are also different ways to write this using match or if let.

    if let Ok(exe_file) = NamedTempFile::with_suffix_in("exe", "./") {
        // ...
    } else {
        eprintln!("Failed to create temprorary file")
    }

Or, the better way, if you want access to both the result and error.

    let exe_file_result = NamedTempFile::with_suffix_in("exe", "./");
    match exe_file_result {
        Ok(exe_file) => {
            // ...
        },
        Err(e) => {
            eprintln!("Failed to create temprorary file. Error: {:#?}", e)
        }
    }

You can also do an else if let Err(e) = ... but it's not the best.
Another good tool is unwrap_or_else, but here you have to return or crash. You're function cannot continue.

If you are unsure if you want to unwrap or return an error, put yourself in the shoes of someone using your crate.
Do you want their app to crash, or do you want them to see an error that they can conditionally handle? And also consider if you want your function to continue or not. Choose if and when you want to return.
Crashing can be valid, if the alternative is not desirable.

Lastly, is there a better way of handling the errors instead of using a bunch of if statements?

There's nothing wrong with it, if the flexibility is important to you. Even with results, sometimes you want more that a simple ?. Like this:

let Ok(value) = result else {
    // Do something else here.
    return Err(...)
}

As I mentioned, maybe std::io::Result<NamedTempFile> is a better option here since you want to create the tempfile and return it like this:

fn write_arti_bin() -> std::io::Result<NamedTempFile> {
    let arti_bin = include_bytes!("path/to/file");
    let exe_file = NamedTempFile::with_suffix_in("exe", "./")?;
    std::fs::write(exe_file.path(), arti_bin)?;
    return Ok(exe_file)
}

On my last edit, Maybe not super important in what you are creating, but maybe you should reconsider your use of include_bytes here. Not sure if it's the right tool for the job.

1 Like
3

You are unwrapping the same result multiple times.

Starting from the top, something I'm having trouble understanding. You say that I'm unwrapping the same result multiple times. Can you elaborate on how?

The way I thought I was doing it was that exe_file was storing the result of creating the temp file object. Reserving it in memory, creating the actual file (so taking action on the file system), then storing the result in the exe_file variable. So whether or not the temp file was successfully created.

Then I want to write data into that temp file. So I create the arti_bin_result object and issue the command to write the bytes to it and store the result of that in arti_bin_result.

Is this where you're saying I'm attempting to unwrap the same result multiple times?

Or wait...maybe it's this. Because I already unwrapped exe_file and assigned the result to arti_bin_result, that changes the...state for lack of a better word, of exe_file? So when I try to print the path of the exe_file I've already unwrapped it so I can't unwrap it again...

If my goal was just to output the full path to the named temp file, to include the name of the temp file itself, can I do that from this part of the code?

Or, the better way, if you want access to both the result and error.

I like that...that feels much nicer than the nested ifs I have.

As I mentioned, maybe std::io::Result<NamedTempFile> is a better option here since you want to create the tempfile and return it like this:

Thank you for that code, that's much cleaner than what I have. In this case, you're returning the result of writing the file right? If I called this function from my Command function, would I have access to the binary with this return? In other words, if I had this in my main.rs:

let arti_exe = write_arti_bin();

Would I be able to execute that arti_exe as if it was binary and pass args to it and everything or is the Result return type just literally a state of the function but not an actionable object if that makes sense?

Maybe not super important in what you are creating, but maybe you should reconsider your use of include_bytes here. Not sure if it's the right tool for the job.

So Arti.exe is an external binary that proxy's HTTP traffic. I use reqwest to send my traffic through it and since I'm so new to Rust, I thought it would be easier to wrap my head around dropping a file to disk, using it, then cleaning it up as opposed to try to do it all in memory.

4

Or wait...maybe it's this. Because I already unwrapped exe_file and assigned the result to arti_bin_result , that changes the...state for lack of a better word, of exe_file ? So when I try to print the path of the exe_file I've already unwrapped it so I can't unwrap it again...

Unwrapping doesn't just peek into the result, it consumes it. It doesn't exist anymore after you call unwrap. You either get the Ok or Err from it.

In this case, you're returning the result of writing the file right?

let arti_exe = write_arti_bin();

It returns either the tempfile or the error in case something went wrong. You would have to handle the result here too. Again, it depends how you want to handle it. By using write_arti_bin()? (which means you also need to return std::io::Result<()> from where you call this) or one of the other ways I mentioned.

So Arti.exe is an external binary that proxy's HTTP traffic. I use reqwest to send my traffic through it and since I'm so new to Rust, I thought it would be easier to wrap my head around dropping a file to disk, using it, then cleaning it up as opposed to try to do it all in memory.

I mentioned that because include_bytes embeds the file into your binary with a static lifetime. This is useful sometimes, especially if you want to ship the crate with it, or if you write this temporary file a lot, but usually it's cleaner to just read the file, and write the contents. It's probably not super important for you as long as it works either way.

1 Like
5

You can improve the code a bit:

use std::io::{self, Seek, Write};
use tempfile::NamedTempFile;

const ARTI_BIN: &[u8] = include_bytes!("C:\\Code\\spiderweb\\src\\bins\\arti.exe");

fn write_exe_cwd(bytes: &[u8]) -> io::Result<NamedTempFile> {
    let mut file = NamedTempFile::with_suffix_in(".exe", "./")?;
    file.write(bytes)?;
    file.rewind()?;
    Ok(file)
}

fn write_arti_bin() -> io::Result<NamedTempFile> {
    write_exe_cwd(ARTI_BIN)
}
  • Static bytes are put into a const.
  • write_exe_cwd() can be reused to write arbitrary bytes to a temporary file within the current working directory, if needed.
  • write_exe_cwd() only uses one file object and does not open a second file handle for writing the data.
  • Also note the leading "." in the file suffix. It won't be automatically included.
1 Like
6

Interesting. I have a few further questions for your code:

  1. Why the rewind()? Once we write the file, does Rust discard the bytes or something so we can't write it a second time? No...that doesn't make sense because we've saved it in a const outside the scope of the function. So...we seek back to the beginning of the temp file...then return the file?
  2. This might be a bit technical for me but I'm curious...where is the difference where we're opening two handles to the file in my code vs one in yours?
  3. Thanks for pointing out the suffix typo, I definitely missed that.

Addendum EDIT:
Does the write_exe_cwd wait for the file to finish writing and rewinding? Logically I would think it does but I'm getting an error that the file is in use by another process when I try to spawn it and I'm not entirely sure which part is in use. This is how I'm trying to call the temp file. This worked prior to changing it over to a Temp file, back when my code was just writing to the current directory.

let arti_bin = write_arti_bin();
match arti_bin {
    Ok(arti_bin) => {
        let mut tor_proxy = Command::new(arti_bin.path())
            .arg("proxy")
            .stdout(Stdio::piped())
            .stderr(Stdio::piped())
            .spawn()
            .expect("Arti couldn't be started...");
[...]

The code isn't getting past the .expect and crashing with this error:

Arti couldn't be started...: Os { code: 32, kind: Uncategorized, message: "The process cannot access the file because it is being used by another process." }

I had to add a bit to the write_arti_bin() function you had in your post since I needed to return the file to the command spawning function. So this is what I changed:

fn write_arti_bin() -> io::Result<NamedTempFile>{
    let arti_bin = write_temp_file(ARTI_BYTES)?;
    Ok(arti_bin)
}
7
  1. Exaclty. We rewind the file to allow to read the file from beginning before we return it. Else a subsequent read of the file object would start at the end of the file and thus would not read anything except EOF. Currently, this is not an issue, because your code does not actually seem to read the content of the file object, but it prevents unexpected behavior in the future.
  2. Some operating systems, such as Windows, cannot have multiple file handles to a file when it is also open in write mode to prevent data races. Other OSes, such as unixoid ones allow this and handle potential data races.
  3. Which is also the reason for the error you experience. The temporary file is still open in with write permissions due to the NamedTempFile not yet having been dropped when you try to invoke the subprocess. Hence, in the invoked subprocess, it cannot open the file. You'd need to close the temp file in Rust first. However, simply dropping the file would result in the NamedTempFile's file being deleted from the file system. Use keep() to prevent that from happening, and conveniently getting the path to it:
let (_fileobj, path) = write_arti_bin()?.keep()?;
1 Like
8

Man return types are going to drive me nuts. Every time I think I get it, I run into something that throws me for a loop.

So with the modification you listed for the '.keep()?', I tried implementing it.

fn write_arti_bin() -> io::Result<NamedTempFile>{
    let arti_bin = write_temp_file(ARTI_BYTES)?.keep()?;
    Ok(arti_bin)
}

Now of course I'm not returning a Result<NamedTempFile> like I was before, now arti_bin is a (File, Pathbuff). Is that a tuple?

So now I need to match my write_arti_bin() return type to that right? The suggested return type when I start typing is: Result<(File, PathBuf), _>. But that errors with the _ saying that you're not allowed to do that here. So I remove it and I get a different error at the end of the function stating that it was expecting a comma.

So what's the proper return type here? I can't do Result<(File, Pathbuf),Error> either because then I get an error with the ?'s.

9

Yeah, the first result has an io::Error and the second one a PersistError instead. So you have multiple options:

  1. Just .expect() the results or early-return on an error with logging (recommended).
  2. Return an anyhow::Error from the using function, which wraps basically any other error type.
  3. Create your own error type (probably not worth the work for a beginner's program).
  4. Don't use .keep() but instead set disable_cleanup(true) on the temp file. This also prevents deleting the file on drop, but does not convert it to a non-temporary, i.e. regular file, which may cause funny behavior on Windows.
1 Like
10

Attempting to do your first option since you said that's the recommended one and this is the pertinent code:

fn write_temp_file (file_bytes: &[u8]) -> io::Result<NamedTempFile> {
    let mut exe_file = NamedTempFile::with_suffix_in(".exe","./")?;
    exe_file.write(file_bytes)?;
    exe_file.rewind()?;
    Ok(exe_file)
}

fn write_arti_bin() -> Result<(File,PathBuf),PersistError>{
    let arti_bin = write_temp_file(ARTI_BYTES).expect("Unable to write temp file.").keep()?;

    Ok(arti_bin)
}

The calling code snippet:

pub fn start_proxy() -> Result<Child,Error>{
    let arti_bin = write_arti_bin();
    match arti_bin {
        Ok(arti_bin) => {
            let mut tor_proxy = Command::new(arti_bin.1)
                .arg("proxy")
                .stdout(Stdio::piped())
                .stderr(Stdio::piped())
                .spawn()
                .expect("Arti couldn't be started...");
[...]

To be honest, I'm not sure why the write_arti_bin function is fine with PersistError and the ?. I swear I did that earlier and it yelled at me but whatever.

The program is still failing at the command though with the same error. I thought I was accessing the path correctly by doing arti_bin.1 to access the PathBuf but maybe not?

EDIT
Interestingly...I can now see the temp file getting written to the current directory so that's progress. It doesn't get removed right now of course so that means the keep is working too. Once the program crashes, if I go to that temp file with my regular terminal, I can execute it just fine.

So...something is still preventing the temp file from being released in windows...

11

Yeah, as long as the temporary file exists in Rust, it cannot be read by another process in Windows, since it's opened for writing by Rust. And once you close it, it will be deleted. And if you convert it to a normal file, it won't be deleted automatically.

So instead, you can create a temporary directory and create a regular file with a static name inside it. Then you can close that file in Rust after writing, but keep the handle on the temporary directory. Then you execute your subprocess and after it's done, you let the temporary directory be dropped, causing all of its contents, i.e. the exe file, to be deleted, too.

1 Like
12

Ahh ok...so even by doing .keep(), Windows still holds it open for writing. Then...is there a point to using temp files in Windows at all? If it holds it as writeable, what is the use of the files on Windows?

The temp directory sounds like it'll do what I'm looking for. It looks like the link you sent is for an external crate. Is the temp_dir function in the standard library not the way to go?

13

std::env::temp_dir() just returns the temporary directory of the system. What you need is a temporary directory inside this directory, which can be managed by the linked crate.
The system's temporary directory is basically the root directory for all temporary files and directories on the system and is managed by the operating system. It should never be modified or deleted by a user-level process.

1 Like
14

Ahhh ok...well oops lol. I did exactly that and I'm writing and deleting the binary from inside the temp_dir return...

I'll rewrite the code for the tempdir crate instead.

Question about it's behavior:

The TempDir type creates a directory on the file system that is deleted once it goes out of scope.

I assume this scope is the function that I call it in correct? This may not work for me then because I have one function that creates the temp dir, a different function that writes a file into that dir, a third function that then executes that binary as a command, and a fourth function that deletes the file. If the temp dir is cleaned up automatically then I won't need the deletion function but I'll still need the other functions to access it. Does returning the temp_dir object prevent it from going out of scope?

From trying to learn about scope in Rust it seems like by returning the value and assigning that return to a variable in a different function...the new variable takes ownership of the heap...the original object is destroyed but the resources have already been passed over so I can still use it?

As an example:

fn grab_value(){
    let other_num = create_value();
    println!(other_num);
}

fn create_value() -> i32{
    let num = 15;
    num
}

So in this example...num goes out of scope when create_value() is finished running right? But the value has been passed to other_num so I can still use it? If, instead of numbers this was the temp dir...would the temp dir be deleted after the function finishes?

15

In short, yes.

They mean when it gets destructed ("drops"). All locals are moved or dropped by the end of the function. Returning it to the caller is moving it into a different scope so it can last longer than the function.

You're getting the basic idea, though there's still some confusion, perhaps caused by whatever languages you know or the Book's over-emphasize on heap vs stack. I'll try to write a whirlwind introduction.

Ownership -- move semantics and destructors -- are about resource management. You have a value that manages some sort of resource, and when the value goes away, it needs to clean up or otherwise do something with that resource. Heap allocating types like String are a go-to example: they need to deallocate the heap memory when they go away. But there are many other types of resource management, such as File (which needs to close the file descriptor when it goes away) and TempDir (which aims to remove the temporary directory when it goes away). A type can manage a resource without using the heap.[1]

In Rust, values have a drop scope. I phrased things in terms of functions before, but inner blocks are also drop scopes, and statements (for temporaries), etc. I won't go into the gritty details, but the main idea here is that any values that haven't been moved or destructed when they reach the end of their drop scope get destructed at that point. This way, the resource management is a lot more automatic, even though Rust has no garbage collection: you don't have to remember to call the destructor yourself.

And as you're getting your head around, you can move values into some containing scope (such as returning out of the function) and the value will not drop. In the case of variables, the original variable isn't around anymore.

Move semantics means that when you assign one variable to another, the value is moved and the original variable becomes uninitialized -- you can't use it anymore.[2] It works this way because it is generally undefined behavior to destruct the same value more than once (free heap memory twice, close a File twice), and Rust doesn't want to implicitly clone values all the time -- it prefers to make such costs explicit.

Finally, some types have copy semantics -- they implement Copy. These types can never manage resources. In fact that is a defining property: all parts of the type have opted into never having a destructor.[3] With copy semantics, when you assign one variable to another, the value is copied and the original variable is still useable.

When a Copy variable goes out of scope, you can still think of the variable becoming uninitialized. You can't access it any more. But you can copy the value out of the scope -- for example return an i32 from a function -- similar to how you can move values out of the scope.

So in the example, the num variable (a Copy type) is inaccessible after create_value is finished running. But you can pass a copy of the value to the caller. There's no heap or "original object"[4] or destructors here, but in terms of keeping a value around, I think you have the idea.

You can't use the variable outside the function. In the example you pass a copy of it to the caller. If it was a TempDir you'd be moving the TempDir to the caller. In both cases, the caller can use it.

No, because you moved it to the caller. The TempDir would drop at the end of grab_value (as the functions are currently written).

  1. including on no_std, where there is no heap. ↩︎

  2. unless you reinitialize it ↩︎

  3. It is not about "stack only data" like the book says; you can manage resources purely on the stack. ↩︎

  4. unless you call a variable an object I guess ↩︎

1 Like
16

Such an amazing response, thank you! Yeah I'm still getting my head around this stuff. All my coding training comes from my game design degree, not software engineering. A lot of the languages I've learned previously have been ActionScript, Javascript, and most predominantly C# Even then, the game engine handled a lot of logic that I never needed to figure out and we were less concerned about any kind of garbage cleaner or when something was in or out of scope.

Thank you!