Fix review follow-ups and add semantic release tagging#13
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request introduces release workflow automation with manual dispatch versioning, enhances JSON serialization for problem details and response metadata, improves request body size error handling with proper HTTP status codes, updates Go toolchain versions and dependencies across examples, adds pagination logic with comprehensive testing, and improves validation field name mapping based on JSON tags. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
examples/restapi/main_test.go (1)
22-30: Nit:tt := ttno longer needed on Go 1.22+.Since the module targets
go 1.25, loop-variable rebinding can be dropped. Harmless either way.Optional cleanup
for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/restapi/main_test.go` around lines 22 - 30, The test loop rebinds the loop variable with "tt := tt" which is unnecessary on Go 1.22+; in the table-driven test that calls t.Run and invokes clampPageWindow, remove the redundant "tt := tt" line so the loop variable "tt" from the range over tests is used directly inside the t.Run closure.examples/chi/go.mod (1)
3-3: Drop the patch version from thegodirective.Prefer
go 1.25overgo 1.25.0for toolchain flexibility. Same applies toexamples/restapi/go.modandvalidation/playground/go.mod.Proposed fix
-go 1.25.0 +go 1.25Based on learnings: "In go.mod files, the Go version should typically be specified with just major and minor version numbers (e.g.,
go 1.23) rather than including the patch version for better flexibility."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/chi/go.mod` at line 3, Change the go directive to drop the patch version: replace the line starting with "go 1.25.0" (the go directive) with "go 1.25" in the affected go.mod files (also update similar lines in examples/restapi/go.mod and validation/playground/go.mod) so the go directive uses only major.minor; keep spacing and file formatting unchanged.response_write.go (1)
86-87: Tweak the write-failure log message.Line 87 now handles
w.Write(...)errors, not JSON encoding errors, so the log text should reflect a response-body write failure.Suggested log message adjustment
- if _, err := w.Write(buffer.Bytes()); err != nil { - log.Printf("Failed to encode problem details: %v", err) + if _, err := w.Write(buffer.Bytes()); err != nil { + log.Printf("Failed to write problem details response body: %v", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@response_write.go` around lines 86 - 87, The log message incorrectly refers to JSON encoding when the error comes from writing the response body; update the log in the block that calls w.Write(buffer.Bytes()) (in response_write.go, around the anonymous write error check) to say something like "Failed to write response body" and include the error value so it reflects a response-body write failure rather than an encoding error.request_internal.go (1)
48-55: Consider a dedicated problem type for 413.The status and title now correctly say
413 Payload Too Large, butTypeURL("bad_request_error")still points clients at the generic bad-request problem type. RFC 9457 problem types are supposed to identify the problem category, so apayload_too_large(orrequest_entity_too_large) key — falling back tobad_request_errorif not configured — would be more accurate and would letProblemConfigconsumers give these responses their own documentation URL.Not a blocker; mentioning because the title and type are diverging now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@request_internal.go` around lines 48 - 55, The handler for BodyDecodeErrorBodyTooLarge currently returns a 413 title but still uses problems.TypeURL("bad_request_error"); update the TypeURL argument in the BodyDecodeErrorBodyTooLarge branch (where NewProblemDetails is called) to use a dedicated key such as "payload_too_large" or "request_entity_too_large" (falling back to "bad_request_error" via your existing ProblemConfig lookup if missing) so clients and ProblemConfig consumers can map this 413 to its own documentation URL rather than the generic bad_request_error.request_decode.go (1)
102-105: Redundant wrapped error on the "trailing content succeeded" branch.When the second
Decodesucceeds, you return aBodyDecodeError{Kind: MultipleDocuments, Err: errors.New(...)}. But(*BodyDecodeError).Error()forBodyDecodeErrorMultipleDocumentsreturns a fixed string and never readsErr, and exposing that syntheticerrors.New(...)throughUnwrap()gives callers nothing useful to match on (it’s not a sentinel). Either dropErr, or keep it consistent with the other two branches that do propagate the underlying decoder error.🔧 Suggested tweak
- return request, &BodyDecodeError{ - Kind: BodyDecodeErrorMultipleDocuments, - Err: errors.New("request body must contain a single JSON document"), - } + return request, &BodyDecodeError{Kind: BodyDecodeErrorMultipleDocuments}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@request_decode.go` around lines 102 - 105, The multiple-documents branch currently returns &BodyDecodeError{Kind: BodyDecodeErrorMultipleDocuments, Err: errors.New(...)} even though BodyDecodeError.Error() ignores Err; remove the synthetic wrapped error and return &BodyDecodeError{Kind: BodyDecodeErrorMultipleDocuments} (i.e., drop Err) so Unwrap() and callers aren't misled, or alternatively propagate the actual decoder error similar to other branches—update the code path that constructs the error in the second Decode success branch and ensure consistency with how BodyDecodeError is constructed elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 88-123: Enable strict shell failure (set -euo pipefail) at the top
of the step, restrict/validate the starting tag selected by latest_tag by using
the git tag list pattern for semver (e.g., only tags matching
v[0-9]*.[0-9]*.[0-9]*) or validate latest_tag against a regex before parsing,
ensure the IFS='.' read into major minor patch yields numeric values (fallback
to 0 or fail with a clear message if not), compute tag_name as before, then
verify the computed tag_name does not already exist locally or remotely (fail
with a clear error if it does) before running git tag "$tag_name" and git push
origin "$tag_name"; surface any failures via non-zero exit so the step fails
deterministically.
- Around line 125-136: The github-release job is currently skipped on tag pushes
because it declares needs: tag-release (which only runs for workflow_dispatch),
so remove tag-release from the job's needs (keep only verify) and change the
job-level if to require verify succeeded and either the event is a tag push or a
workflow_dispatch (e.g. if: needs.verify.result == 'success' &&
(startsWith(github.ref, 'refs/tags/v') || github.event_name ==
'workflow_dispatch')), and update the tag_name input expression on the Create
GitHub release step to conditionally use the tag-release output only when
available (use the tag-release output when workflow_dispatch, otherwise fall
back to github.ref_name).
---
Nitpick comments:
In `@examples/chi/go.mod`:
- Line 3: Change the go directive to drop the patch version: replace the line
starting with "go 1.25.0" (the go directive) with "go 1.25" in the affected
go.mod files (also update similar lines in examples/restapi/go.mod and
validation/playground/go.mod) so the go directive uses only major.minor; keep
spacing and file formatting unchanged.
In `@examples/restapi/main_test.go`:
- Around line 22-30: The test loop rebinds the loop variable with "tt := tt"
which is unnecessary on Go 1.22+; in the table-driven test that calls t.Run and
invokes clampPageWindow, remove the redundant "tt := tt" line so the loop
variable "tt" from the range over tests is used directly inside the t.Run
closure.
In `@request_decode.go`:
- Around line 102-105: The multiple-documents branch currently returns
&BodyDecodeError{Kind: BodyDecodeErrorMultipleDocuments, Err: errors.New(...)}
even though BodyDecodeError.Error() ignores Err; remove the synthetic wrapped
error and return &BodyDecodeError{Kind: BodyDecodeErrorMultipleDocuments} (i.e.,
drop Err) so Unwrap() and callers aren't misled, or alternatively propagate the
actual decoder error similar to other branches—update the code path that
constructs the error in the second Decode success branch and ensure consistency
with how BodyDecodeError is constructed elsewhere.
In `@request_internal.go`:
- Around line 48-55: The handler for BodyDecodeErrorBodyTooLarge currently
returns a 413 title but still uses problems.TypeURL("bad_request_error"); update
the TypeURL argument in the BodyDecodeErrorBodyTooLarge branch (where
NewProblemDetails is called) to use a dedicated key such as "payload_too_large"
or "request_entity_too_large" (falling back to "bad_request_error" via your
existing ProblemConfig lookup if missing) so clients and ProblemConfig consumers
can map this 413 to its own documentation URL rather than the generic
bad_request_error.
In `@response_write.go`:
- Around line 86-87: The log message incorrectly refers to JSON encoding when
the error comes from writing the response body; update the log in the block that
calls w.Write(buffer.Bytes()) (in response_write.go, around the anonymous write
error check) to say something like "Failed to write response body" and include
the error value so it reflects a response-body write failure rather than an
encoding error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a76e0253-27c3-408d-a91a-dee250990b6b
⛔ Files ignored due to path filters (5)
examples/chi/go.sumis excluded by!**/*.sumexamples/restapi/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumvalidation/playground/go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
.github/workflows/release.ymlREADME.mdexamples/chi/go.modexamples/restapi/go.modexamples/restapi/main.goexamples/restapi/main_test.goexamples/stdmux/main.goproblem_config.goproblem_details.goproblem_details_test.gorequest_decode.gorequest_decode_test.gorequest_internal.gorequest_test.goresponse_builder.goresponse_builder_test.goresponse_meta.goresponse_meta_test.goresponse_test.goresponse_write.govalidation/playground/go.modvalidation/playground/validator.govalidation/playground/validator_test.go
1 participant
Summary
major,minor, orpatchtag automaticallyVerification
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Documentation