Skip to content

feat: add timeout middleware #150

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

emmadal
Copy link
Contributor

@emmadal emmadal commented Mar 25, 2025

@jeffotoni i need your help if possible.

in the timeout middleware, after setting the deadline time, the handler continues to be executed. i dont know why ?

can you help me if possible to understand the issues ?

@jeffotoni
Copy link
Owner

@jeffotoni i need your help if possible.

in the timeout middleware, after setting the deadline time, the handler continues to be executed. i dont know why ?

can you help me if possible to understand the issues ?

Yes, I will help, when there is a window down I will check for you. 🐧🚀

@emmadal
Copy link
Contributor Author

emmadal commented Mar 25, 2025

@jeffotoni ok thanks

@jeffotoni
Copy link
Owner

@jeffotoni i need your help if possible.
in the timeout middleware, after setting the deadline time, the handler continues to be executed. i dont know why ?
can you help me if possible to understand the issues ?

Yes, I will help, when there is a window down I will check for you. 🐧🚀

I took a look, and used goroutine to be able to control errors, close the context, I removed its runHandler. The good thing is that you advanced well, this middleware is very annoying...

I'll take advantage and do the tests right away

@emmadal
Copy link
Contributor Author

emmadal commented Mar 26, 2025

@jeffotoni i need your help if possible.
in the timeout middleware, after setting the deadline time, the handler continues to be executed. i dont know why ?
can you help me if possible to understand the issues ?

Yes, I will help, when there is a window down I will check for you. 🐧🚀

I took a look, and used goroutine to be able to control errors, close the context, I removed its runHandler. The good thing is that you advanced well, this middleware is very annoying...

I'll take advantage and do the tests right away

the context will be passed througth a goroutine to handle it efficiently ?

@emmadal
Copy link
Contributor Author

emmadal commented Mar 26, 2025

@jeffotoni like this:

func New(opt ...Options) func(next quick.Handler) quick.Handler {
	option := defaultOptions(opt...)

	return func(next quick.Handler) quick.Handler {
		return quick.HandlerFunc(func(c *quick.Ctx) error {
			// Skip middleware logic if Next returns true
			if option.Next != nil && option.Next(c) {
				return next.ServeQuick(c)
			}

			// Skip if duration is not set
			if option.Duration <= 0 {
				return next.ServeQuick(c)
			}

			// Create a timeout context
			ctx, cancel := context.WithTimeout(c.Request.Context(), option.Duration)
			defer cancel()

			// Attach timeout context to request
			c.Request = c.Request.WithContext(ctx)

			// Channel to capture handler execution result
			errCh := make(chan error, 1)

			// Run handler in a goroutine
			go func() {
				errCh <- next.ServeQuick(c)
                                 close(errCh)
			}()

			select {
			case err := <-errCh:
				// Handler finished before timeout
				return err
			case <-ctx.Done():
				// Timeout exceeded
				return c.Status(quick.StatusRequestTimeout).SendString("Request Timeout")
			}
		})
	}
}

@emmadal
Copy link
Contributor Author

emmadal commented Mar 26, 2025

the issue is when we trying to process the HTTP request with ServeQuick(c), we getting runtime error: invalid memory address or nil pointer dereference

@jeffotoni
Copy link
Owner

the issue is when we trying to process the HTTP request with ServeQuick(c), we getting runtime error: invalid memory address or nil pointer dereference

I'm exactly there, trying to solve it, the goroutine has become quite complicated, I'm having to see what to do in this scenario. 🙈

@jeffotoni
Copy link
Owner

the issue is when we trying to process the HTTP request with ServeQuick(c), we getting runtime error: invalid memory address or nil pointer dereference

I'm exactly there, trying to solve it, the goroutine has become quite complicated, I'm having to see what to do in this scenario. 🙈

We can't use goroutines, Quick won't support the architecture, it wasn't designed for this flow, I tried to do something but it's too risky.
so I'm fixing it to try to stay synchronous with some limitations but it works.
I haven't managed to do it yet, I'll leave it for later but it's a good challenge, I had to make some corrections in Quick because of this.

@emmadal
Copy link
Contributor Author

emmadal commented Mar 26, 2025

the issue is when we trying to process the HTTP request with ServeQuick(c), we getting runtime error: invalid memory address or nil pointer dereference

