feat(context): add ContextWithFallback feature flag (#3166)

[wei840222]

due to breaking change in #3166

[codecov]

Codecov Report

Merging #3172 (9a0d2d2) into master (58303bd) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3172   +/-   ##
=======================================
  Coverage   98.22%   98.22%           
=======================================
  Files          43       43           
  Lines        3148     3148           
=======================================
  Hits         3092     3092           
  Misses         44       44           
  Partials       12       12           

Flag	Coverage Δ
go-1.14	98.22% <100.00%> (ø)
go-1.15	98.06% <100.00%> (+0.02%)	⬆️
go-1.16	98.03% <100.00%> (ø)
go-1.17	97.96% <100.00%> (ø)
go-1.18	97.96% <100.00%> (ø)
macos-latest	98.22% <100.00%> (+0.18%)	⬆️
nomsgpack	98.19% <100.00%> (ø)
ubuntu-latest	98.22% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
gin.go	99.18% <ø> (ø)
context.go	96.76% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58303bd...9a0d2d2. Read the comment docs.

[appleboy]

@wei840222 From #3166 (comment) comment

Add a new flag to make it backward compatible.

We can add a new flag and let the developer decide when to turn on the feature. so we don't need to re-open all related issues.

[wei840222]

oh I see, let me do it

[wei840222]

@appleboy
the one more thing I wanna check
The scope of the flag should include Context.Deadline(), Context.Done(), Context.Err(), Context.Value() or only Context.Deadline(), Context.Done(), Context.Err() ?

[appleboy]

Testing code

func TestGinContextCancel(t *testing.T) {
	serv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
		return
	}))
	defer serv.Close()

	wg := &sync.WaitGroup{}

	r := gin.New()
	r.GET("/", func(ginctx *gin.Context) {
		wg.Add(1)

		ginctx = ginctx.Copy()

		// start async goroutine for calling serv
		go func() {
			defer wg.Done()

			req, err := http.NewRequestWithContext(ginctx, http.MethodGet, serv.URL, nil)
			if err != nil {
				panic(err)
			}

			res, err := http.DefaultClient.Do(req)
			if err != nil {
				// context is always canceled with gin v1.8.0, it is big breaking change with gin v1.7
				t.Error("context is always canceled with gin v1.8.0, it is big breaking change with gin v1.7", err) 
				return
			}

			if res.StatusCode != http.StatusOK {
				log.Println("unexpected status code ", res.Status)
				return
			}
		}()
	})
	go func() {
		err := r.Run(":8080")
		if err != nil {
			panic(err)
		}
	}()

	res, err := http.Get("http://127.0.0.1:8080/")
	if err != nil {
		panic(err)
	}

	if res.StatusCode != http.StatusOK {
		panic(err)
	}

	wg.Wait()
}

#3166 (comment)

[appleboy]

#2751
#2769

We need to review the above two PR and make it backward compatible.

[jerome-laforge]

@appleboy & @wei840222

maybe to avoid flawky test by ensuring that:

• :8080 is always opened (as initially done into goroutine)
• request on http://127.0.0.1:8080/ is done before launch async task with closing the channel ensureRequestIsOver

you can use this:

func TestGinContextCancel(t *testing.T) {
	serv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
		return
	}))
	defer serv.Close()

	ensureRequestIsOver := make(chan struct{})

	wg := &sync.WaitGroup{}

	r := gin.New()
	r.GET("/", func(ginctx *gin.Context) {
		wg.Add(1)

		ginctx = ginctx.Copy()

		// start async goroutine for calling serv
		go func() {
			defer wg.Done()

			<-ensureRequestIsOver // ensure request on http://127.0.0.1:8080/ is done

			req, err := http.NewRequestWithContext(ginctx, http.MethodGet, serv.URL, nil)
			if err != nil {
				panic(err)
			}

			res, err := http.DefaultClient.Do(req)
			if err != nil {
				// context is always canceled with gin v1.8.0, it is big breaking change with gin v1.7
				t.Error("context is always canceled with gin v1.8.0, it is big breaking change with gin v1.7", err)
				return
			}

			if res.StatusCode != http.StatusOK {
				log.Println("unexpected status code ", res.Status)
				return
			}
		}()
	})

	l, err := net.Listen("tcp", ":8080")
	if err != nil {
		panic(err)
	}
	go func() {
		s := &http.Server{
			Handler: r,
		}

		if err := s.Serve(l); err != nil {
			panic(err)
		}
	}()

	res, err := http.Get("http://127.0.0.1:8080/")
	if err != nil {
		panic(err)
	}

	close(ensureRequestIsOver)

	if res.StatusCode != http.StatusOK {
		panic(err)
	}

	wg.Wait()
}

[wei840222]

Hi @appleboy and @jerome-laforge
here is the version with feature for enable context with fallback (default is disable)

[jerome-laforge]

here is the version with feature for enable context with fallback (default is disable)

Hi @wei840222,
I have just tested with replace github.com/gin-gonic/gin v1.8.0 => github.com/wei840222/gin v1.8.1-0.20220606034057-9a0d2d27c9a7 in my go.mod.
It works fine on my end.