Defensively marking enum variants as non-exhaustive for WASI 0.3.0

[yoshuawuyts]

I originally posted in #891 (comment):

If we want to be defensive about the 0.3.x target, we could also consider adding an additional other variant too. In Rust core::io::ErrorKind has an Other variant which is intended to be used for any variants not captured by the existing variants. This has been historically useful because up to Rust 1.40 (November 2019) Rust did not have support for the #[non_exhaustive] attribute, and so ErrorKind could not be extended with more variants.

I checked and we already have other(string) variants for method and scheme. error-code also has a defensive variant (internal-error(option)). Do people have thoughts about applying that more broadly?

This is broader than just wasi:http though, and relevant before we release WASI 0.3.0. Do folks have thoughts about this?

---

cc/ @lukewagner I feel like you might have thoughts about this

[lukewagner]

Until we add proper non-exhaustive enum/variant support in WIT directly (as discussed in CM/#454, which increasingly seems like a good idea), I think it makes sense for us to defensively add other cases to every WASI enum/variant which in the future would be made a non-exhaustive enum.

[ricochet]

Already have a catch-all:

• error-code (sockets): already has unknown
• descriptor-type (filesystem): already has unknown

Not applicable:

• ip-address-family (sockets): protocol-defined (ipv4, ipv6), unlikely to change
• ip-address (sockets): tied to IP protocol, same as above
• ip-socket-address (sockets): tied to IP protocol, same as above
• advice (filesystem): mirrors POSIX posix_fadvise, stable interface
• new-timestamp (filesystem): semantic choices (no-change/now/value), not an extensible enumeration

[badeend]

@ricochet Shall we rename the socket's error-code::unknown to other as well? For consistency with the other WASI interfaces.

[ricochet]

@badeend - thank you for the review! Is unknown a direct translation to an existing API or was this a defensive error code that you added?

In PR's like #894 I started with just adding a plain option to the enum without a payload, but I don't believe this is sufficient. By converting to a variant, the other case can carry an option<string> payload, giving implementations a way to surface the error. Without the payload, a bare other enum case tells consumers that something went wrong, but not what; the optional string lets hosts forward an unstructured description or allow implementers to extend with their own known error.

[vados-cosmonic]

My 2c here is definitely in favor of some indication of what the error was (a variant works fine IMO). It seems like a good use of the expressiveness of WIT vs forcing host implementers to have some sort of get-latest-err() -> string function to help users figure out what exactly the error was about.

[vados-cosmonic]

Another 2c (up to 4c now!) on the socket one -- I think ErrorCode::Unknown gets used as a catch-all right now -- I can't find any errors that match this in socket.h, so it really feels like an other(option<string>) would go well basically all the places that we currently have tracing::debugs right before the error is returned.

There's a little worry about exposing internal details in the errors, but I think having the option should be enough for hosts to choose their own future there.

In the case of the descriptor-type it's a little nuanced... I think unkown is actually enough information there -- but I can see the flexibility of letting host implementers be more descriptive there and essentially support new use cases that we didn't necessarily plan for. Given that we can't make a change I like to lean on the side of flexibility (and of course, if it's option hosts can choose not to share information)...

I think the case for descriptor-type.unknown is weaker but worth doing, socket's error-code.unknown is clearly a catch-all.

[badeend]

Is unknown a direct translation to an existing API or was this a defensive error code that you added?

It was indeed a defensive option, so I think we can safely rename it to other like the (ahem) others.

Regarding the option<string>; POSIX has no equivalent to an "Other" error code, nor does it have a way to get the last error message. So any additional info provided by the host will be tossed away by wasi-libc. Because of that, I don't see a practical benefit for adding option<string> when it comes to the POSIX'y interfaces. But it doesn't get in the way either, so if it brings some cohesion to the WASI ecosystem it's fine by me 👍