Stream: project-error-handling

Topic: Box error requers Size


view this post on Zulip Eh2406 (Nov 15 2021 at 20:55):

In my library, I was trying to move from Box<dyn Error> to T: Error. Most users want to use Box<dyn Error>, but it is more flexible for thems that want a concrete error. But I keep hitting a problem because Box only impls Error, if T impls Error and is sized. Here.

Is there a reason for that?

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:03):

At this point the only reason is backwards compatibility

view this post on Zulip Eh2406 (Nov 15 2021 at 21:04):

Would it be a breaking change to add + ?Sized?

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:04):

The current plan to fix this is to introduce a thin box type that also has the feature of making trait objects only one pointer wide, and on this new type implement the correct error impl

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:04):

Yes, it would be a breaking change

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:04):

It would cause overlap with from e for box dyn error

view this post on Zulip Sean Chen (he/him) (Nov 15 2021 at 21:05):

https://stackoverflow.com/questions/65151237/why-doesnt-boxdyn-error-implement-error gives some more context into this

view this post on Zulip Eh2406 (Nov 15 2021 at 21:07):

Can we add a comment to the code linking to the issue?

view this post on Zulip Eh2406 (Nov 15 2021 at 21:08):

( given how easy the fix looked, I knew I was missing something. )

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:08):

:+1:

view this post on Zulip Eh2406 (Nov 15 2021 at 21:46):

So to back out of my X-Y problem. My library takes a callback witch can return an error. Currently Box<dyn Error>. When my code calls the callback and it returns an error, we wrap it up in some information about why (here) and bubble it back out to the user of my library. It feels restrictive too require dynamic dispatch and allocations just to pass the error through my code. What is the current best practice here?

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:53):

I'm currently away from my computer so I am only giving like vague impression and haven't actually checked the code, but my default go to would be to have trait bounds that require that the return type impls from for your error types that you're going to use to add the extra context

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:53):

Wait no actually I think that's wrong

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:54):

Because presumably you would already have their error type before you would be contextualizing it with your own

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:54):

I'll take a look when I get home

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 21:55):

But I know I've run into this issue almost exactly when working with Tower in the past

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 23:42):

okay so initial pass, you might be able to use E: Into<Box<dyn Error>> instead of E: Error

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 23:42):

which will at least cover both, though you're still working with Box<dyn Error> on your end

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 23:43):

@Eh2406 can you describe more why you want to switch from Box<dyn Error> to T: Error?

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 23:46):

or do you have a diff showing the changes you're trying to make to the DependencyProvider trait?

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 23:46):

I assume you're updating this method: https://github.com/pubgrub-rs/pubgrub/blob/717289be5722dd5caaa0d1f4ed13047d11a7f7fd/src/solver.rs#L256

view this post on Zulip Eh2406 (Nov 15 2021 at 23:47):

Now I am on the road. I can reply after dinner. Thanks for the help.

view this post on Zulip Jane Lusby [she/her] (Nov 15 2021 at 23:47):

:+1:

view this post on Zulip Eh2406 (Nov 16 2021 at 00:36):

That method is a good example. All the methods from DependencyProvider have -> Result< something , Box<dyn Error>>;

view this post on Zulip Eh2406 (Nov 16 2021 at 00:37):

Errors should be rare, so maybe it doesn't matter very much. But it feels like I'm constraining the user.

view this post on Zulip Eh2406 (Nov 16 2021 at 00:40):

If there was an associated type DependencyProvider::Error and everything returned -> Result< something , Self::Error>;, then our users could pick their representation. Do they need an allocation, and dynamic dispatch?

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:41):

That would work, though you'd still need to be able to introduce your own error kinds right?

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:42):

Like a NoSolution error

view this post on Zulip Eh2406 (Nov 16 2021 at 00:47):

Right! We would still need our own error enum. But where it now has Box<dyn std::error::Error> it would have DP::Error.

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:50):

I'm wondering if you might be able to get away with adding a generic parameter to your error enum so that users could specify how you wrap their errors

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:50):

And use that generic parameter instead of box dyn error

view this post on Zulip Eh2406 (Nov 16 2021 at 00:53):

I don't why last time I tried this I ended up constraining the parameter to be E: Std:Error, and fell down the rabbit hole of trying to figure out why Box<dyn Error> did not work as an instantiation.

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:56):

Oh yeah, you might need to use a Deref bound or something instead of an Error bound to make it work for box and other error types

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:56):

Not sure if errors implicitly Deref to a dyn Error :thinking:

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:57):

This is where it always gets really annoying

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:57):

And this is why I want to add thin box that just avoids this problem

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:58):

Honestly, my recommendation would be to just use an error bound and possibly provide some sort of convenience function for converting box dyn errors into something that implements error by just like wrapping it in a new type

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:58):

Like box dyn error not implementing error was a mistake and you shouldn't let it constrain your API

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:59):

It's not too hard for users to work around the issues themselves

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:59):

Hopefully not steering you wrong

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:59):

No promises though

view this post on Zulip Eh2406 (Nov 16 2021 at 00:59):

That is fair.

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 00:59):

:grimacing:

view this post on Zulip Eh2406 (Nov 16 2021 at 01:03):

It's amazing how many ripples a mistake in STD has.

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 01:10):

:/

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 01:11):

hard to avoid in this case, the real cause of the issue was wanting to be able to use ? with Box<dyn Error>, which seems clearly worth it in an abstract sense

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 01:12):

and the only reason we can really get away with not having the From impl now is because of the new Try trait which took a long ass time to get to the current state

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 01:13):

and even then it necessitates using multiple Result types which I'm still not convinced will be worth it

view this post on Zulip Eh2406 (Nov 16 2021 at 01:13):

Thanks for your expertise. I will give the refactor another try.

view this post on Zulip Jane Lusby [she/her] (Nov 16 2021 at 01:14):

my pleasure, let me know if I can help any more


Last updated: Jan 26 2022 at 14:02 UTC