Fix: Use Framework Constants for Http Status Codes #3331
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaced numeric HTTP status literals with net/http constants across many handlers, middleware, and documentation examples; added net/http imports where required. No control-flow, payload, or public API signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @stewartmbofana, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant refactoring effort to standardize the way HTTP status codes are handled across multiple Go Fiber examples. By replacing magic number status codes with named constants from the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great initiative to improve code quality by replacing magic numbers for HTTP status codes with constants from the net/http package. This significantly improves readability and maintainability across many files. While reviewing, I found a few critical issues that will cause compilation errors in gorm-mysql/app.go, gorm-postgres/app.go, and gorm/book/book.go. I've also suggested corrections for some semantically incorrect status codes in the auth-docker-postgres-jwt and auth-jwt examples, where server errors (5xx) were being used for client-side issues (4xx). Please see the detailed comments for suggestions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
neo4j/README.md (1)
51-86: Add the missingnet/httpimport.The example now calls
http.StatusInternalServerErrorbut the snippet’s import block never brings in thenet/httppackage, so anyone copying this code will get a compile error. Please include the import alongside the existing ones.websocket/README.md (1)
66-84: Add missingnet/httpimport to the documentation.The code snippet at line 83 uses
http.StatusForbidden, but the imports section (lines 66-72) does not include"net/http". This will cause compilation errors if users copy the example code verbatim.Apply this diff to add the missing import:
import ( "fmt" "log" + "net/http" "github.com/gofiber/fiber/v2" "github.com/gofiber/contrib/websocket" )postgresql/main.go (1)
120-120: Critical: SQL parameter mismatch will cause runtime error.The UPDATE query uses placeholder
$5but only provides 4 parameters. This will fail at runtime with a parameter index error.Apply this diff to fix the parameter placeholder:
- res, err := db.Query("UPDATE employees SET name=$1,salary=$2,age=$3 WHERE id=$5", u.Name, u.Salary, u.Age, u.ID) + res, err := db.Query("UPDATE employees SET name=$1,salary=$2,age=$3 WHERE id=$4", u.Name, u.Salary, u.Age, u.ID)Note: This is a pre-existing bug, not introduced by this PR.
🧹 Nitpick comments (9)
sqlboiler/api/controller/author_controller.go (4)
23-34: Status constants applied correctly.The literal status codes have been successfully replaced with
http.StatusXxxconstants.Consider using
http.StatusBadRequestfor thestrconv.Atoierror on Line 27, as malformed ID parameters are client errors rather than server errors:authorId, err := strconv.Atoi(id) if err != nil { - return c.Status(http.StatusInternalServerError).JSON(err.Error()) + return c.Status(http.StatusBadRequest).JSON(err.Error()) }
36-45: Status constants applied correctly.The literal status codes have been successfully replaced with
http.StatusXxxconstants.Two optional improvements for HTTP semantics:
- Body parsing errors (Line 39) are typically client errors:
if err := c.BodyParser(&author); err != nil { - return c.Status(http.StatusInternalServerError).JSON(err.Error()) + return c.Status(http.StatusBadRequest).JSON(err.Error()) }
- Resource creation (Line 44) should return
201 Created:- return c.Status(http.StatusOK).JSON(author) + return c.Status(http.StatusCreated).JSON(author)
47-61: Status constants applied correctly.The literal status codes have been successfully replaced with
http.StatusXxxconstants.Consider using
http.StatusBadRequestfor thestrconv.Atoierror on Line 51, as malformed ID parameters are client errors:authorId, err := strconv.Atoi(id) if err != nil { - return c.Status(http.StatusInternalServerError).JSON(err.Error()) + return c.Status(http.StatusBadRequest).JSON(err.Error()) }
63-85: Status constants applied correctly.The literal status codes have been successfully replaced with
http.StatusXxxconstants.Consider using
http.StatusBadRequestfor client errors:
- Line 67 (malformed ID parameter):
authorId, err := strconv.Atoi(id) if err != nil { - return c.Status(http.StatusInternalServerError).JSON(err.Error()) + return c.Status(http.StatusBadRequest).JSON(err.Error()) }
- Line 72 (body parsing error):
if err := c.BodyParser(&newAuthor); err != nil { - return c.Status(http.StatusInternalServerError).JSON(err.Error()) + return c.Status(http.StatusBadRequest).JSON(err.Error()) }postgresql/main.go (1)
129-129: Consider using http.StatusOK for update operations.The constant conversion is correct. However,
http.StatusCreated(201) is semantically intended for resource creation (POST), not updates (PUT). PUT operations typically returnhttp.StatusOK(200) orhttp.StatusNoContent(204) on success.Consider this refactor to align with HTTP semantics:
- return c.Status(http.StatusCreated).JSON(u) + return c.Status(http.StatusOK).JSON(u)Note: This semantic issue existed before this PR; flagging now that status codes are explicit.
gorm/book/book.go (2)
36-36: For consistency, replace numeric status code with constant.Line 36 still uses a numeric status code
503. For consistency with this PR's objective of using framework constants, consider replacing it withhttp.StatusServiceUnavailable.- return c.Status(503).SendString(err.Error()) + return c.Status(http.StatusServiceUnavailable).SendString(err.Error())
47-49: Consider using GORM's error handling for more robust checking.Checking
book.Title == ""to determine if a book was found is fragile—an actual book with an empty title would be misidentified as "not found." GORM providesdb.Errorwitherrors.Is(err, gorm.ErrRecordNotFound)for reliable detection.var book Book - db.First(&book, id) - if book.Title == "" { + if err := db.First(&book, id).Error; errors.Is(err, gorm.ErrRecordNotFound) { return c.Status(http.StatusNotFound).SendString("No Book Found with ID") }You'll also need to add
"errors"to your imports:import ( + "errors" "fiber-gorm/database" "net/http"template-asset-bundling/handlers/handlers.go (1)
23-23: LGTM: Idiomatic constant usage improves readability.Replacing the literal
404withhttp.StatusNotFoundmakes the code more self-documenting and aligns with Go best practices.Optional: Fix pre-existing typo in comment.
Line 21 has a typo: "NoutFound" should be "NotFound". While this is pre-existing code (not part of your changes), consider fixing it for consistency.
-// NoutFound renders the 404 view +// NotFound renders the 404 viewgeoip/README.md (1)
50-81: Documentation example should include net/http import.Line 74 correctly uses
http.StatusInternalServerError, but the example code is missing the required import statement.Add the import to make the example self-contained:
package main import ( "log" + "net/http" "github.com/gofiber/fiber/v2" "github.com/oschwald/geoip2-golang" "net" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
404-handler/README.md(1 hunks)404-handler/main.go(2 hunks)auth-docker-postgres-jwt/handler/product.go(4 hunks)auth-docker-postgres-jwt/handler/user.go(6 hunks)auth-jwt/handler/product.go(4 hunks)auth-jwt/handler/user.go(5 hunks)colly-gorm/app/cmd/api/main.go(2 hunks)geoip-maxmind/handlers/handlers.go(2 hunks)geoip/README.md(1 hunks)gorm-mysql/app.go(2 hunks)gorm-mysql/routes/routes.go(4 hunks)gorm-postgres/README.md(1 hunks)gorm-postgres/app.go(2 hunks)gorm-postgres/routes/routes.go(2 hunks)gorm/book/book.go(1 hunks)jwt/handler/product.go(4 hunks)mongodb/main.go(9 hunks)mysql/main.go(6 hunks)neo4j/README.md(1 hunks)neo4j/main.go(2 hunks)oauth2-google/handler/handler.go(2 hunks)optional-parameter/main.go(2 hunks)postgresql/main.go(6 hunks)sqlboiler/api/controller/author_controller.go(2 hunks)sqlboiler/api/controller/post_controller.go(2 hunks)template-asset-bundling/handlers/handlers.go(2 hunks)websocket/README.md(1 hunks)websocket/main.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
sqlboiler/api/controller/author_controller.go (2)
gorm-mysql/routes/routes.go (2)
Delete(61-69)Update(48-58)gorm-postgres/routes/routes.go (2)
Delete(61-70)Update(48-58)
mongodb/main.go (2)
mysql/main.go (1)
Employee(29-34)postgresql/main.go (1)
Employee(29-34)
gorm-mysql/routes/routes.go (2)
gorm/book/book.go (2)
GetBook(24-30)Book(10-15)gorm-postgres/routes/routes.go (2)
Book(37-45)Update(48-58)
gorm-postgres/routes/routes.go (2)
gorm-mysql/routes/routes.go (3)
AllBooks(39-45)Update(48-58)Delete(61-69)gorm/book/book.go (1)
Book(10-15)
sqlboiler/api/controller/post_controller.go (2)
gorm-mysql/routes/routes.go (2)
Delete(61-69)Update(48-58)gorm-postgres/routes/routes.go (2)
Delete(61-70)Update(48-58)
🔇 Additional comments (34)
websocket/main.go (2)
10-10: LGTM!The
net/httpimport is correctly added to support the use ofhttp.StatusForbidden.
25-25: LGTM!Replacing the literal
403withhttp.StatusForbiddenimproves code readability and maintainability while preserving the same behavior.sqlboiler/api/controller/author_controller.go (2)
5-5: LGTM! Import added correctly.The
net/httpimport is correctly added to support the HTTP status constants used throughout the file.
15-21: LGTM! Status constants applied correctly.The HTTP status codes have been correctly replaced with
http.StatusInternalServerErrorandhttp.StatusOK.postgresql/main.go (5)
10-10: LGTM: Import added correctly.The
net/httpimport is properly added to support the HTTP status code constants used throughout the file.
68-68: LGTM: Correct status constant for database errors.The conversion from
500tohttp.StatusInternalServerErroris appropriate for database query failures.
93-93: LGTM: Appropriate status for body parsing errors.The conversion from
400tohttp.StatusBadRequestcorrectly indicates invalid request body format.
116-116: LGTM: Consistent error handling across handlers.The conversion from
400tohttp.StatusBadRequestmaintains consistency with the POST and DELETE handlers' body parsing error responses.
139-139: LGTM: Proper error status for invalid input.The conversion from
400tohttp.StatusBadRequestis appropriate and maintains consistency across all handler error responses.sqlboiler/api/controller/post_controller.go (2)
5-5: LGTM!The import of
net/httpis correctly added to support the HTTP status constants used throughout the file.
15-21: LGTM!The refactoring correctly replaces numeric status codes with constants. The status codes are semantically appropriate:
StatusInternalServerErrorfor database query failures andStatusOKfor successful responses.template-asset-bundling/handlers/handlers.go (1)
4-5: LGTM: Import addition follows Go conventions.The
net/httpimport is correctly added and properly formatted to support the HTTP status constant usage.optional-parameter/main.go (1)
9-9: LGTM! Clean migration to status constants.The import and usage of
http.StatusNotFoundcorrectly replaces the hard-coded 404, improving code maintainability.Also applies to: 26-26
oauth2-google/handler/handler.go (1)
5-5: LGTM! Correct status constant usage.The addition of the
net/httpimport and replacement of the hard-coded 200 withhttp.StatusOKaligns with best practices.Also applies to: 24-24
neo4j/main.go (1)
6-6: LGTM! Appropriate status constant for bad request.The
http.StatusBadRequestconstant correctly replaces the literal 400 in the error handling path.Also applies to: 56-56
colly-gorm/app/cmd/api/main.go (1)
5-5: LGTM! Consistent status constant usage across handlers.The
net/httpimport and usage ofhttp.StatusOKacross all three success response paths is correct and improves code consistency.Also applies to: 38-38, 46-46, 54-54
geoip-maxmind/handlers/handlers.go (1)
6-6: LGTM! Correct status constant for validation error.The
http.StatusBadRequestconstant appropriately replaces the literal 400 for the invalid IP address error case.Also applies to: 46-46
gorm-mysql/routes/routes.go (1)
4-4: LGTM! Comprehensive and consistent status constant migration.All handlers correctly use
http.StatusBadRequestfor validation errors andhttp.StatusOKfor successful responses. The migration is thorough and maintains consistency across the entire routes package.Also applies to: 22-22, 27-27, 35-35, 44-44, 51-51, 57-57, 68-68
gorm-postgres/routes/routes.go (1)
4-69: LGTM! HTTP status constants correctly applied.The migration from literal status codes to
net/httpconstants improves maintainability and readability across all handlers (AddBook, AllBooks, Book, Update, Delete). The import is properly added, and all mappings are correct.mysql/main.go (1)
10-140: HTTP status constants correctly applied, but note semantic issue.The migration to
net/httpconstants is correct throughout. However, Line 130 useshttp.StatusCreated(201) for a PUT/Update operation. While this change preserves existing behavior, 201 Created is semantically intended for POST/create operations; PUT/update typically returns 200 OK or 204 No Content.Consider whether Line 130 should use
http.StatusOKinstead:- return c.Status(http.StatusCreated).JSON(u) + return c.Status(http.StatusOK).JSON(u)jwt/handler/product.go (1)
6-50: HTTP status constants correctly applied.The migration to
net/httpconstants is correct. Note that Line 36 useshttp.StatusInternalServerError(500) for body parsing errors, which preserves existing behavior but typically 400 Bad Request is more semantically appropriate for client input validation failures.auth-jwt/handler/product.go (1)
6-50: HTTP status constants correctly applied.The migration to
net/httpconstants is correct. Line 36 useshttp.StatusInternalServerError(500) for body parsing errors, preserving existing behavior, though 400 Bad Request is typically more semantically appropriate for client input errors.mongodb/main.go (1)
9-219: LGTM! Comprehensive and correct status constant migration.All HTTP status code replacements are correct and semantically appropriate:
- 500 →
http.StatusInternalServerErrorfor server errors- 400 →
http.StatusBadRequestfor client input errors- 201 →
http.StatusCreatedfor POST operations- 200 →
http.StatusOKfor successful updates- 404 →
http.StatusNotFoundfor missing resources- 204 →
http.StatusNoContentfor successful deletionsauth-docker-postgres-jwt/handler/product.go (1)
6-50: HTTP status constants correctly applied.The migration to
net/httpconstants is correct. Line 36 useshttp.StatusInternalServerError(500) for body parsing errors, which preserves existing behavior, though 400 Bad Request would be more semantically appropriate for client input validation failures.auth-docker-postgres-jwt/handler/user.go (4)
4-4: LGTM! Import added correctly.The
net/httpimport is necessary for using standard HTTP status constants.
53-53: LGTM! Correct status code for missing resource.Using
http.StatusNotFoundfor a missing user is semantically correct.
84-84: LGTM! Correct status code for server-side error.Using
http.StatusInternalServerErrorfor password hashing failures is appropriate, as this represents a server-side cryptographic operation error.
89-89: LGTM! Correct status code for database error.Using
http.StatusInternalServerErrorfor database creation failures is appropriate, as this represents a server-side persistence error.auth-jwt/handler/user.go (3)
4-4: LGTM! Import added correctly.The
net/httpimport is necessary for using standard HTTP status constants.
52-52: LGTM! Correct status code for missing resource.Using
http.StatusNotFoundfor a missing user is semantically correct.
72-72: LGTM! Correct status codes for server-side errors.Using
http.StatusInternalServerErrorfor password hashing and database creation failures is appropriate.Also applies to: 77-77
404-handler/main.go (2)
9-9: LGTM! Import added correctly.The
net/httpimport is necessary for using standard HTTP status constants.
23-23: LGTM! Correct status code for 404 handler.Using
http.StatusNotFoundin the 404 handler is semantically correct and improves code readability.404-handler/README.md (1)
61-61: LGTM! Documentation updated to match code changes.The README correctly reflects the updated code example using
http.StatusNotFoundinstead of the literal 404.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gorm/book/book.go (1)
37-37: Replace literal status code and use semantically correct status.This line still uses a literal status code (503), which is inconsistent with the PR objective to replace all literals with framework constants. Additionally, 503 Service Unavailable is semantically incorrect for a body parsing error—parsing failures are client errors that should return 400 Bad Request.
Apply this diff:
- return c.Status(503).SendString(err.Error()) + return c.Status(http.StatusBadRequest).SendString(err.Error())
♻️ Duplicate comments (3)
auth-docker-postgres-jwt/handler/user.go (1)
133-133: BodyParser failure should return 400 Bad Request, not 500.Line 133 returns
http.StatusInternalServerError(500) for BodyParser errors, which is inconsistent with lines 70 and 107 that correctly returnhttp.StatusBadRequest(400). BodyParser failures indicate malformed client input and should return 400, not 500.Apply this diff to fix the issue:
- return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"status": "error", "message": "Review your input", "errors": err.Error()}) + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"status": "error", "message": "Review your input", "errors": err.Error()})auth-jwt/handler/user.go (2)
121-121: Pre-existing issue: Body parsing errors should return 400 Bad Request.Body parsing failures indicate malformed client input and should return
http.StatusBadRequest(400) instead ofhttp.StatusInternalServerError(500).This is a pre-existing issue already flagged in past review comments.
131-131: Pre-existing issue: Invalid credentials should return 401 Unauthorized.The
validUserfunction validates the user's password, which is an authentication check. Failed authentication should returnhttp.StatusUnauthorized(401) instead ofhttp.StatusForbidden(403).This is a pre-existing issue already flagged in past review comments.
🧹 Nitpick comments (3)
auth-docker-postgres-jwt/handler/user.go (3)
70-89: Consider usingfiber.Status*constants for consistency.The status codes are semantically correct, and line 70 now properly returns 400 for BodyParser errors (addressing a past review comment). However, the file mixes
http.Status*(lines 70, 84, 89) andfiber.Status*(lines 75, 79) constants.For consistency within this codebase that uses the Fiber framework, consider using
fiber.StatusBadRequestandfiber.StatusInternalServerErrorinstead of thehttppackage equivalents.Apply this diff for consistency:
- return c.Status(http.StatusBadRequest).JSON(fiber.Map{"status": "error", "message": "Review your input", "errors": err.Error()}) + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"status": "error", "message": "Review your input", "errors": err.Error()})- return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"status": "error", "message": "Couldn't hash password", "errors": err.Error()}) + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"status": "error", "message": "Couldn't hash password", "errors": err.Error()})- return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"status": "error", "message": "Couldn't create user", "errors": err.Error()}) + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"status": "error", "message": "Couldn't create user", "errors": err.Error()})
107-113: Consider usingfiber.Status*constants for consistency.The status codes are semantically appropriate:
- Line 107: Correctly returns 400 for BodyParser errors (addressing a past review comment).
- Line 113: Returns 403 Forbidden for invalid token ID, which is appropriate when the token is valid but the user is not authorized to update this resource.
However, for consistency with the rest of the codebase, consider using
fiber.StatusBadRequestandfiber.StatusForbiddeninstead ofhttppackage equivalents.Apply this diff for consistency:
- return c.Status(http.StatusBadRequest).JSON(fiber.Map{"status": "error", "message": "Review your input", "errors": err.Error()}) + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"status": "error", "message": "Review your input", "errors": err.Error()})- return c.Status(http.StatusForbidden).JSON(fiber.Map{"status": "error", "message": "Invalid token id", "data": nil}) + return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"status": "error", "message": "Invalid token id", "data": nil})
139-143: Consider usingfiber.Status*constants for consistency.The status codes are semantically correct:
- Line 139: Returns 403 Forbidden for invalid token ID.
- Line 143: Returns 401 Unauthorized for authentication failure (addressing a past review comment).
However, for consistency with the rest of the codebase, consider using
fiber.StatusForbiddenandfiber.StatusUnauthorizedconsistently. Note that line 143 already usesfiber.StatusUnauthorized, which is good.Apply this diff for consistency:
- return c.Status(http.StatusForbidden).JSON(fiber.Map{"status": "error", "message": "Invalid token id", "data": nil}) + return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"status": "error", "message": "Invalid token id", "data": nil})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
auth-docker-postgres-jwt/handler/user.go(6 hunks)auth-jwt/handler/user.go(5 hunks)gorm-mysql/app.go(2 hunks)gorm-postgres/app.go(2 hunks)gorm/book/book.go(2 hunks)sqlboiler/api/controller/post_controller.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- gorm-postgres/app.go
- sqlboiler/api/controller/post_controller.go
- gorm-mysql/app.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-23T19:54:01.389Z
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/handlers/books.go:28-43
Timestamp: 2024-11-23T19:54:01.389Z
Learning: In 'clean-code/app/server/handlers/books.go', validation is out of scope and should not be suggested.
Applied to files:
gorm/book/book.go
🔇 Additional comments (9)
gorm/book/book.go (1)
5-5: LGTM! Previous review feedback addressed.The addition of the
net/httpimport and the use ofhttp.StatusNotFoundcorrectly address the compilation error and semantic issues raised in previous reviews. A 404 status is the appropriate response for a missing resource.Also applies to: 50-50
auth-docker-postgres-jwt/handler/user.go (2)
4-4: LGTM!The
net/httpimport is necessary for using the HTTP status constants introduced in this file.
53-53: LGTM!Correctly replaced literal
404withhttp.StatusNotFound.auth-jwt/handler/user.go (6)
4-4: Import addition is correct.The
net/httpimport is necessary to use HTTP status constants throughout the file.
52-52: Correct status code for missing user.Using
http.StatusNotFound(404) is semantically appropriate when a user is not found.
67-77: Status codes are semantically correct.All three status codes are appropriate:
- Line 67:
StatusBadRequest(400) for body parsing errors- Line 72:
StatusInternalServerError(500) for password hashing failures- Line 77:
StatusInternalServerError(500) for database creation failures
95-95: Correct status code for body parsing error.Using
http.StatusBadRequest(400) is appropriate for malformed request input.
101-101: Mechanical replacement is correct; 403 is semantically appropriate.The status code
http.StatusForbidden(403) is appropriate here. The user is authenticated (JWT passed middleware) but is attempting to modify another user's account, which is an authorization failure rather than an authentication failure.Past comments suggested using 401 Unauthorized, but 403 Forbidden is semantically correct for this authorization check scenario.
127-127: Mechanical replacement is correct; 403 is semantically appropriate.Same as line 101:
http.StatusForbidden(403) is appropriate for this authorization check where an authenticated user attempts to delete another user's account.
1 participant
Changes:
Summary by CodeRabbit