Unify error handling to have IpcError and TryRecvError be the main errortypes.

[Narfinger]

These types will give the Bincode error, Io errors (which are converted
from platform errors) and Disconnect.
Additionally we now use the thiserror crate which removes some of the
code.

This is a breaking change but because there are others comming we do not increase the version yet.

[Narfinger]

Ok the async errors should be fixed now. Please rerun the CI to see if there is anything else missing.

[sagudev]

What's the status here, given that you closed #408? If we do not plan to change serialization library I wish to avoid landing any breaking changes at least in short term.

[Narfinger]

I still want to change serialization libraries but adding the benchmark comes first. I noticed that bitcode did not provide any improvements for the performance but postcard seems to be good.
I think we should first fix the benchmark, then we can see if the performance improvements for postcard hold up.

[jschwe]

@sagudev Would you be opening to revisiting this (breaking change) now? I think it would make sense to remove bincode from the public API, especially now that it is essentially unmaintained.

[sagudev]

especially now that it is essentially unmaintained

I think bincode is still maintained.

Would you be opening to revisiting this (breaking change) now?

I mean I would still like to avoid breaking change (we have nothing breaking on main, I think) for now until we have some real changes, but I am not against to change per se. Alternatively we can just land this and keep the main unreleased and patch 0.20 on another branch, but we have no such process in place.

[jschwe]

I think bincode is still maintained.

As of yesterday it is not. I don't really want to get into the details of the drama, but given that it is unmaintained, comments of the author on the reddit thread, and lastly someone claiming to be peripharly involved saying there are other redflags for the project, I think we should try to migrate away (not urgently / this year, but soonish, it's definitly easier than auditing the code).

I'm going to level with you here, bincode has far bigger supply chain attack red flags than the history rewrite, and should have never been used by any project that had a supply chain attack in its threat model without someone personally auditing the code. It's been a single person project with minimial to sometimes no community involvement for most of its existence, even in the rare instances where there's been multiple people working on it at the same time, there's been effectively no code review process for internal contributions.

My role in the project was as an emergency keyholder for the github organization, which really was the extent of the project's security practice and honestly is another supply chain attack red flag bigger than the git commit history rewrite. I have been kept informed of these goings on and provided some advice for how to achieve the specifics details of the transition that stygianentity wanted to achieve, but they were not my decisions to make, I was just made aware of them.

Source 1

[sagudev]

IIRC I read somewhere that he would still respond to CVEs.

Anyway I think webrender also uses it internally, so we will see what mozilla will do about that. Given the circumstance I think this PR is imminent anyway.

[sagudev]

Whoops, I was sure this already landed.