[TT-5180] - Allow specifying request timeout in JS middleware requests

[LowCostCustoms]

Is your feature request related to a problem? Please describe.

So far Tyk allows to set JSVM middleware timeout. However, when requests to external APIs are made via TykMakeHttpRequest and TykBatchRequest functions, the middleware could respond with HTTP 500 error code when one of the requests takes too much time to complete. In several situations, this becomes an unwanted behavior since gathering responses from all services could be unnecessary. Examples of such cases are liveness and readiness endpoints, which collect the upstream service status and should not fail even if an upstream fails to reply.

Describe the solution you'd like

It would be nice if we could somehow pass the request timeout configuration in TykMakeHttpRequest and TykBatchRequest JS functions. For instance, it could be either an optional argument or a timeout property of each RequestDefinition.

Describe alternatives you've considered

Raising JSVM timeout configuration value should potentially help. However, we don't want to make a client wait for a few minutes to get a response.

Additional context

The following patch for Tyk version v3.2.2 introduces an optional timeout property, which is used to determine the corresponding request timeout. Although it seems that MakeRequests function is called in multiple places and I'm not sure how it'll affect other pieces of the software.

diff --git a/gateway/batch_requests.go b/gateway/batch_requests.go
index aeda76c0..37009b81 100644
--- a/gateway/batch_requests.go
+++ b/gateway/batch_requests.go
@@ -1,15 +1,16 @@
 package gateway
 
 import (
+	"context"
 	"crypto/tls"
 	"encoding/json"
 	"fmt"
+	"github.com/TykTechnologies/tyk/config"
 	"io/ioutil"
 	"net/http"
 	"strconv"
 	"strings"
-
-	"github.com/TykTechnologies/tyk/config"
+	"time"
 )
 
 // RequestDefinition defines a batch request
@@ -18,6 +19,7 @@ type RequestDefinition struct {
 	Headers     map[string]string `json:"headers"`
 	Body        string            `json:"body"`
 	RelativeURL string            `json:"relative_url"`
+	Timeout     uint              `json:"timeout"`
 }
 
 // BatchRequestStructure defines a batch request order
@@ -130,8 +132,13 @@ func (b *BatchRequestHandler) MakeRequests(batchRequest BatchRequestStructure, r
 	if !batchRequest.SuppressParallelExecution {
 		replies := make(chan BatchReplyUnit)
 		for i, req := range requestSet {
+			requestItem := batchRequest.Requests[i]
+			requestContext, cancelFunc := makeRequestContext(requestItem.Timeout)
+
+			defer cancelFunc()
+
 			go func(i int, req *http.Request) {
-				reply := b.doRequest(req, batchRequest.Requests[i].RelativeURL)
+				reply := b.doRequest(req.WithContext(requestContext), requestItem.RelativeURL)
 				replies <- reply
 			}(i, req)
 		}
@@ -141,7 +148,12 @@ func (b *BatchRequestHandler) MakeRequests(batchRequest BatchRequestStructure, r
 		}
 	} else {
 		for i, req := range requestSet {
-			reply := b.doRequest(req, batchRequest.Requests[i].RelativeURL)
+			requestItem := batchRequest.Requests[i]
+			requestContext, cancelFunc := makeRequestContext(requestItem.Timeout)
+
+			defer cancelFunc()
+
+			reply := b.doRequest(req.WithContext(requestContext), batchRequest.Requests[i].RelativeURL)
 			replySet = append(replySet, reply)
 		}
 	}
@@ -149,6 +161,18 @@ func (b *BatchRequestHandler) MakeRequests(batchRequest BatchRequestStructure, r
 	return replySet
 }
 
+// makeRequestContext creates a request context based on the timeout value.
+func makeRequestContext(timeout uint) (context.Context, context.CancelFunc) {
+	if timeout == 0 {
+		return context.WithCancel(context.Background())
+	} else {
+		return context.WithTimeout(
+			context.Background(),
+			time.Duration(timeout)*time.Second,
+		)
+	}
+}
+
 // HandleBatchRequest is the actual http handler for a batch request on an API definition
 func (b *BatchRequestHandler) HandleBatchRequest(w http.ResponseWriter, r *http.Request) {
 	if r.Method != "POST" {

[joshblakeley]

Like this a lot - I think it would probably be better to add two new functions to the JSVM i.e. TykMakeHttpRequestWithTimeout so we can ensure we keep backwards compatibility. Another option might be to use the new possibility to have a Go function run on a per endpoint basis which is available from version 4 - there you can be a bit more flexible with the way you aggregate because it doesn't rely on the predefined API the JSVM does.

[andyo-tyk]

There are a number of enhancements that we would like to make to Tyk's already flexible custom middleware capability; this suggestion is definitely on the list but there is no plan at this stage to implement it, so I will close the ticket.

If we do introduce this or similar capability in the future then we will update this ticket for reference.