I'm exactly there, trying to solve it, the goroutine has become quite complicated, I'm having to see what to do in this scenario. 🙈

We can't use goroutines, Quick won't support the architecture, it wasn't designed for this flow, I tried to do something but it's too risky.
so I'm fixing it to try to stay synchronous with some limitations but it works.
I haven't managed to do it yet, I'll leave it for later but it's a good challenge, I had to make some corrections in Quick because of this.

👌 cool. In the debugging, also i see we encountered a race condition. I will try to work on another middleware today .

@jeffotoni
Copy link
Owner

o problema é quando tentamos processar a solicitação HTTP com ServeQuick(c), obtemos um erro de tempo de execução: endereço de memória inválido ou desreferência de ponteiro nulo

Estou exatamente aí, tentando resolver, a goroutine ficou bem complicada, estou tendo que ver o que fazer nesse cenário. 🙈

Não podemos usar goroutines, o Quick não suporta a arquitetura, não foi projetado para esse fluxo, tentei fazer algo, mas é muito arriscado.
então estou consertando para tentar ficar sincronizado com algumas limitações, mas funciona.
Ainda não consegui fazer, vou deixar para depois, mas é um bom desafio, tive que fazer algumas correções no Quick por causa disso.

👌 legal. Na depuração, também vejo que encontramos uma condição de corrida. Tentarei trabalhar em outro middleware hoje.

Every morning I manage to work on Quick a little, this timeout has led to some improvements in Quick's architecture.
The middleware's c.Status is not reached, nor is the error that is added to the handler, because the Handler overrides the middleware.
Well, I'm investigating and testing to find the best solution.
But with this some improvements have been made to Quick, I'll try every morning.
So I'll go back to it with a clearer head and try to improve it.
This is all due to our timeout in the middleware. 🚀😍🥳

@emmadal
Copy link
Contributor Author

emmadal commented Mar 26, 2025

o problema é quando tentamos processar a solicitação HTTP com ServeQuick(c), obtemos um erro de tempo de execução: endereço de memória inválido ou desreferência de ponteiro nulo

Estou exatamente aí, tentando resolver, a goroutine ficou bem complicada, estou tendo que ver o que fazer nesse cenário. 🙈

Não podemos usar goroutines, o Quick não suporta a arquitetura, não foi projetado para esse fluxo, tentei fazer algo, mas é muito arriscado.
então estou consertando para tentar ficar sincronizado com algumas limitações, mas funciona.
Ainda não consegui fazer, vou deixar para depois, mas é um bom desafio, tive que fazer algumas correções no Quick por causa disso.

👌 legal. Na depuração, também vejo que encontramos uma condição de corrida. Tentarei trabalhar em outro middleware hoje.

Every morning I manage to work on Quick a little, this timeout has led to some improvements in Quick's architecture. The middleware's c.Status is not reached, nor is the error that is added to the handler, because the Handler overrides the middleware. Well, I'm investigating and testing to find the best solution. But with this some improvements have been made to Quick, I'll try every morning. So I'll go back to it with a clearer head and try to improve it. This is all due to our timeout in the middleware. 🚀😍🥳

Ok no poblem @jeffotoni
on my side im trying to implement profiling middleware for better debugging and performance bottlenecks only in dev mode.

@emmadal emmadal force-pushed the feat/middleware-timeout branch from 38902c4 to dac5fbb Compare April 3, 2025 11:09
@emmadal
Copy link
Contributor Author

emmadal commented Apr 3, 2025

hello @jeffotoni i will investigate on this pr today to fix

@jeffotoni
Copy link
Owner

hello @jeffotoni i will investigate on this pr today to fix

Or what did I receive here was not the test, only the middleware and the example was the same or what I said was wrong? 🐧

@emmadal emmadal force-pushed the feat/middleware-timeout branch from dac5fbb to 561b911 Compare April 4, 2025 12:24
@emmadal
Copy link
Contributor Author

emmadal commented Apr 4, 2025

hello @jeffotoni i will investigate on this pr today to fix

Or what did I receive here was not the test, only the middleware and the example was the same or what I said was wrong? 🐧

it's the middleware with example test. but it's currently in draft PR. the issues was the program send a panic after timeout elapsed

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

Successfully merging this pull request may close these issues.

2 participants