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.

Sign up for GitHub

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

Jump to bottom

Feat: custom timeout config #373

Closed
wants to merge 6 commits into from
Closed

Feat: custom timeout config #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a96a275
make request timeout configurable
fengkx May 10, 2019
df50596
corect test
fengkx May 10, 2019
e21a319
correct timeout log
fengkx May 10, 2019
5f56836
fix lint
fengkx May 14, 2019
8965107
change env name to HTTP_CLIENT_TIMEOUT
fengkx May 15, 2019
917a3e9
change env in test
fengkx May 17, 2019
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
24 changes: 17 additions & 7 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"strconv"
"strings"
"time"

"miniflux.app/logger"
)
Expand All @@ -33,14 +34,16 @@ const (
defaultOAuth2ClientSecret = ""
defaultOAuth2RedirectURL = ""
defaultOAuth2Provider = ""
defaultRequestTimeout = 20 //Second
)

// Config manages configuration parameters.
type Config struct {
IsHTTPS bool
baseURL string
rootURL string
basePath string
IsHTTPS bool
baseURL string
rootURL string
basePath string
requestTimout int
}

func (c *Config) parseBaseURL() {
Expand Down Expand Up @@ -230,12 +233,19 @@ func (c *Config) ArchiveReadDays() int {
return getIntValue("ARCHIVE_READ_DAYS", defaultArchiveReadDays)
}

// RequestTimeout returns a Duration.time represent the request timeout
func (c *Config) RequestTimeout() time.Duration {
requestTimout := getIntValue("HTTP_CLIENT_TIMEOUT", defaultRequestTimeout)
return time.Duration(requestTimout * int(time.Second))
}

// NewConfig returns a new Config.
func NewConfig() *Config {
cfg := &Config{
baseURL: defaultBaseURL,
rootURL: defaultBaseURL,
IsHTTPS: getBooleanValue("HTTPS"),
baseURL: defaultBaseURL,
rootURL: defaultBaseURL,
IsHTTPS: getBooleanValue("HTTPS"),
requestTimout: defaultRequestTimeout,
}

cfg.parseBaseURL()
Expand Down
11 changes: 11 additions & 0 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package config // import "miniflux.app/config"
import (
"os"
"testing"
"time"
)

func TestGetBooleanValueWithUnsetVariable(t *testing.T) {
Expand Down Expand Up @@ -838,3 +839,13 @@ func TestHTTPSOn(t *testing.T) {
t.Fatalf(`Unexpected HTTPS value, got "%v"`, cfg.IsHTTPS)
}
}

func TestRequestTimeout(t *testing.T) {
os.Clearenv()
os.Setenv("HTTP_CLIENT_TIMEOUT", "500")
cfg := NewConfig()
timeout := cfg.RequestTimeout()
if timeout != 500 * time.Second {
t.Fatalf(`Unexpected RequestTimeout value, got "%v"`, timeout)
}
}
10 changes: 5 additions & 5 deletions http/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"fmt"
"io"
"io/ioutil"
"miniflux.app/config"
"net"
"net/http"
"net/url"
Expand All @@ -25,13 +26,12 @@ import (
)

const (
// 20 seconds max.
requestTimeout = 20

// 15MB max.
maxBodySize = 1024 * 1024 * 15
)

var cfg = config.NewConfig()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum.. I should refactor this config package. This is going to parse the config values twice during the process startup.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it when I wrote this.

It seem the config never change after the process startup. So it is OK to work though not so good. For now I cannot find out a way without parsing the config value again. Looking forward to your good news


var (
// DefaultUserAgent sets the User-Agent header used for any requests by miniflux.
DefaultUserAgent = "Mozilla/5.0 (compatible; Miniflux/" + version.Version + "; +https://miniflux.app)"
Expand Down Expand Up @@ -144,7 +144,7 @@ func (c *Client) executeRequest(request *http.Request) (*Response, error) {
case net.Error:
nerr := uerr.Err.(net.Error)
if nerr.Timeout() {
err = errors.NewLocalizedError(errRequestTimeout, requestTimeout)
err = errors.NewLocalizedError(errRequestTimeout, cfg.RequestTimeout().Seconds())
} else if nerr.Temporary() {
err = errors.NewLocalizedError(errTemporaryNetworkOperation, nerr)
}
Expand Down Expand Up @@ -212,7 +212,7 @@ func (c *Client) buildRequest(method string, body io.Reader) (*http.Request, err
}

func (c *Client) buildClient() http.Client {
client := http.Client{Timeout: time.Duration(requestTimeout * time.Second)}
client := http.Client{Timeout: cfg.RequestTimeout()}
if c.Insecure {
client.Transport = &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
Expand Down