chore: simplify, modernize (Go 1.26), update deps#84
Conversation
- Fix mutex left locked when CreateListener fails in Serve (deadlock on Stop) - Use ShutdownWithContext(ctx) in Stop so the shutdown respects caller deadline - Call Config.Valid() in Init to surface misconfiguration early - Use := for inner err in Serve goroutine to avoid closing over the outer var - Merge endCh+errCh into a single chan error in Stop - Remove 7 explicit zero-value fields from fiber.Config (noise) - Use range-over-value (Go 1.22) in Serve and Config.Valid
|
Warning Review limit reached
More reviews will be available in 17 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #84 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 2 2
Lines 88 76 -12
======================================
+ Misses 88 76 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR modernizes and simplifies the fileserver plugin while addressing shutdown and configuration validation behavior.
Changes:
- Calls
Config.Valid()during plugin initialization. - Simplifies Fiber config setup and static route iteration.
- Updates shutdown handling to use
ShutdownWithContext(ctx)and consolidates stop channels.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
plugin.go |
Adds config validation, simplifies Fiber setup, fixes listener error unlock path, and updates shutdown handling. |
config.go |
Simplifies iteration over serve configurations while preserving default value assignment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The nil check passed an explicitly empty serve: [] slice, allowing Init to accept a fileserver config with no routes despite the schema requiring minItems: 1. Use len() to cover both nil and empty slices.
Applied fixes
Bug fixes (R)
plugin.go:90— Mutex was left locked whenCreateListenerfails;Stopwould deadlock. Fixed by callingp.Unlock()on the error path before returning.plugin.go:117—Stopcalledapp.Shutdown()which ignores the passedctxdeadline. Replaced withapp.ShutdownWithContext(ctx).plugin.go:36—Config.Valid()was defined but never called inInit; invalid config (empty address, missing serve entries) would silently produce a broken server. Now called afterUnmarshalKey.plugin.go:99—err = p.app.Listener(ln)reused the outererrvia closure; replaced with:=scoped to the goroutine.Simplify (S)
plugin.go:57— Removed 7 explicit zero-value fields fromfiber.Config(noise).plugin.go:80— Index range over[]*Cfgreplaced with range-over-value.plugin.go:110— MergedendCh+errChinto a singlechan errorinStop.config.go:44— Index range over[]*Cfgreplaced with range-over-value.Modern Go (M)
plugin.go:80,config.go:44— Go 1.22 range-over-element (same as S items above).Not applied (needs manual review)
None — all 10 findings were applied.
Deps
go get -u all && go mod tidy— all dependencies were already at latest; no version changes in go.mod.Verification
go build ./...— cleango vet ./...— cleangolangci-lint run— 0 issues