fix up timeout middleware
Description
Fixed timeout middleware
Needs to be checked
Fixes #3394
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.
- [ ] Benchmarks: Describe any performance benchmarks and improvements related to the changes.
- [ ] Documentation Update: Detail the updates made to the documentation and links to the changed files.
- [ ] Changelog/What's New: Include a summary of the additions for the upcoming release notes.
- [ ] Migration Guide: If necessary, provide a guide or steps for users to migrate their existing code to accommodate these changes.
- [ ] API Alignment with Express: Explain how the changes align with the Express API.
- [ ] API Longevity: Discuss the steps taken to ensure that the new or updated APIs are consistent and not prone to breaking changes.
- [ ] Examples: Provide examples demonstrating the new features or changes in action.
Type of change
Please delete options that are not relevant.
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Enhancement (improvement to existing features and functionality)
- [ ] Documentation update (changes to documentation)
- [ ] Performance improvement (non-breaking change which improves efficiency)
- [ ] Code consistency (non-breaking change which improves code reliability and robustness)
Checklist
Before you submit your pull request, please make sure you meet these requirements:
- [ ] Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
- [ ] Conducted a self-review of the code and provided comments for complex or critical parts.
- [ ] Updated the documentation in the
/docs/directory for Fiber's documentation. - [ ] Added or updated unit tests to validate the effectiveness of the changes or new features.
- [ ] Ensured that new and existing unit tests pass locally with the changes.
- [ ] Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
- [ ] Aimed for optimal performance with minimal allocations in the new code.
- [ ] Provided benchmarks for the new code to analyze and improve upon.
Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md
[!WARNING]
Rate limit exceeded
@pjebs has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 15 seconds before requesting another review.
⌛ How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the
@coderabbitai reviewcommand as a PR comment. Alternatively, push new commits to this PR.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.
📥 Commits
Reviewing files that changed from the base of the PR and between 970c9de4d219dae6a419c436c3eb5a6224d8923f and 9d42576f03f91a83336a9740f932266d34f56c4f.
📒 Files selected for processing (2)
middleware/timeout/timeout.go(2 hunks)middleware/timeout/timeout_test.go(2 hunks)
[!NOTE]
Other AI code review bot(s) detected
CodeRabbit 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.
Walkthrough
The timeout middleware was refactored to run handlers in a goroutine and use a select on the handler result channel vs. time.After. On timeout the middleware returns immediately (optionally invoking OnTimeout) and no longer uses context.WithTimeout or cancels the handler.
Changes
| Cohort / File(s) | Summary |
|---|---|
Async timeout redesign middleware/timeout/timeout.go |
Replaced context.WithTimeout approach with goroutine + channel and select on time.After. Handler result is received asynchronously; on timeout the middleware returns immediately and invokes OnTimeout when configured. Removed context import; added time and utils imports; removed prior runHandler wrapper. |
Test updates middleware/timeout/timeout_test.go |
Added/modified runHandler implementation used by tests, updated tests to simulate timeouts via time.Sleep and context.DeadlineExceeded, added timer cleanup (defer timer.Stop()), removed obsolete TestRunHandler_DefaultOnTimeout, adjusted TestTimeout_CustomHandler and TestRunHandler_CustomOnTimeout. |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Middleware
participant Handler
rect rgb(220,255,220)
Note over Middleware: New async flow (goroutine + select)
Client->>Middleware: HTTP request
Middleware->>Handler: start handler in goroutine
Middleware->>Middleware: select { handler result | time.After(timeout) }
alt timeout fires first
Middleware-->>Client: return fiber.ErrRequestTimeout (invoke OnTimeout if set)
Note right of Handler: handler continues in background (not awaited)
else handler completes first
Handler-->>Middleware: result via channel
Middleware-->>Client: return handler result
end
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
- Review concurrency: goroutine spawning, channel buffering/close, potential leaks.
- Verify OnTimeout invocation paths and error mapping to
fiber.ErrRequestTimeout. - Confirm non-positive timeout short-circuit and removed context behavior don't break callers.
- Check tests for reliable timing (flaky/time-dependent assertions).
Possibly related issues
- #3671 — The change (removing context.WithTimeout and running handlers in goroutines) matches concerns about timeouts not enforcing cancellation; related for validation of design trade-offs.
Possibly related PRs
- #3720 — Also edits middleware/timeout and changes context/timeout handling; may overlap or conflict.
- #3604 — Modifies timeout middleware and runHandler behavior; closely related to error/OnTimeout logic.
- #3275 — Previously refactored timeout/runHandler logic and custom timeout-error detection; likely touches same areas.
Suggested labels
v3, codex
Suggested reviewers
- sixcolors
- efectn
- gaby
- ReneWerner87
Poem
🐇 I hopped in quick with channels bright and keen,
Goroutines running where contexts had been.
When time runs out I shout, "Return!" with cheer,
OnTimeout nudges — the client gets answer near.
Hop, skip, and code — our requests are swift and seen.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description Check | ⚠️ Warning | The pull request description is largely incomplete and fails to meet the requirements of the provided template. While the description correctly references the linked issue (#3394), it provides only minimal information ("Fixed timeout middleware" and "Needs to be checked"). The substantive sections of the template—including a detailed description of changes, type of change selection, and all checklist items—remain unfilled. None of the change categories (benchmarks, documentation, changelog, migration guide, API alignment, examples) are addressed, and the type of change classification is not selected. The description appears to be submitted as a template without meaningful content added by the author. | |
| Title Check | ❓ Inconclusive | The pull request title "fix up timeout middleware" does reference the main component being modified (timeout middleware) and is directly related to the changeset. However, the title uses vague language that lacks specificity about what is actually being fixed. The significant redesign involves changing from synchronous to asynchronous timeout handling with immediate response on timeout expiry, but this fundamental shift is not conveyed in the title. The phrasing "fix up" is generic and doesn't clearly communicate the nature of the change to someone scanning the project history. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues Check | ✅ Passed | The code changes directly implement the objectives specified in linked issue #3394. The issue requests that the timeout middleware be redesigned to return a timeout error immediately when the deadline expires, rather than waiting for the handler to complete. The changes accomplish this by replacing the synchronous context.WithTimeout approach with an asynchronous model where the handler runs in a goroutine and a select statement waits for either the handler result or a timeout signal. When the timeout occurs, the middleware can now return fiber.ErrRequestTimeout immediately to the client without waiting for handler completion, which is the core objective of the linked issue. |
| Out of Scope Changes Check | ✅ Passed | All changes in this pull request are directly scoped to the linked issue #3394. The modifications are limited to two files in the timeout middleware package: timeout.go (implementing the asynchronous redesign) and timeout_test.go (updating tests for the new implementation). No changes appear in unrelated packages, utilities, or features. The changes are focused exclusively on addressing the specific architectural problem identified in the issue, with no extraneous modifications or scope creep detected. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
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 help to get the list of available commands and usage tips.
Summary of Changes
Hello @pjebs, I'm Gemini Code Assist[^1]! 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 significantly refactors the timeout middleware to enhance its robustness and control over request processing. The changes replace the previous context.WithTimeout approach with a more explicit goroutine and select statement, allowing for precise termination and handling of requests that exceed the configured timeout duration. This ensures that Fiber applications can manage request timeouts more reliably, providing consistent error responses and status codes.
Highlights
-
Timeout Mechanism Refactor: The core timeout logic in the middleware has been completely rewritten, moving from
context.WithTimeoutto agoroutineandselectstatement withtime.Afterfor more explicit and direct timeout management. -
Dependency Update: The
contextpackage import has been removed frommiddleware/timeout/timeout.go, andtimeandgithub.com/gofiber/utils/v2packages have been added to support the new timeout implementation. -
Error Handling Improvement: Timeout error handling now directly interacts with the underlying
fasthttp.RequestCtxusingctx.RequestCtx().TimeoutErrorWithResponseandctx.RequestCtx().TimeoutErrorWithCodefor more precise control over timeout responses. -
Code Structure and Testing: The
runHandlerhelper function has been moved from the main middleware file to the test file, indicating it's no longer part of the core middleware logic. Test cases have also been simplified to directly simulate timeouts.
Using Gemini Code Assist
The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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 :thumbsup: and :thumbsdown: 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.
[^1]: Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.
@pjebs There's a race condition happening in the implementation.
@gaby I'm not sure if it can be fixed without knowing fasthttp better. Or even betterm the custom error functionality should be removed. It doesn't belong with this implementation.
When the timeout occurs the handler should not have influence over the response or anything for that matter. The only question is if the handler is permitted to continue or is instantly cancelled (that can be sorted at a later day).
@pjebs one problem is the response, you have to create a new object.
Look at the fasthttp test case for doing this for example https://github.com/valyala/fasthttp/blob/master/server_test.go#L3148
The middleware should work like this:
- If handler returns before timeout => operate as per normal.
- If timeout occurs before handler returns => return a timeout http response (or custom timeout response). Totally ignore the handler. (Ideally, the handler should also be notified perhaps by context cancellation that it's no longer needed).
The handler returning errors influencing the final response part is creating race conditions.
there is another timeout middleware PR https://github.com/gofiber/fiber/pull/3680 and this should solve https://github.com/gofiber/fiber/issues/3671
maybe we can combine both in one