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

Improve Error Handling with Snafu and Eliminate panic! Calls #190

Closed
13 tasks
ChristopherRabotin opened this issue Jun 24, 2023 · 0 comments · Fixed by #273
Closed
13 tasks

Improve Error Handling with Snafu and Eliminate panic! Calls #190

ChristopherRabotin opened this issue Jun 24, 2023 · 0 comments · Fixed by #273
Labels
Interface: Rust Kind: Improvement This is a proposed improvement Priority: normal Status: Design Issue at Design phase of the quality assurance process

Comments

@ChristopherRabotin
Copy link
Member

High level description

Nyx Space currently handles errors through a large enum, making the process of adding new variants complex and less clear. The error handling could greatly benefit from improved organization, specificity, and scalability. Using the snafu library can provide these benefits, as it allows us to create context-specific errors and provides backtraces. Transitioning to snafu would enhance our error handling capabilities, making it easier to add, manage, and debug errors.

Requirements

  • The error handling system should be refactored using the snafu library to allow for more granular, context-specific errors.
  • The current large error enum should be divided into smaller, more specific error enums that clearly state their purpose and context.
  • Each error variant should provide a detailed and clear message to aid in debugging.
  • The new error handling system should be scalable, making it easy to add new error variants as the Nyx Space project evolves.
  • The refactoring process should include updating all current error calls to their corresponding snafu variants.
  • The system should provide backtraces when errors occur to aid in debugging.
  • All uses of panic! throughout the code base should be removed and replaced with appropriate error handling. This will help to ensure that the application fails gracefully, providing informative error messages to the user rather than causing abrupt termination.

Test plans

  • Unit tests for each error variant: For each error variant in our new error handling system, create a unit test that triggers this error. This will verify that each error can be triggered as expected and that it returns the expected message.
  • Integration tests for error propagation: Create tests that involve multiple functions to ensure that errors are correctly propagated up the call stack. This can be done by intentionally causing an error in a low-level function and verifying that the high-level function that calls it receives and correctly handles the error.
  • 'panic!' removal verification: Run the existing test suite and intentionally cause errors that would have previously caused a panic. Verify that these errors now result in an appropriate error message and graceful termination.
  • Code review: A thorough code review should be conducted to ensure no instances of panic! remain and that the new error handling system has been implemented correctly and consistently.
  • Manual testing: Perform manual testing to induce errors in real-world use cases, ensuring the application's behavior is as expected. This also allows us to see the error messages a user would see.
  • Edge case testing: Think about and test the edge cases where errors might be less likely to occur, ensuring the error handling system performs correctly in all scenarios. This can include things like unexpected user input, resource limitations, etc.

Design

The design section would typically include a MermaidJS diagram, but since the changes here are primarily concerned with improving code quality and don't involve new components, relationships, or data flow, it's not straightforward to represent these changes in a diagram. The changes mostly consist of internal refactoring and don't impact the architecture at a level that could be usefully visualized with MermaidJS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Interface: Rust Kind: Improvement This is a proposed improvement Priority: normal Status: Design Issue at Design phase of the quality assurance process
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant