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

APis return Box<dyn Error> which isn't compatibile with useful ecosystem crates #282

Closed
cpcloud opened this issue Oct 18, 2020 · 2 comments · Fixed by #284
Closed

APis return Box<dyn Error> which isn't compatibile with useful ecosystem crates #282

cpcloud opened this issue Oct 18, 2020 · 2 comments · Fixed by #284

Comments

@cpcloud
Copy link
Contributor

cpcloud commented Oct 18, 2020

A number of APIs, especially the new_pipeline().install() flavors return Box<dyn Error>. This trait bound is incompatible with useful ecosystem crates like anyhow, whose Context trait requires Box<dyn Error + Send + Sync + 'static>.

I'm happy to submit a PR to fix this. Are there any APIs where we don't want this trait bound?

@frigus02
Copy link
Member

Good point. Using Box<dyn Error + Send + Sync + 'static> seems sensible to me. I can't think of a reason why we wouldn't want this. Can you, @jtescher?

@jtescher
Copy link
Member

Yeah this sounds good. We want to unify the error types with the current MetricsError type in the future, but this can come first 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants