Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
skonto committed Dec 11, 2023
1 parent f55a7bd commit d11f08d
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 8 deletions.
6 changes: 5 additions & 1 deletion pkg/activator/handler/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ func BenchmarkHandlerChain(b *testing.B) {
})
}

func TestProxyWithChainHandler(t *testing.T) {
// TestActivatorChainHandlerWithFullDuplex tests activator's chain handler with the new http1 full duplex support against the issue
// https://github.com/golang/go/issues/40747, where reverse proxy failed with read errors.
// The test uses the reproducer in https://github.com/golang/go/issues/40747#issuecomment-733552061.
// We enable full duplex by setting the annotation `features.knative.dev/http-full-duplex` at the revision level.
func TestActivatorChainHandlerWithFullDuplex(t *testing.T) {
ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(t)
rev := revision(testNamespace, testRevName)
defer reset()
Expand Down
15 changes: 8 additions & 7 deletions pkg/queue/sharedmain/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func mainHandler(
composedHandler = tracing.HTTPSpanMiddleware(composedHandler)
}

composedHandler = wrapQPHandlerWithFullDuplex(composedHandler, env.EnableHTTPFullDuplex, logger)
composedHandler = withFullDuplex(composedHandler, env.EnableHTTPFullDuplex, logger)

drainer := &pkghandler.Drainer{
QuietPeriod: drainSleepDuration,
Expand Down Expand Up @@ -127,13 +127,14 @@ func adminHandler(ctx context.Context, logger *zap.SugaredLogger, drainer *pkgha
return mux
}

func wrapQPHandlerWithFullDuplex(h http.Handler, enableFullDuplex bool, logger *zap.SugaredLogger) http.HandlerFunc {
func withFullDuplex(h http.Handler, enableFullDuplex bool, logger *zap.SugaredLogger) http.Handler {
if !enableFullDuplex {
return h
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if enableFullDuplex {
rc := http.NewResponseController(w)
if err := rc.EnableFullDuplex(); err != nil {
logger.Errorw("Unable to enable full duplex", zap.Error(err))
}
rc := http.NewResponseController(w)
if err := rc.EnableFullDuplex(); err != nil {
logger.Errorw("Unable to enable full duplex", zap.Error(err))
}
h.ServeHTTP(w, r)
})
Expand Down

0 comments on commit d11f08d

Please sign in to comment.