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.

Sign up for GitHub

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

Jump to bottom

Fix StatusBadRequest -> StatusInternalServerError for internal errors in http handler #3103

Closed
shahzadlone opened this issue Oct 4, 2024 · 0 comments · Fixed by #3134
Closed

Fix StatusBadRequest -> StatusInternalServerError for internal errors in http handler #3103

shahzadlone opened this issue Oct 4, 2024 · 0 comments · Fixed by #3134
Assignees
@shahzadlone
Labels
area/acp Related to the acp (access control) system code quality Related to improving code quality
Milestone
DefraDB v0.15

Comments

@shahzadlone
Copy link
Member

Suggestion from @nasdf:
#3099 (comment)

Change:

	db, ok := req.Context().Value(dbContextKey).(client.DB)
	if !ok {
		responseJSON(rw, http.StatusBadRequest, errorResponse{NewErrFailedToGetContext("db")})
		return
	}

To return error 500 so roughly this:

	db, ok := req.Context().Value(dbContextKey).(client.DB)
	if !ok {
		responseJSON(rw, http.StatusInternalServerError, errorResponse{NewErrFailedToGetContext("db")})
		return
	}
@shahzadlone shahzadlone added code quality Related to improving code quality area/acp Related to the acp (access control) system labels Oct 4, 2024
@shahzadlone shahzadlone added this to the DefraDB v0.14 milestone Oct 4, 2024
@shahzadlone shahzadlone self-assigned this Oct 4, 2024
@shahzadlone shahzadlone mentioned this issue Oct 4, 2024
Merged
@shahzadlone shahzadlone changed the title Fix HTTP StatusBadRequest -> StatusInternalServerError for internal stuff in handler Fix StatusBadRequest -> StatusInternalServerError for internal errors in http handler Oct 4, 2024
@shahzadlone shahzadlone mentioned this issue Oct 15, 2024
Merged
ChrisBQu pushed a commit to ChrisBQu/defradb that referenced this issue Feb 21, 2025
@shahzadlone
refactor(i): Lint casts & move panics under must funcs (sourcenetwo…
9d34a44 
…rk#3134)

## Relevant issue(s)
Resolves sourcenetwork#3103 

## Description
- Fix the BadRequest in the acp handlers to now be panics
- Make the panics documented under `must` functions
- Add the linter to avoid increasing of undocumented panics
- Ignore linter in existing files outside cli and http (this should
either be resolved gradually over time or in a single PR, regardless
would prefer to not block this PR because of them)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shahzadlone shahzadlone

Labels
area/acp Related to the acp (access control) system code quality Related to improving code quality
Projects
None yet
Milestone
DefraDB v0.15
Development

Successfully merging a pull request may close this issue.

refactor(i): Lint casts & move panics under must funcs shahzadlone/defradb
1 participant
@shahzadlone