Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove StunError #472

Closed
thomaseizinger opened this issue Mar 5, 2024 · 5 comments
Closed

refactor: remove StunError #472

thomaseizinger opened this issue Mar 5, 2024 · 5 comments

Comments

@thomaseizinger
Copy link
Collaborator

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

algesten commented Mar 5, 2024

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

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

It makes for better testing. No text matching.

@thomaseizinger
Copy link
Collaborator Author

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

I prefer to keep this for now.

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

No branches or pull requests

2 participants