Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [Bug]: Timeout middleware needs to be redesigned #3394

Open
3 tasks done
pjebs opened this issue Apr 6, 2025 · 1 comment
Open
3 tasks done

🐛 [Bug]: Timeout middleware needs to be redesigned #3394

pjebs opened this issue Apr 6, 2025 · 1 comment

Comments

@pjebs
Copy link
Contributor

pjebs commented Apr 6, 2025

Bug Description

Currently, the timeout middleware waits for a certain duration.
If the handler returns before that time, it works as per normal.

If the handler takes longer, it waits for handler to finish.
Then it returns a timeout error.

It can be redesigned so that if the timeout completes first, the timeout error is immediately returned EVEN IF the handler continues until completion. At least the client gets a response.

How to Reproduce

As it operates normally

Expected Behavior

After timeout, return immediately (even if request handler continues)

Fiber Version

v3

Code Snippet (optional)

// New enforces a timeout for each incoming request. If the timeout expires or
// any of the specified errors occur, fiber.ErrRequestTimeout is returned.
func New(h fiber.Handler, timeout time.Duration, tErrs ...error) fiber.Handler {
	return func(ctx fiber.Ctx) error {
		// If timeout <= 0, skip context.WithTimeout and run the handler as-is.
		if timeout <= 0 {
			return runHandler(ctx, h, tErrs)
		}

		// Create a context with the specified timeout; any operation exceeding
		// this deadline will be canceled automatically.
		timeoutContext, cancel := context.WithTimeout(ctx, timeout)
		defer cancel()

		// Run the handler and check for relevant errors.
		err := runHandler(ctx, h, tErrs)

		// If the context actually timed out, return a timeout error.
		if errors.Is(timeoutContext.Err(), context.DeadlineExceeded) {
			return fiber.ErrRequestTimeout
		}
		return err
	}
}

// runHandler executes the handler and returns fiber.ErrRequestTimeout if it
// sees a deadline exceeded error or one of the custom "timeout-like" errors.
func runHandler(c fiber.Ctx, h fiber.Handler, tErrs []error) error {
	// Execute the wrapped handler synchronously.
	err := h(c)
	// If the context has timed out, return a request timeout error.
	if err != nil && (errors.Is(err, context.DeadlineExceeded) || isCustomError(err, tErrs)) {
		return fiber.ErrRequestTimeout
	}
	return err
}

// isCustomError checks whether err matches any error in errList using errors.Is.
func isCustomError(err error, errList []error) bool {
	for _, e := range errList {
		if errors.Is(err, e) {
			return true
		}
	}
	return false
}

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my problem prior to opening this one.
  • I understand that improperly formatted bug reports may be closed without explanation.
@pjebs
Copy link
Contributor Author

pjebs commented Apr 6, 2025

I understand that it's implemented this way because currently Fiber internals prevent a request from responding until the handler no longer is using the fiber.Context.

I think with a few adjustments it can be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant