gin-gonic / gin

Gin is a HTTP web framework written in Go (Golang). It features a Martini-like API with much better performance -- up to 40 times faster. If you need smashing performance, get yourself some Gin.
https://gin-gonic.com/
MIT License
76.77k stars 7.92k forks source link

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

Open ksaritek opened 2 years ago

ksaritek commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@ksaritek may workaround as follow ?

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

@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 commented 2 years ago

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 commented 2 years ago

ohh nice, thank you @abderan

alphashaw commented 1 year ago

This should be closed now.

ZheruiL commented 1 year ago

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 commented 4 months ago

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 ?