1. upgrade golangci-lint 2. optimize some code #45
Conversation
WalkthroughUpdates CI and linter configs, upgrades Go to 1.24 and many dependencies, adds new public option constructors in the timeout package, refactors timeout header propagation via a copy helper and composite literal initialization, and performs minor import/order cleanups and test import adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant G as Gin Router
participant T as Timeout Middleware
participant H as Handler
C->>G: HTTP Request
G->>T: Invoke middleware
rect rgb(240,248,255)
note right of T: Apply options (error code, default msg, content-type, callbacks)
T->>H: Forward request
alt handler completes before timeout
H-->>T: Response (status, headers, body)
T->>T: copyHeaders(dst, src)
T-->>G: Write status/headers/body
T->>T: invoke CallBack/GinCtxCallBack (if set)
else timeout occurs
T->>T: Prepare default timeout response
T-->>G: Write timeout status/headers/body
T->>T: invoke CallBack/GinCtxCallBack (if set)
end
end
G-->>C: HTTP Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
timeout.go (1)
95-99: Bug: setting Content-Type after WriteHeader loses the header on real servers.net/http sends headers when WriteHeader is called; setting headers afterward has no effect. Move the header set before WriteHeader.
- tw.ResponseWriter.WriteHeader(tw.Response.GetCode(&cp)) - - tw.ResponseWriter.Header().Set("Content-Type", tw.Response.GetContentType(&cp)) - n, err = tw.ResponseWriter.Write(encodeBytes(tw.Response.GetContent(&cp))) + // set headers first + tw.ResponseWriter.Header().Set("Content-Type", tw.Response.GetContentType(&cp)) + // then write status and body + tw.ResponseWriter.WriteHeader(tw.Response.GetCode(&cp)) + n, err = tw.ResponseWriter.Write(encodeBytes(tw.Response.GetContent(&cp)))options.go (1)
28-53: Avoid mutating the shared default responseEach of the new option helpers assigns
defaultResponsedirectly tot.Responsebefore mutating it. BecausedefaultResponseis the global fallback instance, callingSetCode/SetContent/SetContentTypehere changes the shared object, so later middleware instances (even those that never opted in) inherit the mutated code/content. Under load, concurrent requests can race on the same instance and leak one user’s timeout payload into another’s response. Please make sure these setters operate on a fresh copy—e.g. clone or instantiate a new default response per writer—before applying user overrides.
🧹 Nitpick comments (5)
.github/workflows/golang-ci.yml (2)
26-26: Pin to patch and fetch latest 1.24 automatically.Use 1.24.x with check-latest to keep CI on the latest security patch.
- go-version: 1.24 + go-version: 1.24.x + check-latest: trueAlso consider actions/setup-go@v5 for built‑in caching.
35-35: Good upgrade; consider aligning module mode and adding a version check.
- Lint runs with modules-download-mode=mod while tests use vendor. Aligning these avoids “works in one job, fails in another.”
- Add a quick version step to surface the linter version in logs.
- name: golangci-lint version run: golangci-lint --versionOptionally switch lint to vendor mode if you intend to keep vendor in CI.
timeout.go (2)
118-119: Header copy semantics: avoid slice aliasing and ensure canonical behavior.Assigning slices (dst[k] = vv) aliases backing arrays and may overwrite pre‑set multi‑value headers unexpectedly. Prefer Del+Add or clone values.
- copyHeaders(tw.ResponseWriter.Header(), tw.Header()) + copyHeaders(tw.ResponseWriter.Header(), tw.Header())And update copyHeaders below:
-func copyHeaders(dst, src http.Header) { - for k, vv := range src { - dst[k] = vv - } -} +func copyHeaders(dst, src http.Header) { + for k, vv := range src { + dst.Del(k) + for _, v := range vv { + dst.Add(k, v) + } + } +}
109-111: Which context should callbacks see?You invoke GinCtxCallBack with the original c (already Aborted), not the cp passed through the remaining handlers. Confirm this is intentional; some users may expect request-scoped values set by later handlers.
If you want callbacks to observe the handler-modified context, pass &cp instead.
- if tw.GinCtxCallBack != nil { - tw.GinCtxCallBack(c) - } + if tw.GinCtxCallBack != nil { + tw.GinCtxCallBack(&cp) + }.golangci.yml (1)
24-29: Path exclusions: minor cleanup.You exclude both example and examples$. Consider consolidating to a single regex (e.g., ^examples?$) to avoid drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/golang-ci.yml(2 hunks).golangci.yml(1 hunks)go.mod(1 hunks)options.go(5 hunks)response.go(1 hunks)timeout.go(4 hunks)timeout_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
timeout.go (2)
writer.go (1)
TimeoutWriter(11-24)options.go (1)
TimeoutOptions(13-18)
options.go (1)
writer.go (1)
TimeoutWriter(11-24)
🔇 Additional comments (3)
response.go (1)
5-5: Import reshuffle only — LGTM.timeout_test.go (1)
5-6: Test imports update — LGTM..golangci.yml (1)
1-1: Approve .golangci.yml v2 config — keys match v2.4.0 spec
Alllinters.*,linters.settings.govet.*,exclusions.*, andformatters.exclusions.*keys align with the official golangci-lint v2.4.0 documentation.
Summary by CodeRabbit
New Features
Refactor
Chores