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

bugfix/TT-12343 #6678

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions gateway/reverse_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,14 @@ func (p *ReverseProxy) ServeHTTPForCache(rw http.ResponseWriter, req *http.Reque

const defaultProxyTimeout float64 = 30

func proxyTimeout(spec *APISpec) float64 {
// ProxyTimeout checks if there is a path-specific enforced timeout, otherwise it checks the global timeout
// The path-specific timeout overrides the global timeout.
// The path-specific timeout is enforced by the API spec version, while the global timeout is enforced by the global config.
// TT-12343
func proxyTimeout(spec *APISpec, enforcedTimeout float64) float64 {
if enforcedTimeout > 0 {
return enforcedTimeout
}
if spec.GlobalConfig.ProxyDefaultTimeout > 0 {
return spec.GlobalConfig.ProxyDefaultTimeout
}
Expand Down Expand Up @@ -1182,7 +1189,7 @@ func (p *ReverseProxy) WrappedServeHTTP(rw http.ResponseWriter, req *http.Reques
oldTransport.DisableKeepAlives = true
}

timeout := proxyTimeout(p.TykAPISpec)
timeout := proxyTimeout(p.TykAPISpec, enforcedTimeout)

p.TykAPISpec.HTTPTransport = p.httpTransport(timeout, rw, req, outreq)
p.TykAPISpec.HTTPTransportCreated = time.Now()
Expand Down
124 changes: 124 additions & 0 deletions gateway/reverse_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,130 @@
})
}

func TestTimeoutBehavior(t *testing.T) {

test.Flaky(t) // Because involves timing tests


tests := []struct {
name string
expectedDelay time.Duration
proxyTimeout float64
enforcedTimeout float64

}{
{
name: "TEST 1: Enforced timeout is set, so used instead of default proxy timeout, enforced timeout is lesser than proxy timeout",
proxyTimeout: 3,
enforcedTimeout: 1,
expectedDelay: 1 * time.Second,

},
{
name: "TEST 2: Enforced timeout is set, so used instead of default proxy timeout, enforced timeout is greater than proxy timeout",
proxyTimeout: 1,
enforcedTimeout: 3,
expectedDelay: 3 * time.Second,

},
{
name: "TEST 3: Enforced timeout not set, so default proxy timeout is used",
proxyTimeout: 2,
enforcedTimeout: 0,
expectedDelay: 2 * time.Second,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Create upstream server that delays
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

Check failure on line 467 in gateway/reverse_proxy_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'r' seems to be unused, consider removing or renaming it to match ^_ (revive)
// Keep upstream alive for a bit longer than the enforced timeout
time.Sleep(tc.expectedDelay + 5 * time.Second)
w.WriteHeader(http.StatusOK)
}))
defer upstream.Close()

// Start test gateway with timeout config
ts := StartTest(func(globalConf *config.Config) {
globalConf.ProxyDefaultTimeout = tc.proxyTimeout
})
defer ts.Close()

ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
spec.Name = "Timeout Test API"
spec.APIID = "timeout_test_api"
spec.Proxy.ListenPath = "/timeout-test"
spec.Proxy.StripListenPath = true
spec.Proxy.TargetURL = upstream.URL

// Only set the enforced timeout if it's greater than 0
if tc.enforcedTimeout > 0 {

spec.EnforcedTimeoutEnabled = true
// Configure version with timeout
spec.VersionData = apidef.VersionData{
NotVersioned: true,
DefaultVersion: "Default",
Versions: map[string]apidef.VersionInfo{
"Default": {
UseExtendedPaths: true,
Name: "Default",
ExtendedPaths: apidef.ExtendedPathsSet{
HardTimeouts: []apidef.HardTimeoutMeta{
{
Path: "/",
Method: "GET",
TimeOut: int(tc.enforcedTimeout),
},
},
},
},
},
}


}
})

// // Add small delay for API registration
time.Sleep(100 * time.Millisecond)

// // Make request and measure time
start := time.Now()

_, err := ts.Run(t, []test.TestCase{
{
Data: nil,
Path: "/timeout-test",
Code: http.StatusGatewayTimeout,
BodyMatch: `"error": "Upstream service reached hard timeout."`,
Method: http.MethodGet,
},
}...)

if err != nil {
t.Fatalf("error: %v", err)
}

// Add a small buffer (e.g. 100ms) to account for processing overhead
duration := time.Since(start)
const timeoutBuffer = 100 * time.Millisecond
maxAllowedDuration := tc.expectedDelay + timeoutBuffer

if duration > maxAllowedDuration {
t.Fatalf("duration: %v exceeded maximum allowed duration: %v (expected: %v + %v buffer)",
duration, maxAllowedDuration, tc.expectedDelay, timeoutBuffer)
} else {
t.Logf("duration: %v was as expected: %v", duration, maxAllowedDuration)
}

t.Logf("proxy timeout: %v", tc.proxyTimeout)
t.Logf("enforced timeout: %v", tc.enforcedTimeout)
})
}
}

func TestCircuitBreakerEvents(t *testing.T) {
// Use this channel to capture webhook events:
triggeredEvent := make(chan apidef.TykEvent)
Expand Down
Loading