-
Notifications
You must be signed in to change notification settings - Fork 12
Description
Presently, the idol codegen functions called in build scripts return a Result<(), Box<dyn std::error::Error + Send + Sync>>:
Lines 12 to 31 in 6c54f3b
| pub fn build_client_stub( | |
| source: &str, | |
| stub_name: &str, | |
| ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> { | |
| Generator::default().build_client_stub(source, stub_name) | |
| } | |
| pub fn generate_client_stub_from_file( | |
| source: impl AsRef<std::path::Path>, | |
| out: impl std::io::Write, | |
| ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> { | |
| Generator::default().generate_client_stub_from_file(source, out) | |
| } | |
| pub fn generate_client_stub( | |
| iface: &syntax::Interface, | |
| out: impl std::io::Write, | |
| ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> { | |
| Generator::default().generate_client_stub(iface, out) | |
| } |
This error type is not really the greatest thing. In particular, if a build script unwrap()s it, or ?s it and returns Result<(), Box<dyn Error + Send + Sync>> from its fn main(), too, then the error gets formatted with fmt::Debug, rather than fmt::Display. This is sad if the error includes a cause chain, or has something like a parse error from the input IDL file, as these are formatted in a less nice way.
To ensure that these errors get Display printed when returned from main() rather than Debug'd, some of our build scripts wrap them in anyhow::Error...but there's no From<Box<dyn std::error::Error + Send + Sync>> for anyhow::Error, so you have to manually convert it by formatting them, in .map_err(|e| anyhow::anyhow!("{e}"))? rather than just ?ing it. This is mildly annoying to have to do, and worse, it and gets you a nitpick from @mkeeter in code review.
It would be nice to be able to return better errors from idol codegen. In particular, it would be nice if we could provide good error messages for syntax errors parsing the input IDL file, because this is a user error that the person changing an IPC interface will have to go debug. Previously, I started a change to switch idol to use the miette crate, since it has nice inbuilt support for displaying code snippets in errors. We probably should pick that back up at some point.