algesten / str0m

A Sans I/O WebRTC implementation in Rust.
MIT License
317 stars 49 forks source link

refactor: remove `StunError` #472

Closed thomaseizinger closed 6 months ago

thomaseizinger commented 7 months ago

Currently, StunError has two variants: Parse and Io.

I'd suggest to remove StunError in favor of using io::Error::custom with a string. That makes the public API smaller.

algesten commented 7 months ago

I think that's going the wrong way, losing semantics. I think instead we should make specific Parse error codes for the various problems instead of relying on a string description.

thomaseizinger commented 7 months ago

I think instead we should make specific Parse error codes for the various problems instead of relying on a string description.

Is that really useful? It is not like a parse error can be corrected, i.e. matched upon. Modelling each error out as a variant is very desxriptive but as a user, you just end up logging / bubbling it up anyway. Hence the thinking of making the API simpler.

algesten commented 6 months ago

It makes for better testing. No text matching.

thomaseizinger commented 6 months ago

It makes for better testing. No text matching.

It looks ugly to compare err.to_string I agree. But it only the test compares it, I don't really see a difference. YMMV :)

algesten commented 6 months ago

I prefer to keep this for now.