Show more errors #1967
Show more errors #1967
Conversation
There was a problem hiding this comment.
I sketched some more how I think we would be ideally handling errors and which direction it's best to move in. I appreciate it may be hard to do however and am not objecting to this as an intermediate step to improve error reported right now. But if you're working on improving errors I think it's good to look at the big picture as well.
| @@ -574,7 +574,7 @@ pub enum Error { | |||
| #[error("Invalid email address: {0:?}")] | |||
| InvalidEmailAddress(String), | |||
|
|
|||
| #[error("XML error at position {position}")] | |||
| #[error("XML error at position {position}: {error}")] | |||
There was a problem hiding this comment.
wasn't your PR wrt the FFI error reporting supposed to automatically print causes? Somehow manually chaining error descriptions like this seems a bit off, the anyhow::Error reporting-type with {:#} formatting should be taking care of this should it not? My guess is that the actual problem is that whoever reports/logs this doesn't report the causes.
There was a problem hiding this comment.
Problem is, that's a thiserror error and thiserror does not do this: dtolnay/thiserror#98
| @@ -12,7 +12,7 @@ pub async fn read_url(context: &Context, url: &str) -> Result<String, Error> { | |||
| match surf::get(url).recv_string().await { | |||
| Ok(res) => Ok(res), | |||
| Err(err) => { | |||
| info!(context, "Can\'t read URL {}", url); | |||
| info!(context, "Can\'t read URL {}: {}", url, err); | |||
There was a problem hiding this comment.
Here in an ideal world, we wouldn't do any error logging and just report it up. On line 6 the cause should have a #[from] annotation so that you can just propagate using ? and be done. And someone much closer to the application should be doing the logging.
I think as a general rule "much closer to the application" for us means the FFI interface should be doing all the error logging. The rest of the deltachat core should be treated as a library and errors should be propagated out and not logged (bearing in mind there are always exceptions and practical obstacles. I'm well aware we are far away from being in that nice place)
There was a problem hiding this comment.
Problem is, this is during configure and many URLs are tried for autoconfig and if we do not find any, we just guess the config. So, if configuration fails, we don't know if the problem was that we could not read the autoconfig or that we could not reach the server. But we can return only one error. Of course we could add more info to this error (which is also shown to the user) but I don't really think that's worth it.
|
Let's just merge it and upvote dtolnay/thiserror#98 and dtolnay/anyhow#111 so that maybe one day we can get closer to this ideal world but I currently have no idea how to realize @flub's ideas. |
Examining deltachat/deltachat-android#1631 I realized that the current warnings are not very helpful.