Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VfsError does not have an easy way to programmatically match against Error Variants #33

Closed
Technohacker opened this issue May 10, 2022 · 5 comments · Fixed by #34
Closed

Comments

@Technohacker
Copy link
Contributor

Hey again!
Due to the existence of the VfsError::WithContext variant, it's harder to match on a specific error type since there may be multiple nested WithContext error levels to go through. My use case for this is to ignore an error if a file is missing, but to handle all other cases as invalid situations

There is also the situation of certain implementations returning errors like NotFound via std::io::Error rather than the VfsError variants, but I believe that's an implementation specific issue

To work around this, I currently have a function called get_root_cause that returns the true root cause of a VfsError, which also normalizes I/O error types to their respective VfsError variant

fn get_root_cause(err: VfsError) -> VfsError {
    match err {
        VfsError::WithContext { cause, .. } => get_root_cause(*cause),
        VfsError::IoError(ref io_err) => match io_err.kind() {
            io::ErrorKind::NotFound => {
                VfsError::FileNotFound { path: "".into() }
            }
            _ => err,
        },
        _ => err,
    }
}

If this fix is alright, I can make a PR for the same. But I wanted to have some discussion on how to proceed with this bug in case there were other solutions available

@manuel-woelker
Copy link
Owner

Thank your for the suggestion, this does indeed seem like common use case.
A PR would me much appreciated! 👍

@Technohacker
Copy link
Contributor Author

If it's alright to have a breaking change, it may also make sense to change the error type to a struct that contains the type of error, along with an optional cause for it. That way VfsErrorKind can handle only those error types that make sense from the VFS abstraction, leaving all other error situations to the error source

@manuel-woelker
Copy link
Owner

Hi, sorry, for the late reply. I am not sure using an additional struct is really necessary. Using source() should allow getting to the root cause. Can you elaborate on the benefits you see using this approach?

@Technohacker
Copy link
Contributor Author

My main reasoning behind that idea is that you could do an exhaustive match over each error kind, without having to match on WithContext and repeating the match, like how my current implementation of get_root_cause works

@manuel-woelker
Copy link
Owner

I see, yeah, that sounds quite a bit more ergonomic. Looking forward to the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants