🔥 feat(limiter): Add ExpirationFunc for dynamic expiration #3984
Conversation
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Limiter as Limiter Middleware
participant ExpFunc as ExpirationFunc
participant Store as Backend Store
participant App as App Handler
Client->>Limiter: HTTP request
Limiter->>ExpFunc: call cfg.ExpirationFunc(c)
ExpFunc-->>Limiter: expirationDuration
Limiter->>Store: read/update rate state (use expirationDuration for TTL)
Store-->>Limiter: state (count, window, TTL)
alt allowed
Limiter->>App: forward request
App-->>Client: 200 OK
else blocked
Limiter-->>Client: 429 Too Many Requests
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
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
📜 Recent review detailsConfiguration used: Repository 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 |
Summary of ChangesHello @ErfanMomeniii, 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 enhances the Limiter middleware by introducing a flexible Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 introduces a new ExpirationFunc to the limiter middleware, allowing for dynamic expiration times per request. The implementation correctly mirrors the existing MaxFunc pattern, and the changes are well-contained across the configuration, middleware logic, and documentation. The new tests are also comprehensive. I have one suggestion to improve a code comment for better clarity and maintainability.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3984 +/- ##
==========================================
- Coverage 91.60% 91.11% -0.50%
==========================================
Files 119 119
Lines 10262 10859 +597
==========================================
+ Hits 9401 9894 +493
- Misses 544 610 +66
- Partials 317 355 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ 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 adds an ExpirationFunc configuration option to the Limiter middleware, enabling dynamic expiration duration per request based on request context. This mirrors the existing MaxFunc pattern for consistency and allows more flexible rate-limiting strategies.
Key changes:
- Added
ExpirationFuncfield to Config struct with appropriate defaults - Updated both FixedWindow and SlidingWindow implementations to use dynamic expiration
- Added comprehensive tests for both limiter types with the new functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| middleware/limiter/config.go | Added ExpirationFunc field with default implementation and configuration logic |
| middleware/limiter/limiter_fixed.go | Updated to calculate expiration per-request and use it in storage operations |
| middleware/limiter/limiter_sliding.go | Updated to calculate expiration per-request for sliding window algorithm |
| middleware/limiter/limiter_test.go | Added tests validating ExpirationFunc overrides static Expiration for both window types |
| docs/middleware/limiter.md | Added documentation for ExpirationFunc field and usage example |
| return 20 | ||
| }, | ||
| Expiration: 30 * time.Second, | ||
| ExpirationFunc: func(c fiber.Ctx) time.Duration { |
There was a problem hiding this comment.
The ExpirationFunc in this example is redundant because it returns the same value as the static Expiration field above it (30 * time.Second). For a more useful demonstration, consider showing either a different value or a dynamic calculation based on the request context, similar to the dynamic expiration example below.
| ExpirationFunc: func(c fiber.Ctx) time.Duration { | |
| ExpirationFunc: func(c fiber.Ctx) time.Duration { | |
| if c.Path() == "/login" { | |
| return 60 * time.Second | |
| } |
|
@codex review |
|
@ErfanMomeniii Run |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dc53410ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
middleware/limiter/limiter_fixed.gomiddleware/limiter/limiter_sliding.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Prefer
github.com/gofiber/utils/v2helpers (for example,utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
Files:
middleware/limiter/limiter_fixed.gomiddleware/limiter/limiter_sliding.go
🔇 Additional comments (4)
middleware/limiter/limiter_sliding.go (2)
24-25: Good refactoring to enable dynamic expiration.Removing the static expiration initialization in favor of per-request computation is the correct approach to support the new
ExpirationFuncfeature.
43-45: Excellent implementation of per-request expiration.The pattern correctly computes
expirationDurationonce per request and reuses it consistently throughout the request lifecycle, including in the post-response path. This ensures the same expiration window applies to all operations within a single request, which is the correct behavior.Also applies to: 66-66, 73-73, 93-93, 134-134, 138-138, 145-145
middleware/limiter/limiter_fixed.go (2)
22-23: Good refactoring to enable dynamic expiration.Consistent with the sliding window implementation, removing the static expiration initialization is the correct approach for supporting per-request dynamic expiration.
41-43: Well-implemented per-request expiration for fixed window.The implementation correctly follows the same pattern as the sliding window and
MaxFunc, computing the expiration once per request and using it consistently throughout. The fixed window logic properly applies the dynamic expiration.Also applies to: 65-65, 69-69, 82-82, 122-122
| // Generate expiration from generator | ||
| expirationDuration := cfg.ExpirationFunc(c) | ||
| expiration := uint64(expirationDuration.Seconds()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Same duration validation needed here.
The same conversion safety concerns from limiter_sliding.go apply here. Please add validation to ensure ExpirationFunc returns a positive, reasonable duration before converting to uint64.
🔎 Suggested validation
// Generate expiration from generator
expirationDuration := cfg.ExpirationFunc(c)
+if expirationDuration <= 0 {
+ return fmt.Errorf("limiter: ExpirationFunc must return a positive duration, got %v", expirationDuration)
+}
expiration := uint64(expirationDuration.Seconds())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Generate expiration from generator | |
| expirationDuration := cfg.ExpirationFunc(c) | |
| expiration := uint64(expirationDuration.Seconds()) | |
| // Generate expiration from generator | |
| expirationDuration := cfg.ExpirationFunc(c) | |
| if expirationDuration <= 0 { | |
| return fmt.Errorf("limiter: ExpirationFunc must return a positive duration, got %v", expirationDuration) | |
| } | |
| expiration := uint64(expirationDuration.Seconds()) |
🤖 Prompt for AI Agents
In middleware/limiter/limiter_fixed.go around lines 41-43, validate the duration
returned by cfg.ExpirationFunc(c) before converting to uint64: ensure the
duration is positive (>0) and clamp it to a safe upper bound to avoid overflow
when calling Seconds(); if the duration is <=0, set a sensible default (e.g.,
1s) and log or warn; if Seconds() exceeds the max uint64 value, clamp to
math.MaxUint64 (or a defined max) before casting to uint64. Implement these
checks immediately after calling ExpirationFunc and use the validated/clamped
value for expiration.
|
|
@ErfanMomeniii thx for this feature |
Done @ReneWerner87 |
|
I have added tests to increase coverage. |
3 participants
Description
Add ExpirationFunc configuration option to the Limiter middleware, enabling dynamic expiration duration per request. This mirrors the existing MaxFunc pattern and allows users to apply different rate-limiting windows based on request context.
Fixes (#3749)
Changes introduced
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
Type of change
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.