Skip to content

Conversation

@Narfinger
Copy link
Contributor

@Narfinger Narfinger commented Aug 5, 2025

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 Narfinger force-pushed the error-handling branch 2 times, most recently from a9f4457 to 3f0752a Compare August 5, 2025 09:47
@Narfinger Narfinger changed the title Improved error handling Unify error handling to have IpcError and TryRecvError be the main errortypes. Aug 5, 2025
@Narfinger Narfinger force-pushed the error-handling branch 4 times, most recently from fae67df to 4ef7ae4 Compare August 6, 2025 07:49
@Narfinger
Copy link
Contributor Author

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

@Narfinger Narfinger marked this pull request as ready for review August 6, 2025 07:50
@sagudev sagudev self-requested a review August 6, 2025 07:52
@Narfinger Narfinger force-pushed the error-handling branch 2 times, most recently from 512913c to d606cd9 Compare August 7, 2025 08:21
@sagudev
Copy link
Member

sagudev commented Aug 8, 2025

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
Copy link
Contributor Author

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
Copy link
Member

jschwe commented Dec 16, 2025

@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
Copy link
Member

sagudev commented Dec 16, 2025

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
Copy link
Member

jschwe commented Dec 17, 2025

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
Copy link
Member

sagudev commented Dec 17, 2025

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.

@Narfinger Narfinger force-pushed the error-handling branch 2 times, most recently from 6bfcfb3 to 09ce2e9 Compare December 17, 2025 09:31
types.

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.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
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 this pull request may close these issues.

3 participants