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 errors #1331

Merged
merged 3 commits into from
Jan 30, 2024
Merged

Refactor errors #1331

merged 3 commits into from
Jan 30, 2024

Conversation

cyqsimon
Copy link
Contributor

Motivation

I noticed that the ResponseError impl for ContexualError was not very complete. Many variants simply default to 500-ISE by wildcard match.

So I tried to fix that, but in the process I realised that some error variants are only encountered at program startup and not at runtime. Therefore I think it's much more appropriate to put them in separate enums.

Change summary

  • Split ContexualError into StartError & RuntimeError & AuthParseError
  • Made sure every RuntimeError variant has an accurate status code

- Split `ContexualError` into `StartError` & `RuntimeError`
- Made sure every `RuntimeError` variant has an accurate status code
@cyqsimon
Copy link
Contributor Author

Btw I intend to queue #1093 after this one.

Copy link
Owner

@svenstaro svenstaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this! Maybe just rename the Start stuff to Startup and we should be good.

src/errors.rs Outdated Show resolved Hide resolved
src/errors.rs Show resolved Hide resolved
@svenstaro svenstaro merged commit 4058805 into svenstaro:master Jan 30, 2024
17 checks passed
@svenstaro
Copy link
Owner

Very nice stuff!

@cyqsimon cyqsimon deleted the error-refactor branch January 30, 2024 02:27
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

Successfully merging this pull request may close these issues.

2 participants