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

chore: Review HTTP support codes #3357

Closed
wants to merge 1 commit into from

Conversation

ChrisBQu
Copy link
Collaborator

@ChrisBQu ChrisBQu commented Jan 7, 2025

Relevant issue(s)

Resolves #3139

Description

Across the code base we sometimes use http.StatusBadRequest (400) where we should be using http.StatusInternalServerError (500). I have gone through the code and made some of the changes that made logical sense to me.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Specify the platform(s) on which this was tested:

  • Windows

@ChrisBQu ChrisBQu self-assigned this Jan 8, 2025
@ChrisBQu ChrisBQu added the code quality Related to improving code quality label Jan 8, 2025
@ChrisBQu ChrisBQu added this to the DefraDB v0.16 milestone Jan 8, 2025
@ChrisBQu ChrisBQu requested a review from a team January 13, 2025 14:50
@AndrewSisley
Copy link
Contributor

question: Why is this a draft PR?

@ChrisBQu ChrisBQu marked this pull request as ready for review January 13, 2025 16:46
@ChrisBQu
Copy link
Collaborator Author

question: Why is this a draft PR?

I forgot to change it! I try to always create my PRs as drafts, then adjust them when I'm ready. I just forgot here.

@AndrewSisley
Copy link
Contributor

I forgot to change it! I try to always create my PRs as drafts, then adjust them when I'm ready. I just forgot here.

Ah okay 😁

@AndrewSisley
Copy link
Contributor

thought: I'm not sure these are all correct - many of these errors returned could be user input errors, in which case BadRequest would be appropriate no?

@ChrisBQu ChrisBQu closed this Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Related to improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review http status codes
2 participants