Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow returning errors from eframe app creator #4474

Closed
emilk opened this issue May 7, 2024 · 0 comments · Fixed by #4565
Closed

Allow returning errors from eframe app creator #4474

emilk opened this issue May 7, 2024 · 0 comments · Fixed by #4565
Assignees
Labels
eframe Relates to epi and eframe rerun Desired for Rerun.io

Comments

@emilk
Copy link
Owner

emilk commented May 7, 2024

eframe::run_native takes a closure that creates the user's application. We should have some way of returning errors from it.

It already has a rather complicated signature:

pub type AppCreator = Box<dyn FnOnce(&CreationContext<'_>) -> Box<dyn App>>;

And that would need to become something like

type DynError = Box<dyn std::error::Error>;
pub type AppCreator = Box<dyn FnOnce(&CreationContext<'_>) -> Result<Box<dyn App>>, DynError>;

…unless we can come up with something nicer.

Motivation

In Rerun we initialize the renderer in the app creation callback, and that can fail because of missing features of the host GPU.

@emilk emilk added this to the Next Major Release milestone May 7, 2024
@emilk emilk added the eframe Relates to epi and eframe label May 7, 2024
emilk added a commit to rerun-io/rerun that referenced this issue May 10, 2024
### What
Using `scripts/fetch_crashes.py` to look at our analytics, I noticed
that missing wgpu capabilities would lead to a assert/panic, which is
not a nice way to report errors to users.

I added an egui issue to improve this further in the future:
* emilk/egui#4474

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6252?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6252?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6252)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
@emilk emilk added the rerun Desired for Rerun.io label May 28, 2024
@emilk emilk self-assigned this May 28, 2024
emilk added a commit that referenced this issue May 28, 2024
The closure passed to `eframe::run_native` now returns a `Result`,
allowing you to return an error during app creation, which will be
returned to the caller of `run_native`.

This means you need to wrap your `Box::new(MyApp::new(…))` in an
`Ok(…)`.

* Closes #4474
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe rerun Desired for Rerun.io
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant