Reexport nix::Errno

[Ten0]

This is useful because the Error enum in its ApiError error variant exports that type, so in order to match on the error value it's necessary to use the same version of nix as this crate uses.

The idiom in this case is to reexport nix (or reexporting Errno would likely be fine as well, but I'm not sure if nix leaks in other ways maybe so I propose exporting the entire nix to be sure that it's correct.)

[Xuanwo]

Hello, thank you for bringing this up. I'm not fond of reexport. Do you think it would be better to return std::io::Error instead?

[Ten0]

Thanks for your quick answer.

It doesn't look like that would be better: it's useful to be able to match on the error code (which is specifically what I'm using nix for here, to catch "file already exists" and "file does not exist" errors after making custom write operations), and IoError would make that significantly harder. Also it seems that it would make it less clear that these are precisely the errors that are returned by the librados functions.

Why do you not like reexports?

[Xuanwo]

Hi, @Ten0, thanks for your explanation of motivation.

I think it's a failure to expose these error types directly. I wish we could use ceph::ErrorKind::NotFound and ceph::ErrorKind::AlreadyExists with source errors instead of exposing external types like nix::Error, uuid::Error, or serde_json::error::Error.

Given that our API currently works this way, it might be fine to re-export nix if you want. We can discuss the error kind design in our next major version. What do you think?

[Ten0]

Given that our API currently works this way, it might be fine to re-export nix if you want.

Thanks. 😊

I think it's a failure to expose these error types directly. I wish we could use ceph::ErrorKind::NotFound and ceph::ErrorKind::AlreadyExists with source errors instead of exposing external types like nix::Error, uuid::Error, or serde_json::error::Error.

I agree that in an ideal world the Rust API wouldn't expose those, as they are hard to catch by users because they have to work out what each means.

Errors such as uuid::Error are unlikely to ever be caught, they will likely always be turned into generic textual errors for reporting, especially since the enum doesn't give information about where the error occurred, only about what error occurred.

However it seems difficult to be exhaustive on the ApiError variant: librados C api's error codes are undocumented, so the only way to know what error would occur in what scenario would be to test each of them manually, which seems like a very tedious task at this point, and we wouldn't even have the guarantee that the same error code is used for similar semantic meanings across C functions.
Currently a user who wants to catch a particular error probably has encountered it (or can reasonably easily get themselves to encounter it), so it's reasonably easy for them to obtain the error code that they want to catch.
If we were to only cover for a small part of the scenarios, that would make every addition to the list of known scenarios a breaking change (because people may be catching the "unknown" error), which doesn't seem great.
This makes me feel like at this point we shouldn't be over-ambitious here, and the compromise of letting the user experiment with what error code they get and catch that seems reasonable. => I like the current RadosError::ApiError API.

However it's also true that reexporting the whole of nix seems dirty. For the particular use-case described above I really only need Errno to be re-exported so maybe we can just re-export that ?
I just put nix in the PR because I didn't take the time to check whether there was anything else from nix that was leaking from the crate...

[Ten0]

So I've checked and actually the only use of nix is for Errno so I've updated the PR to just reexport Errno.
25fa4be

[Xuanwo]

Thank you @Ten0 for this!