invisi/gotenberg
1
0
Fork 0
mirror of https://github.com/gotenberg/gotenberg.git synced 2026-07-02 08:27:41 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: rework api and webhook module timeout properties (fixes #396)

Browse Source
This commit is contained in:
Julien Neuhart
2021-12-17 19:13:14 +01:00
parent 4b1791d35c
commit 030b358b0e
8 changed files with 34 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Makefile
+4 -6
View File
@@ -29,9 +29,7 @@ build: ## Build the Gotenberg's Docker image
GOTENBERG_GRACEFUL_SHUTDOWN_DURATION=30s
API_PORT=3000
API_PORT_FROM_ENV=
API_READ_TIMEOUT=30s
API_PROCESS_TIMEOUT=30s
API_WRITE_TIMEOUT=30s
API_TIMEOUT=30s
API_ROOT_PATH=/
API_TRACE_HEADER=Gotenberg-Trace
API_DISABLE_HEALTH_CHECK_LOGGING=false
@@ -61,6 +59,7 @@ WEBHOOK_ERROR_DENY_LIST=
WEBHOOK_MAX_RETRY=4
WEBHOOK_RETRY_MIN_WAIT=1s
WEBHOOK_RETRY_MAX_WAIT=30s
WEBHOOK_CLIENT_TIMEOUT=30s
WEBHOOK_DISABLE=false
.PHONY: run
@@ -72,9 +71,7 @@ run: ## Start a Gotenberg container
--gotenberg-graceful-shutdown-duration=$(GOTENBERG_GRACEFUL_SHUTDOWN_DURATION) \
--api-port=$(API_PORT) \
--api-port-from-env=$(API_PORT_FROM_ENV) \
--api-read-timeout=$(API_READ_TIMEOUT) \
--api-process-timeout=$(API_PROCESS_TIMEOUT) \
--api-write-timeout=$(API_WRITE_TIMEOUT) \
--api-timeout=$(API_TIMEOUT) \
--api-root-path=$(API_ROOT_PATH) \
--api-trace-header=$(API_TRACE_HEADER) \
--api-disable-health-check-logging=$(API_DISABLE_HEALTH_CHECK_LOGGING) \
@@ -104,6 +101,7 @@ run: ## Start a Gotenberg container
--webhook-max-retry=$(WEBHOOK_MAX_RETRY) \
--webhook-retry-min-wait=$(WEBHOOK_RETRY_MIN_WAIT) \
--webhook-retry-max-wait=$(WEBHOOK_RETRY_MAX_WAIT) \
--webhook-client-timeout=$(WEBHOOK_CLIENT_TIMEOUT) \
--webhook-disable=$(WEBHOOK_DISABLE)
.PHONY: build-tests
pkg/modules/api/api.go
+21 -10
View File
@@ -30,8 +30,8 @@ func init() {
type API struct {
port int
readTimeout time.Duration
processTimeout time.Duration
writeTimeout time.Duration
timeout time.Duration
rootPath string
traceHeader string
disableHealthCheckLogging bool
@@ -166,10 +166,20 @@ func (API) Descriptor() gotenberg.ModuleDescriptor {
fs.Duration("api-read-timeout", time.Duration(30)*time.Second, "Set the maximum duration allowed to read a complete request, including the body")
fs.Duration("api-process-timeout", time.Duration(30)*time.Second, "Set the maximum duration allowed to process a request")
fs.Duration("api-write-timeout", time.Duration(30)*time.Second, "Set the maximum duration before timing out writes of the response")
fs.Duration("api-timeout", time.Duration(30)*time.Second, "Set the time limit for requests")
fs.String("api-root-path", "/", "Set the root path of the API - for service discovery via URL paths")
fs.String("api-trace-header", "Gotenberg-Trace", "Set the header name to use for identifying requests")
fs.Bool("api-disable-health-check-logging", false, "Disable health check logging")
var err error
err = multierr.Append(err, fs.MarkDeprecated("api-read-timeout", "use api-timeout instead"))
err = multierr.Append(err, fs.MarkDeprecated("api-process-timeout", "use api-timeout instead"))
err = multierr.Append(err, fs.MarkDeprecated("api-write-timeout", "use api-timeout instead"))
if err != nil {
panic(fmt.Errorf("create deprecated flags for the api module: %v", err))
}
return fs
}(),
New: func() gotenberg.Module { return new(API) },
@@ -180,9 +190,9 @@ func (API) Descriptor() gotenberg.ModuleDescriptor {
func (a *API) Provision(ctx *gotenberg.Context) error {
flags := ctx.ParsedFlags()
a.port = flags.MustInt("api-port")
a.readTimeout = flags.MustDuration("api-read-timeout")
a.processTimeout = flags.MustDuration("api-process-timeout")
a.writeTimeout = flags.MustDuration("api-write-timeout")
a.readTimeout = flags.MustDeprecatedDuration("api-read-timeout", "api-timeout")
a.writeTimeout = flags.MustDeprecatedDuration("api-write-timeout", "api-timeout")
a.timeout = flags.MustDeprecatedDuration("api-process-timeout", "api-timeout")
a.rootPath = flags.MustString("api-root-path")
a.traceHeader = flags.MustString("api-trace-header")
a.disableHealthCheckLogging = flags.MustBool("api-disable-health-check-logging")
@@ -274,7 +284,7 @@ func (a *API) Provision(ctx *gotenberg.Context) error {
}
// Grace duration.
a.gcGraceDuration = a.readTimeout + a.processTimeout + a.writeTimeout
a.gcGraceDuration = a.timeout
mods, err = ctx.Modules(new(GarbageCollectorGraceDurationIncrementer))
if err != nil {
@@ -378,7 +388,9 @@ func (a *API) Start() error {
a.srv.HideBanner = true
a.srv.HidePort = true
a.srv.Server.ReadTimeout = a.readTimeout
a.srv.Server.WriteTimeout = a.writeTimeout
a.srv.Server.IdleTimeout = a.timeout
// See https://github.com/gotenberg/gotenberg/issues/396.
a.srv.Server.WriteTimeout = a.writeTimeout + a.writeTimeout
a.srv.HTTPErrorHandler = httpErrorHandler()
// Let's prepare the modules' routes.
@@ -402,7 +414,6 @@ func (a *API) Start() error {
latencyMiddleware(),
rootPathMiddleware(a.rootPath),
traceMiddleware(a.traceHeader),
timeoutsMiddleware(a.readTimeout, a.processTimeout, a.writeTimeout),
loggerMiddleware(a.logger, disableLoggingForPaths),
)
@@ -419,14 +430,14 @@ func (a *API) Start() error {
}
}
hardTimeout := a.processTimeout + (time.Duration(5) * time.Second)
hardTimeout := a.timeout + (time.Duration(5) * time.Second)
// Add the modules' routes and their specific middlewares.
for _, route := range a.routes {
var middlewares []echo.MiddlewareFunc
if route.IsMultipart {
middlewares = append(middlewares, contextMiddleware(a.processTimeout))
middlewares = append(middlewares, contextMiddleware(a.timeout))
for _, externalMultipartMiddleware := range externalMultipartMiddlewares {
middlewares = append(middlewares, externalMultipartMiddleware.Handler)
@@ -447,7 +458,7 @@ func (a *API) Start() error {
a.srv.GET(
fmt.Sprintf("%s%s", a.rootPath, "health"),
func() echo.HandlerFunc {
checks := append(a.healthChecks, health.WithTimeout(a.processTimeout))
checks := append(a.healthChecks, health.WithTimeout(a.timeout))
checker := health.NewChecker(checks...)
return echo.WrapHandler(health.NewHandler(checker))
pkg/modules/api/api_test.go
+1 -1
View File
@@ -386,7 +386,7 @@ func TestAPI_Provision(t *testing.T) {
},
)
}(),
expectGraceDuration: time.Duration(93) * time.Second,
expectGraceDuration: time.Duration(33) * time.Second,
expectErr: true,
},
{
pkg/modules/api/middlewares.go
+2 -21
View File
@@ -120,25 +120,6 @@ func traceMiddleware(header string) echo.MiddlewareFunc {
}
}
// timeoutsMiddleware sets the read, process and write timeouts in the
// echo.Context under "readTimeout", "processTimeout" and "writeTimeout".
//
// readTimeout := c.Get("readTimeout").(time.Duration)
// processTimeout := c.Get("processTimeout").(time.Duration)
// writeTimeout := c.Get("writeTimeout").(time.Duration)
func timeoutsMiddleware(readTimeout, processTimeout, writeTimeout time.Duration) echo.MiddlewareFunc {
return func(next echo.HandlerFunc) echo.HandlerFunc {
return func(c echo.Context) error {
c.Set("readTimeout", readTimeout)
c.Set("processTimeout", processTimeout)
c.Set("writeTimeout", writeTimeout)
// Call the next middleware in the chain.
return next(c)
}
}
}
// loggerMiddleware sets the logger in the echo.Context under "logger" and logs
// a synchronous request result.
//
@@ -218,14 +199,14 @@ func loggerMiddleware(logger *zap.Logger, disableLoggingForPaths []string) echo.
//
// ctx := c.Get("context").(*api.Context)
// cancel := c.Get("cancel").(context.CancelFunc)
func contextMiddleware(processTimeout time.Duration) echo.MiddlewareFunc {
func contextMiddleware(timeout time.Duration) echo.MiddlewareFunc {
return func(next echo.HandlerFunc) echo.HandlerFunc {
return func(c echo.Context) error {
logger := c.Get("logger").(*zap.Logger)
// We create a context with a timeout so that underlying processes are
// able to stop early and handle correctly a timeout scenario.
ctx, cancel, err := newContext(c, logger, processTimeout)
ctx, cancel, err := newContext(c, logger, timeout)
if err != nil {
cancel()
pkg/modules/api/middlewares_test.go
-41
View File
@@ -222,47 +222,6 @@ func TestTraceMiddleware(t *testing.T) {
}
}
func TestTimeoutsMiddleware(t *testing.T) {
recorder := httptest.NewRecorder()
request := httptest.NewRequest(http.MethodGet, "/foo", nil)
srv := echo.New()
srv.HideBanner = true
srv.HidePort = true
c := srv.NewContext(request, recorder)
expectReadTimeout := time.Duration(1) * time.Second
expectProcessTimeout := time.Duration(2) * time.Second
expectWriteTimeout := time.Duration(3) * time.Second
err := timeoutsMiddleware(expectReadTimeout, expectProcessTimeout, expectWriteTimeout)(
func(c echo.Context) error {
return nil
},
)(c)
if err != nil {
t.Fatalf("expected no error but got: %v", err)
}
actualReadTimeout := c.Get("readTimeout").(time.Duration)
actualProcessTimeout := c.Get("processTimeout").(time.Duration)
actualWriteTimeout := c.Get("writeTimeout").(time.Duration)
if actualReadTimeout != expectReadTimeout {
t.Errorf("expected '%s' but got '%s", expectReadTimeout, actualReadTimeout)
}
if actualProcessTimeout != expectProcessTimeout {
t.Errorf("expected '%s' but got '%s", expectProcessTimeout, actualProcessTimeout)
}
if actualWriteTimeout != expectWriteTimeout {
t.Errorf("expected '%s' but got '%s", actualWriteTimeout, expectWriteTimeout)
}
}
func TestLoggerMiddleware(t *testing.T) {
for i, tc := range []struct {
request *http.Request
pkg/modules/webhook/middleware.go
+1 -1
View File
@@ -143,7 +143,7 @@ func webhookMiddleware(w Webhook) api.Middleware {
client: &retryablehttp.Client{
HTTPClient: &http.Client{
Timeout: c.Get("writeTimeout").(time.Duration),
Timeout: w.clientTimeout,
},
RetryMax: w.maxRetry,
RetryWaitMin: w.retryMinWait,
pkg/modules/webhook/middleware_test.go
+1 -1
View File
@@ -324,6 +324,7 @@ func TestWebhookMiddlewareAsynchronousProcess(t *testing.T) {
maxRetry: 0,
retryMinWait: 0,
retryMaxWait: 0,
clientTimeout: time.Duration(30) * time.Second,
disable: false,
}
}
@@ -397,7 +398,6 @@ func TestWebhookMiddlewareAsynchronousProcess(t *testing.T) {
c.Set("traceHeader", "Gotenberg-Trace")
c.Set("trace", "foo")
c.Set("startTime", time.Now())
c.Set("writeTimeout", time.Duration(10)*time.Second)
ctx := &api.MockContext{Context: &api.Context{}}
ctx.SetLogger(zap.NewNop())
pkg/modules/webhook/webhook.go
+4 -1
View File
@@ -25,6 +25,7 @@ type Webhook struct {
maxRetry int
retryMinWait time.Duration
retryMaxWait time.Duration
clientTimeout time.Duration
disable bool
}
@@ -55,7 +56,7 @@ func (Webhook) Descriptor() gotenberg.ModuleDescriptor {
err = multierr.Append(err, fs.MarkDeprecated("api-disable-webhook", "use webhook-disable instead"))
if err != nil {
panic(fmt.Errorf("create deprecated flags for webhook module: %v", err))
panic(fmt.Errorf("create deprecated flags for the webhook module: %v", err))
}
// New flags.
@@ -66,6 +67,7 @@ func (Webhook) Descriptor() gotenberg.ModuleDescriptor {
fs.Int("webhook-max-retry", 4, "Set the maximum number of retries for the webhook feature")
fs.Duration("webhook-retry-min-wait", time.Duration(1)*time.Second, "Set the minimum duration to wait before trying to call the webhook again")
fs.Duration("webhook-retry-max-wait", time.Duration(30)*time.Second, "Set the maximum duration to wait before trying to call the webhook again")
fs.Duration("webhook-client-timeout", time.Duration(30)*time.Second, "Set the time limit for requests to the webhook")
fs.Bool("webhook-disable", false, "Disable the webhook feature")
return fs
@@ -84,6 +86,7 @@ func (w *Webhook) Provision(ctx *gotenberg.Context) error {
w.maxRetry = flags.MustDeprecatedInt("api-webhook-max-retry", "webhook-max-retry")
w.retryMinWait = flags.MustDeprecatedDuration("api-webhook-retry-min-wait", "webhook-retry-min-wait")
w.retryMaxWait = flags.MustDeprecatedDuration("api-webhook-retry-min-wait", "webhook-retry-max-wait")
w.clientTimeout = flags.MustDuration("webhook-client-timeout")
w.disable = flags.MustDeprecatedBool("api-disable-webhook", "webhook-disable")
return nil