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

Improved Error Type (more specificity, fewer semver hazards, etc...) #112

Open
geigerzaehler opened this issue Oct 19, 2022 · 1 comment
Labels
API-ergonomics Nothing's "broken", but the API could be improved design-required Getting this right will require some thought documentation Improvements or additions to documentation

Comments

@geigerzaehler
Copy link
Contributor

I recently started experimenting with this crate for AVR. When implementing the target I ran into TargetMismatch errors . The information in those errors was not enough for me to fix the issues—I had to dig through the source code instead.

The first issue was that Arch::Usize was different from what GDB send. To fix the issue I needed to know what GDB expected. The second issue was that Arch::BreakpointKind kind was different from what GDB sent. (I assumed that it would be ()).

What would have helped me to fix this issues faster is an error message pointing to what to fix and how to fix it.

(Happy to provide a PR for this if you want me to.)

@daniel5151
Copy link
Owner

Taking a step back: I think the broader issue here is that there is plenty of room for improvement in gdbstub's error types in general.

I've learned quite a bit about proper error handling in Rust in the years since starting gdbstub, and one thing that's for certain is that you want to have error types that uniquely identify where in the code something failed - something that most certainly isn't the case in the library today.

I spent a bit of time looking through the current code, and honestly - I'm not sure if I have a good "easy" solution off the top of my head. My gut feeling is that the current approach of a single top-level Error enum isn't ideal (it has some semver compat guarantees I'm not a fan of, and isn't particularly well "organized" wrt. different classes of error), and that a more holistic rework of the error type is in order.

That is to say: thanks for the feedback, and for offering to help out with a PR here, but I think this is something I should take a stab at myself at some point :)

@daniel5151 daniel5151 added documentation Improvements or additions to documentation API-ergonomics Nothing's "broken", but the API could be improved design-required Getting this right will require some thought labels Oct 22, 2022
@daniel5151 daniel5151 changed the title TargetMismatch did not help me fix wrong Arch implementation Improved Error Type (more specificity, fewer semver hazards, etc...) Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-ergonomics Nothing's "broken", but the API could be improved design-required Getting this right will require some thought documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants