use custom http client instead of the http DefaultClient (#823)

This commit replaces the default client from the http lib with a custom one.
The previously used default client doesn't come with a timeout. This could cause
unwanted locks.
That solution chosen here creates a http client in the base component dendrite
with a constant timeout of 30 seconds. If it should be necessary to overwrite
this, we could include the timeout in the dendrite configuration.
Here it would be a good idea to extend the type "Address" by a timeout and
create an http client for each service.

Closes #820

Signed-off-by: Benedikt Bongartz <benne@klimlive.de>

Co-authored-by: Kegsay <kegan@matrix.org>

appservice/api/query.go

@@ -20,6 +20,7 @@ package api
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"net/http"
 
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
@@ -97,15 +98,15 @@ type httpAppServiceQueryAPI struct {
 
 // NewAppServiceQueryAPIHTTP creates a AppServiceQueryAPI implemented by talking
 // to a HTTP POST API.
-// If httpClient is nil then it uses http.DefaultClient
+// If httpClient is nil an error is returned
 func NewAppServiceQueryAPIHTTP(
 	appserviceURL string,
 	httpClient *http.Client,
-) AppServiceQueryAPI {
+) (AppServiceQueryAPI, error) {
 	if httpClient == nil {
-		httpClient = http.DefaultClient
+		return nil, errors.New("NewRoomserverAliasAPIHTTP: httpClient is <nil>")
 	}
-	return &httpAppServiceQueryAPI{appserviceURL, httpClient}
+	return &httpAppServiceQueryAPI{appserviceURL, httpClient}, nil
 }
 
 // RoomAliasExists implements AppServiceQueryAPI

cmd/roomserver-integration-tests/main.go

@@ -44,6 +44,8 @@ var (
 	// This needs to be high enough to account for the time it takes to create
 	// the postgres database tables which can take a while on travis.
 	timeoutString = defaulting(os.Getenv("TIMEOUT"), "60s")
+	// Timeout for http client
+	timeoutHTTPClient = defaulting(os.Getenv("TIMEOUT_HTTP"), "30s")
 	// The name of maintenance database to connect to in order to create the test database.
 	postgresDatabase = defaulting(os.Getenv("POSTGRES_DATABASE"), "postgres")
 	// The name of the test database to create.
@@ -68,7 +70,10 @@ func defaulting(value, defaultValue string) string {
 	return value
 }
 
-var timeout time.Duration
+var (
+	timeout     time.Duration
+	timeoutHTTP time.Duration
+)
 
 func init() {
 	var err error
@@ -76,6 +81,10 @@ func init() {
 	if err != nil {
 		panic(err)
 	}
+	timeoutHTTP, err = time.ParseDuration(timeoutHTTPClient)
+	if err != nil {
+		panic(err)
+	}
 }
 
 func createDatabase(database string) error {
@@ -199,7 +208,10 @@ func writeToRoomServer(input []string, roomserverURL string) error {
 			return err
 		}
 	}
-	x := api.NewRoomserverInputAPIHTTP(roomserverURL, nil)
+	x, err := api.NewRoomserverInputAPIHTTP(roomserverURL, &http.Client{Timeout: timeoutHTTP})
+	if err != nil {
+		return err
+	}
 	return x.InputRoomEvents(context.Background(), &request, &response)
 }
 
@@ -258,7 +270,7 @@ func testRoomserver(input []string, wantOutput []string, checkQueries func(api.R
 	cmd.Args = []string{"dendrite-room-server", "--config", filepath.Join(dir, test.ConfigFile)}
 
 	gotOutput, err := runAndReadFromTopic(cmd, cfg.RoomServerURL()+"/metrics", doInput, outputTopic, len(wantOutput), func() {
-		queryAPI := api.NewRoomserverQueryAPIHTTP("http://"+string(cfg.Listen.RoomServer), nil)
+		queryAPI, _ := api.NewRoomserverQueryAPIHTTP("http://"+string(cfg.Listen.RoomServer), &http.Client{Timeout: timeoutHTTP})
 		checkQueries(queryAPI)
 	})
 	if err != nil {

common/basecomponent/base.go

@@ -19,6 +19,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"time"
 
 	"golang.org/x/crypto/ed25519"
 
@@ -52,6 +53,7 @@ type BaseDendrite struct {
 
 	// APIMux should be used to register new public matrix api endpoints
 	APIMux        *mux.Router
+	httpClient    *http.Client
 	Cfg           *config.Dendrite
 	KafkaConsumer sarama.Consumer
 	KafkaProducer sarama.SyncProducer
@@ -77,11 +79,14 @@ func NewBaseDendrite(cfg *config.Dendrite, componentName string) *BaseDendrite {
 		kafkaConsumer, kafkaProducer = setupKafka(cfg)
 	}
 
+	const defaultHTTPTimeout = 30 * time.Second
+
 	return &BaseDendrite{
 		componentName: componentName,
 		tracerCloser:  closer,
 		Cfg:           cfg,
 		APIMux:        mux.NewRouter().UseEncodedPath(),
+		httpClient:    &http.Client{Timeout: defaultHTTPTimeout},
 		KafkaConsumer: kafkaConsumer,
 		KafkaProducer: kafkaProducer,
 	}
@@ -95,7 +100,11 @@ func (b *BaseDendrite) Close() error {
 // CreateHTTPAppServiceAPIs returns the QueryAPI for hitting the appservice
 // component over HTTP.
 func (b *BaseDendrite) CreateHTTPAppServiceAPIs() appserviceAPI.AppServiceQueryAPI {
-	return appserviceAPI.NewAppServiceQueryAPIHTTP(b.Cfg.AppServiceURL(), nil)
+	a, err := appserviceAPI.NewAppServiceQueryAPIHTTP(b.Cfg.AppServiceURL(), b.httpClient)
+	if err != nil {
+		logrus.WithError(err).Panic("CreateHTTPAppServiceAPIs failed")
+	}
+	return a
 }
 
 // CreateHTTPRoomserverAPIs returns the AliasAPI, InputAPI and QueryAPI for hitting
@@ -105,22 +114,40 @@ func (b *BaseDendrite) CreateHTTPRoomserverAPIs() (
 	roomserverAPI.RoomserverInputAPI,
 	roomserverAPI.RoomserverQueryAPI,
 ) {
-	alias := roomserverAPI.NewRoomserverAliasAPIHTTP(b.Cfg.RoomServerURL(), nil)
+
-	input := roomserverAPI.NewRoomserverInputAPIHTTP(b.Cfg.RoomServerURL(), nil)
+	alias, err := roomserverAPI.NewRoomserverAliasAPIHTTP(b.Cfg.RoomServerURL(), b.httpClient)
-	query := roomserverAPI.NewRoomserverQueryAPIHTTP(b.Cfg.RoomServerURL(), nil)
+	if err != nil {
+		logrus.WithError(err).Panic("NewRoomserverAliasAPIHTTP failed")
+	}
+	input, err := roomserverAPI.NewRoomserverInputAPIHTTP(b.Cfg.RoomServerURL(), b.httpClient)
+	if err != nil {
+		logrus.WithError(err).Panic("NewRoomserverInputAPIHTTP failed", b.httpClient)
+	}
+	query, err := roomserverAPI.NewRoomserverQueryAPIHTTP(b.Cfg.RoomServerURL(), nil)
+	if err != nil {
+		logrus.WithError(err).Panic("NewRoomserverQueryAPIHTTP failed", b.httpClient)
+	}
 	return alias, input, query
 }
 
 // CreateHTTPEDUServerAPIs returns eduInputAPI for hitting the EDU
 // server over HTTP
 func (b *BaseDendrite) CreateHTTPEDUServerAPIs() eduServerAPI.EDUServerInputAPI {
-	return eduServerAPI.NewEDUServerInputAPIHTTP(b.Cfg.EDUServerURL(), nil)
+	e, err := eduServerAPI.NewEDUServerInputAPIHTTP(b.Cfg.EDUServerURL(), nil)
+	if err != nil {
+		logrus.WithError(err).Panic("NewEDUServerInputAPIHTTP failed", b.httpClient)
+	}
+	return e
 }
 
 // CreateHTTPFederationSenderAPIs returns FederationSenderQueryAPI for hitting
 // the federation sender over HTTP
 func (b *BaseDendrite) CreateHTTPFederationSenderAPIs() federationSenderAPI.FederationSenderQueryAPI {
-	return federationSenderAPI.NewFederationSenderQueryAPIHTTP(b.Cfg.FederationSenderURL(), nil)
+	f, err := federationSenderAPI.NewFederationSenderQueryAPIHTTP(b.Cfg.FederationSenderURL(), nil)
+	if err != nil {
+		logrus.WithError(err).Panic("NewFederationSenderQueryAPIHTTP failed", b.httpClient)
+	}
+	return f
 }
 
 // CreateDeviceDB creates a new instance of the device database. Should only be

eduserver/api/input.go

@@ -15,6 +15,7 @@ package api
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	commonHTTP "github.com/matrix-org/dendrite/common/http"
@@ -57,11 +58,11 @@ type EDUServerInputAPI interface {
 const EDUServerInputTypingEventPath = "/api/eduserver/input"
 
 // NewEDUServerInputAPIHTTP creates a EDUServerInputAPI implemented by talking to a HTTP POST API.
-func NewEDUServerInputAPIHTTP(eduServerURL string, httpClient *http.Client) EDUServerInputAPI {
+func NewEDUServerInputAPIHTTP(eduServerURL string, httpClient *http.Client) (EDUServerInputAPI, error) {
 	if httpClient == nil {
-		httpClient = http.DefaultClient
+		return nil, errors.New("NewTypingServerInputAPIHTTP: httpClient is <nil>")
 	}
-	return &httpEDUServerInputAPI{eduServerURL, httpClient}
+	return &httpEDUServerInputAPI{eduServerURL, httpClient}, nil
 }
 
 type httpEDUServerInputAPI struct {

federationsender/api/query.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	commonHTTP "github.com/matrix-org/dendrite/common/http"
@@ -58,12 +59,12 @@ const FederationSenderQueryJoinedHostsInRoomPath = "/api/federationsender/queryJ
 const FederationSenderQueryJoinedHostServerNamesInRoomPath = "/api/federationsender/queryJoinedHostServerNamesInRoom"
 
 // NewFederationSenderQueryAPIHTTP creates a FederationSenderQueryAPI implemented by talking to a HTTP POST API.
-// If httpClient is nil then it uses the http.DefaultClient
+// If httpClient is nil an error is returned
-func NewFederationSenderQueryAPIHTTP(federationSenderURL string, httpClient *http.Client) FederationSenderQueryAPI {
+func NewFederationSenderQueryAPIHTTP(federationSenderURL string, httpClient *http.Client) (FederationSenderQueryAPI, error) {
 	if httpClient == nil {
-		httpClient = http.DefaultClient
+		return nil, errors.New("NewFederationSenderQueryAPIHTTP: httpClient is <nil>")
 	}
-	return &httpFederationSenderQueryAPI{federationSenderURL, httpClient}
+	return &httpFederationSenderQueryAPI{federationSenderURL, httpClient}, nil
 }
 
 type httpFederationSenderQueryAPI struct {

roomserver/api/alias.go

@@ -16,6 +16,7 @@ package api
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	commonHTTP "github.com/matrix-org/dendrite/common/http"
@@ -139,12 +140,12 @@ const RoomserverGetCreatorIDForAliasPath = "/api/roomserver/GetCreatorIDForAlias
 const RoomserverRemoveRoomAliasPath = "/api/roomserver/removeRoomAlias"
 
 // NewRoomserverAliasAPIHTTP creates a RoomserverAliasAPI implemented by talking to a HTTP POST API.
-// If httpClient is nil then it uses the http.DefaultClient
+// If httpClient is nil an error is returned
-func NewRoomserverAliasAPIHTTP(roomserverURL string, httpClient *http.Client) RoomserverAliasAPI {
+func NewRoomserverAliasAPIHTTP(roomserverURL string, httpClient *http.Client) (RoomserverAliasAPI, error) {
 	if httpClient == nil {
-		httpClient = http.DefaultClient
+		return nil, errors.New("NewRoomserverAliasAPIHTTP: httpClient is <nil>")
 	}
-	return &httpRoomserverAliasAPI{roomserverURL, httpClient}
+	return &httpRoomserverAliasAPI{roomserverURL, httpClient}, nil
 }
 
 type httpRoomserverAliasAPI struct {

roomserver/api/input.go

@@ -17,6 +17,7 @@ package api
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	commonHTTP "github.com/matrix-org/dendrite/common/http"
@@ -112,12 +113,12 @@ type RoomserverInputAPI interface {
 const RoomserverInputRoomEventsPath = "/api/roomserver/inputRoomEvents"
 
 // NewRoomserverInputAPIHTTP creates a RoomserverInputAPI implemented by talking to a HTTP POST API.
-// If httpClient is nil then it uses the http.DefaultClient
+// If httpClient is nil an error is returned
-func NewRoomserverInputAPIHTTP(roomserverURL string, httpClient *http.Client) RoomserverInputAPI {
+func NewRoomserverInputAPIHTTP(roomserverURL string, httpClient *http.Client) (RoomserverInputAPI, error) {
 	if httpClient == nil {
-		httpClient = http.DefaultClient
+		return nil, errors.New("NewRoomserverInputAPIHTTP: httpClient is <nil>")
 	}
-	return &httpRoomserverInputAPI{roomserverURL, httpClient}
+	return &httpRoomserverInputAPI{roomserverURL, httpClient}, nil
 }
 
 type httpRoomserverInputAPI struct {

roomserver/api/query.go

@@ -18,6 +18,7 @@ package api
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	commonHTTP "github.com/matrix-org/dendrite/common/http"
@@ -406,12 +407,12 @@ const RoomserverQueryRoomVersionCapabilitiesPath = "/api/roomserver/queryRoomVer
 const RoomserverQueryRoomVersionForRoomPath = "/api/roomserver/queryRoomVersionForRoom"
 
 // NewRoomserverQueryAPIHTTP creates a RoomserverQueryAPI implemented by talking to a HTTP POST API.
-// If httpClient is nil then it uses the http.DefaultClient
+// If httpClient is nil an error is returned
-func NewRoomserverQueryAPIHTTP(roomserverURL string, httpClient *http.Client) RoomserverQueryAPI {
+func NewRoomserverQueryAPIHTTP(roomserverURL string, httpClient *http.Client) (RoomserverQueryAPI, error) {
 	if httpClient == nil {
-		httpClient = http.DefaultClient
+		return nil, errors.New("NewRoomserverQueryAPIHTTP: httpClient is <nil>")
 	}
-	return &httpRoomserverQueryAPI{roomserverURL, httpClient}
+	return &httpRoomserverQueryAPI{roomserverURL, httpClient}, nil
 }
 
 type httpRoomserverQueryAPI struct {