y-crdt
y-crdt copied to clipboard
add `Result` for `Encode` trait
fix #288
this pr aims to add Result<(), lib0::error::Error> return value to the encode function of Encode trait
this allows developers to safely handle errors when encountering erroneous data
after this pr is merged, I plan to modify the panic in yrs to a YrsError structure generated by thiserror in the next pr, which can make the error message clearer
related issues: #272
at present, the related calls of yffi/ywasm directly unwrap the encode result. that can keep cause thread panic like the original internal panic. we can improve the related apis to allow catch errors in the future.
i have added the Result return value to more apis here: https://github.com/toeverything/y-crdt/commit/1dc82f202450342738940c687de06358da7fdcdc
but the test case needs to add a lot of unwrap: https://github.com/toeverything/y-crdt/commit/e278bb63252d172729d24ca418566e18e0cf72f4
i'm worried about whether this will cause review pressure, if you think this will not be a problem, I will cherry pick these commits into this pr
@darkskygit thanks for your contribution. I'll check it in more detail, once I return from the vacation.
While I was in favour of using results and std::io::Write-based encoding once I joined a project, we eventually decided to no go this way. It wasn't an accidental decision. It may be a good time to reevaluate it and make changes, however please keep in mind that doing this kind of PR without prior proposal may turn into a wasted effort from your side.
@darkskygit thanks for your contribution. I'll check it in more detail, once I return from the vacation.
While I was in favour of using results and
std::io::Write-based encoding once I joined a project, we eventually decided to no go this way. It wasn't an accidental decision. It may be a good time to reevaluate it and make changes, however please keep in mind that doing this kind of PR without prior proposal may turn into a wasted effort from your side.
Thank you for your reply, if you have any suggestions for revision, please feel free to let me know, I will make revisions as soon as possible.