Use anyhow or some other systematic error handling approach

[DCNick3]

Currently, some errors are handled as just Result<T, &'static str> or Result<T, String>. While this may be fine in some cases, I suggest using a more systematic approach, like, for example, anyhow provides.

Once of its nicer features is providing context to previous errors, giving nicer error messages overall.

For example:

fn work_with_files() -> anyhow::Result {
  std::fs::read("hello.txt").context("reading hello.txt")?;
  std::fs::write("bye.txt", "sayonara").context("writing bye.txt")?;
}

When printing the error not only the underlying IoError will be printed, but the context chain as well, like:

reading hello.txt
Caused by:
- Os { code: 9, kind: Uncategorized, message: "Bad file descriptor" }

Maybe you should also consider snafu: it gives similar features (context too), but allows you to type all your errors (still providing the Whatever escape hatch). This might be useful for exposing errors to the guest, as they are expected to be classified as some errno code, which is not easy for pure string errors...

[hikari-no-yume]

Mm, optimal error handling is a topic I'm not super familiar with in Rust. I know how to use Result and Option pretty well, but I haven't even touched Error, let alone third-party libraries.

On that note, in general I want to keep the number of dependencies pretty small and therefore I want to avoid using an external dependency unless I really have to, so if there's a way to do something with just what std provides, even if it's a bit more fiddly or verbose, I prefer to do it that way. I think I picked up this habit from C programming, and I know it's not as necessary in Rust, but considering the sheer number of things an iPhone OS emulator is going to need to do (audio, video, graphics, text, input, and so much more), I think it'll be good for keeping the binary size and build time under control.

Anyway, I have some thoughts about error management in this project in general.

The overwhelmingly most common error handling approach is to panic, directly or indirectly (unwrap(), assert!, etc). I believe that's generally discouraged as a last resort, but in this project, most of the time there's no better alternative. Usually the error is related to the emulator not supporting something the app needs, or the app doing something unexpected, and in either case it's safest to immediately crash so it doesn't continue into a broken state. Perhaps one reason people don't like panics, especially unwraps, is they don't necessarily give good context, but I have a nice panic handler that gives you a backtrace for the guest code (the emulated stack, which exists in parallel to the host/Rust stack), and I think that's often the best information we possibly could offer.

There are of course some errors that aren't just immediate panics, which are what you're talking about, though. I think there's just three categories of these:

1. Errors from the early phase of emulator startup, before the CPU emulation
2. Errors we need to be able to propagate to the guest code
3. Errors that aren't specific to either of these, but are going to be consumed by code that's one or both of the above

For (1), I guess the motivation was that having main() return a Result is more idiomatic and pretty, but it also had the benefit of being able to provide slightly more helpful error messages for common user errors, like passing the wrong bundle path. I used a String just for simplicity. I don't think there's ever much context going on here, the call stacks aren't very deep and the errors are usually simple stuff like failing to load Info.plist.

For (2), there's some possibility that embedding more context in the error would help debugging, but I think I'd find it more useful to just add more debug log statements instead. For the error value, the most important thing is that we can translate it to the error values the guest code expects, rather than human readability. In some places I've used Err(()) simply because I haven't needed to expose an error yet. I'll surely replace some of them with enums eventually.

Finally, I guess (3) is very similar to (2) in terms of the considerations involved.

Sorry for the essay. :)

[DCNick3]

I don't think the "minimal dependencies" approach is really pragmatic, as you would probably end up inventing already existing libraries in your codebase, but ok...

I tend to agree that using panics in HLEs is fine. In my view, people don't like them because they can't really handle them in downstream code (like in a server or smth). Context is quite easy to obtain: just run it under debugger ;)

Regarding (1): fair, if you wouldn't get too many code which may produce such errors, it'll be fine. Even then, you can manually add context with .map_err(|e| format!("Failed to shave yak: {}", e)), it's just more tedious ig.

(2) & (3): I think this is where snafu might help, but if you don't want the context for debugging (can't really expose it guest) you will probably be fine with one flat error enum

[hikari-no-yume]

I don't think the "minimal dependencies" approach is really pragmatic, as you would probably end up inventing already existing libraries in your codebase, but ok...

Well, I'm trying to be pragmatic about it and I make decisions on a case-by-case basis. If it's not something trivial to reimplement, then I'll pull in an external dependency. All the current dependencies are examples of that; I didn't want to reinvent the wheel with image and audio decoding, plist file parsing, font decoding, window management, opengl function loading, etc etc. On the other hand I didn't pull in a command-line argument parser because str::strip_prefix() goes a long way, I didn't pull in the byteorder crate because ::from_le_bytes() works fine, etc.

That said, reimplementing things ends up being the cleanest approach fairly often during this project, even for slightly more complex things, and that's simply because Rust libraries tend to have nice safe, clean, high-level abstractions… and then the Objective-C or C API we want to implement tends to also be fairly high-level and abstract, but work completely differently in some important way, and so we get abstraction layering problems. ^^;

[hikari-no-yume]

Based on my reading of the room here and in #34, I'll close this for now, but maybe it can be reopened in future if it seems relevant.