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

Is there a possibility to use gin.Context behalf of the c.Request.Context() #2845

Open
ksaritek opened this issue Aug 26, 2021 · 9 comments
Open

Comments

@ksaritek
Copy link

version v1.6.2

We have long-running queries and functions. I want to bind those processes to request context. Whenever an incoming request is canceled, It should stop. That is what we try to achieve.

If I pass c.Request.Context(), there is no problem. However, we would like to do that through gin.Context. The reason behind that is, it is possible to pass gin.Context instead of c.Request.Context() accidentally, and we don't want that.

If I do pass c.Request.Context()

	route.GET("/", func(c *gin.Context) {
                 ctx := c.Request.Context()
		longQueryWithContext(ctx, h)
	})

It is working fine. But, It is so hard to track who passed which context.
for instance:

	route.GET("/", func(c *gin.Context) {
		longQueryWithContext(c, h)
	})

As stated at the gin.Context source file, the proper way is to pass c.Request.Context(), but I could not find a way to track and fix it at the coding level. Is it possible to bind those methods to c.Request.Context()?

// Deadline always returns that there is no deadline (ok==false),
// maybe you want to use Request.Context().Deadline() instead.
func (c *Context) Deadline() (deadline time.Time, ok bool) {
	return
}

// Done always returns nil (chan which will wait forever),
// if you want to abort your work when the connection was closed
// you should use Request.Context().Done() instead.
func (c *Context) Done() <-chan struct{} {
	return nil
}

// Err always returns nil, maybe you want to use Request.Context().Err() instead.
func (c *Context) Err() error {
	return nil
}
@luchuanbing123
Copy link

@ksaritek why not use assert to find the bad usage and fix it,

func longQueryWithContext(ctx context.Context) {
	if c, ok := ctx.(*gin.Context); ok {
		// c.Request.RequestURI
		// c.Request.Method
	}
}

@ksaritek
Copy link
Author

@luchuanbing123 I am only interested in c.Request.Context(). The problem is you can pass gin.Context and gin.Context.Request.Context() as a parameter to the longQueryWithContext. The problem is I could not find a linter or any check to prevent passing gin.Context instead of gin.Context.Request.Context(). gin.Context.Request.Context() already keeps connection info for incoming request and if the client kills the request, context is canceled. I need that cancel information to stop SQL query.

@luchuanbing123
Copy link

@ksaritek may workaround as follow ?

func longQueryWithContext(ctx context.Context) {
	if c, ok := ctx.(*gin.Context); ok {
                ctx = c.Request.Context() 
	}
}

@ksaritek
Copy link
Author

ksaritek commented Aug 27, 2021

@luchuanbing123 if more then one developer, it is not easy to track it. We generate the models with https://github.com/volatiletech/sqlboiler with context. if Developers accidentally pass gin.Context, it is end of the game. I tried to write a linter for that but I could not do it.

So is there any feature or work around to prevent not to pass gin.Context instead of c.Request.Context()? It is so easy to make a mistake for that.

@abderang
Copy link

abderang commented Sep 3, 2021

I see the fix for this is already merged to master about a month ago, so I would suggest to either wait for the next release or fork it and release it on your own.

// Deadline returns that there is no deadline (ok==false) when c.Request has no Context.
func (c *Context) Deadline() (deadline time.Time, ok bool) {
	if c.Request == nil || c.Request.Context() == nil {
		return
	}
	return c.Request.Context().Deadline()
}

// Done returns nil (chan which will wait forever) when c.Request has no Context.
func (c *Context) Done() <-chan struct{} {
	if c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}

// Err returns nil when c.Request has no Context.
func (c *Context) Err() error {
	if c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Err()
}

@ksaritek
Copy link
Author

ohh nice, thank you @abderan

@alphashaw
Copy link

This should be closed now.

@ZheruiL
Copy link

ZheruiL commented Feb 23, 2023

I see the fix for this is already merged to master about a month ago, so I would suggest to either wait for the next release or fork it and release it on your own.

// Deadline returns that there is no deadline (ok==false) when c.Request has no Context.
func (c *Context) Deadline() (deadline time.Time, ok bool) {
	if c.Request == nil || c.Request.Context() == nil {
		return
	}
	return c.Request.Context().Deadline()
}

// Done returns nil (chan which will wait forever) when c.Request has no Context.
func (c *Context) Done() <-chan struct{} {
	if c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}

// Err returns nil when c.Request has no Context.
func (c *Context) Err() error {
	if c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Err()
}

hi @abde8ng ,
I find that for the newest version, there's one more param called ContextWithFallback, so I think we need to set it as true manually for this situation? Is it correct? please tell.

func (c *Context) Done() <-chan struct{} {
	if !c.engine.ContextWithFallback || c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}
router := gin.Default()
router.ContextWithFallback = true

@opvexe
Copy link

opvexe commented Feb 17, 2024

I see the fix for this is already merged to master about a month ago, so I would suggest to either wait for the next release or fork it and release it on your own.

// Deadline returns that there is no deadline (ok==false) when c.Request has no Context.
func (c *Context) Deadline() (deadline time.Time, ok bool) {
	if c.Request == nil || c.Request.Context() == nil {
		return
	}
	return c.Request.Context().Deadline()
}

// Done returns nil (chan which will wait forever) when c.Request has no Context.
func (c *Context) Done() <-chan struct{} {
	if c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}

// Err returns nil when c.Request has no Context.
func (c *Context) Err() error {
	if c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Err()
}

hi @abde8ng , I find that for the newest version, there's one more param called ContextWithFallback, so I think we need to set it as true manually for this situation? Is it correct? please tell.

func (c *Context) Done() <-chan struct{} {
	if !c.engine.ContextWithFallback || c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}
router := gin.Default()
router.ContextWithFallback = true

func main() {
r := gin.New()
r.ContextWithFallback = true
r.Use(cors.Default())
r.GET("/health", health)

r.Run(":8080")

}

func health(c *gin.Context) {
c.JSON(http.StatusOK, gin.H{"status": "ok"})
}

ContextWithFallback is always false ?

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

No branches or pull requests

6 participants