{"action":"created","issue":{"url":"https://api.github.com/repos/maps4print/azul/issues/13","repository_url":"https://api.github.com/repos/maps4print/azul","labels_url":"https://api.github.com/repos/maps4print/azul/issues/13/labels{/name}","comments_url":"https://api.github.com/repos/maps4print/azul/issues/13/comments","events_url":"https://api.github.com/repos/maps4print/azul/issues/13/events","html_url":"https://github.com/maps4print/azul/issues/13","id":356203377,"node_id":"MDU6SXNzdWUzNTYyMDMzNzc=","number":13,"title":"[Feature Request] Allow error-propagation","user":{"login":"Xoronic","id":30958364,"node_id":"MDQ6VXNlcjMwOTU4MzY0","avatar_url":"https://avatars0.githubusercontent.com/u/30958364?v=4","gravatar_id":"","url":"https://api.github.com/users/Xoronic","html_url":"https://github.com/Xoronic","followers_url":"https://api.github.com/users/Xoronic/followers","following_url":"https://api.github.com/users/Xoronic/following{/other_user}","gists_url":"https://api.github.com/users/Xoronic/gists{/gist_id}","starred_url":"https://api.github.com/users/Xoronic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Xoronic/subscriptions","organizations_url":"https://api.github.com/users/Xoronic/orgs","repos_url":"https://api.github.com/users/Xoronic/repos","events_url":"https://api.github.com/users/Xoronic/events{/privacy}","received_events_url":"https://api.github.com/users/Xoronic/received_events","type":"User","site_admin":false},"labels":[{"id":806834497,"node_id":"MDU6TGFiZWw4MDY4MzQ0OTc=","url":"https://api.github.com/repos/maps4print/azul/labels/feature-request","name":"feature-request","color":"84b6eb","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-09-01T13:41:43Z","updated_at":"2018-09-04T10:06:34Z","closed_at":null,"author_association":"NONE","body":"## Description\r\n\r\nI tried to propagate errors instead of letting the program `panic!` with `unwrap()` calls:\r\n\r\n```rust\r\nfn start() -> Result<(), /* What to put here? */> {\r\n    // Instantiate the app with default values.\r\n    let mut app = App::new(DataModel::default(), AppConfig::default());\r\n\r\n    // Create a new window.\r\n    app.create_window(WindowCreateOptions::default(), Css::native())?;\r\n\r\n    // Run the app.\r\n    app.run()?\r\n}\r\n```\r\n\r\nThe example above illustrates the issue: \r\n- **`app.create_window`** returns a `Result<(), WindowCreateError>`\r\n- **`app.run`** returns a `Result<T, RuntimeError<T>>`\r\n\r\nBut they do not share an `Into` (or `From`) implementation. In other words: There is a lack of an Error-implementation higher in the hierarchy.\r\n\r\nThis forces the API-consumer to either `unwrap` all errors which:\r\n- Causes visual clutter in the code\r\n- Panics: Exactly the thing that the `Error` type should prevent\r\n\r\n<!--\r\nA clear and concise description of what the current problem is.\r\nEx. \"I have an issue when [...]\"\r\n-->\r\n\r\n## Proposed solutions\r\n### Option 1: Implement `std::error::Error` for all the top-level error-types in Azul\r\nFrom looking at the code, that would be:\r\n- `CssParseError`\r\n- `FontError`\r\n- `ImageError`\r\n- `ClipboardError`\r\n- `WindowCreateError`\r\n\r\nI'm not sure, but I think that those are the only errors returned from functions in the library.\r\n\r\n### Option 2: Create one top-level `azul::Error`\r\nThe `azul::Error` should then be implemented by all the errors mentioned in *option 1*.\r\n\r\nWhich all other azul Errors can convert into. (Also see [rfc-356](https://doc.rust-lang.org/1.0.0/style/style/naming/README.html#avoid-redundant-prefixes-[rfc-356]) about stuttering names)\r\n\r\n### Option 3: Kind-of combination of option 1 and option 2\r\nCreating one top-level `azul::Error` which implements `std::error::Error`. This allows API-consumers to use the azul::Error together with other libraries and use the errors interchangeably.\r\n\r\nThis one is my favourite :)\r\n\r\n## Is this a breaking change\r\nI'm not sure,  option 2 and 3 may be be a breaking change?\r\n\r\n__________________\r\n\r\nI'm willing to help and write the code if you find one of the solutions a good idea.\r\n\r\nGreat job by the way, I like the library and I think it has a big potential of becoming a widely used GUI library for Rust (which is something Rust really needs).\r\n"},"comment":{"url":"https://api.github.com/repos/maps4print/azul/issues/comments/418312637","html_url":"https://github.com/maps4print/azul/issues/13#issuecomment-418312637","issue_url":"https://api.github.com/repos/maps4print/azul/issues/13","id":418312637,"node_id":"MDEyOklzc3VlQ29tbWVudDQxODMxMjYzNw==","user":{"login":"fschutt","id":12084016,"node_id":"MDQ6VXNlcjEyMDg0MDE2","avatar_url":"https://avatars1.githubusercontent.com/u/12084016?v=4","gravatar_id":"","url":"https://api.github.com/users/fschutt","html_url":"https://github.com/fschutt","followers_url":"https://api.github.com/users/fschutt/followers","following_url":"https://api.github.com/users/fschutt/following{/other_user}","gists_url":"https://api.github.com/users/fschutt/gists{/gist_id}","starred_url":"https://api.github.com/users/fschutt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fschutt/subscriptions","organizations_url":"https://api.github.com/users/fschutt/orgs","repos_url":"https://api.github.com/users/fschutt/repos","events_url":"https://api.github.com/users/fschutt/events{/privacy}","received_events_url":"https://api.github.com/users/fschutt/received_events","type":"User","site_admin":false},"created_at":"2018-09-04T10:06:34Z","updated_at":"2018-09-04T10:06:34Z","author_association":"MEMBER","body":"Do not use failure. Azul already has 113 dependencies and failure hugely overcomplicates things for no reason. Failure is mainly a convenience library for **quick CLI applications** and has [huge performance problems](https://www.reddit.com/r/rust/comments/7te8si/personal_experience_can_cause_hard_to_notice/). Failure is overengineered to the extreme and in this case it's absolutely unnecessary.\r\n\r\nDon't implement `std::error::Error` - I should have said this earlier. In my opinion, `std::error::Error` is the most useless trait in the whole standard library because `.cause()` can only return a `&str`, not a `String`, so you can't return dynamic error information without jumping through hoops (which nobody does, so everybody just returns a `&static str` and leaves the error information out, which is not the point of printing an error). The `.cause()` method also is often completely unnecessary, because the information conveyed in the `Error` returned by `.cause()` is already conveyed by `std::fmt::Debug` or `std::fmt::Display`. The only thing you can do with the `Error` object returned from `.cause()` is to print its `.description()` - but that's already what the `Debug` and `Display` traits do! Plus it now limits every error to only return a `&str`, so you can't put dynamic formatting in your error printing, as above.\r\n\r\nSo there is absolutely nothing gained by implementing `std::error::Error`, it's purely boilerplate noise that nobody needs. It's just something that many people do because \"it's in the standard library, so it must be good to implement\". Most people only use `std::error::Error` to print the error - but `Display` and `Debug` are much more flexible (because they can dynamically format an error).\r\n\r\n`std::error::Error` also makes other problems, such as having error information duplicated between `Display` and `Error`, requiring an impl for all sub-types, which again, leads to more boilerplate noise as you've already experienced - or overengineered solutions such as failure or error-chain.\r\n\r\nAnother reason to not do it is that this is a GUI library and I don't expect anyone to wrap his errors in any sub-functions. If a user wants to create an error, he has to put these errors in his data model (callbacks aren't allowed to return an error) - if the user then wants to implement `std::error::Error` for them, that's fine by me, but that's not the concern of this library.\r\n\r\nLastly, remember that the only reason why you wanted to implement this is that you can do `fn main() -> Result<(), azul::Error>` - that doesn't require you to implement `std::error::Error` for `azul::Error`, only `std::fmt::Debug`: https://play.rust-lang.org/?gist=edd6d6dc2f5ebe2a5ff12a58b7fee3a6&version=stable&mode=debug&edition=2015\r\n\r\nSo just make a wrapper error enum, implement `std::fmt::Debug` and/or `std::fmt::Display` on it and leave `std::error::Error` out. It's neither necessary nor a particularly useful trait to implement."}}
{"actor":{"display_login":"fschutt"}}