Propagate error on loading Receiver

[shinghim]

Closes #718

[coveralls]

Pull Request Test Coverage Report for Build 15305466743

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 84.553%

---

Totals
Change from base Build 15273225656:	0.0%
Covered Lines:	6519
Relevant Lines:	7710

---

💛 - Coveralls

[spacebear21]

ACK

[DanGould]

@spacebear21 I appreaciate your review on the road but I think this one needs a closer eye. Isn't this change both lossy and unnecessarily assigns, making it asymmetrical from the others? Beyond the asymmetry, have we considered what it would take to keep the error data as long as possible before it needs to be used across the FFI boundary?

[spacebear21]

Isn't this change both lossy and unnecessarily assigns, making it asymmetrical from the others?

I suppose it does unnecessarily assign. Do you mean it's lossy because of the string conversion? iirc it's the only way we're currently able to convert a generic std::error::Error + Send + Sync + 'static error to the ffi ImplementationError type.

have we considered what it would take to keep the error data as long as possible before it needs to be used across the FFI boundary?

Is this the same concern re: lossiness? I guess we could change the Result error type for load to not be an ImplementationError, and instead make it P::Error.

[shinghim]

iirc it's the only way we're currently able to convert a generic std::error::Error + Send + Sync + 'static error to the ffi ImplementationError type.

yeah, the main reason i converted it from a string was that it's the only way to convert

and unnecessarily assigns

I thought it might be better for readability (maybe not by much), and the compiler should optimize the unnecessary assignment during release builds. I don't mind going back and updating this if we figure out the lossy conversion and/or removing the unnecessary assignment