From e867d0f10a80e86a773b199e3546bb272340c4ba Mon Sep 17 00:00:00 2001 From: DarthSim Date: Mon, 3 Jun 2024 20:10:46 +0300 Subject: [PATCH] Add write response timeout --- .golangci.yml | 3 +++ CHANGELOG.md | 3 ++- config/config.go | 6 ++++++ processing_handler.go | 31 ++++++++++++++++++--------- router/router.go | 2 ++ router/timeout_response.go | 43 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 router/timeout_response.go diff --git a/.golangci.yml b/.golangci.yml index 70b41c3e..212f1f09 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -33,6 +33,9 @@ issues: - linters: [bodyclose] path: ".*_test.go" + - linters: [bodyclose] + path: "router/timeout_response.go" + # False positives on CGO generated code - linters: [staticcheck] text: "SA4000:" diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d389604..2f390ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,10 @@ ## [Unreleased] ### Add - Add [IMGPROXY_S3_ASSUME_ROLE_EXTERNAL_ID](https://docs.imgproxy.net/latest/configuration/options#IMGPROXY_S3_ASSUME_ROLE_EXTERNAL_ID) config. +- Add [IMGPROXY_WRITE_RESPONSE_TIMEOUT](https://docs.imgproxy.net/latest/configuration/options#IMGPROXY_WRITE_RESPONSE_TIMEOUT) config. +- Add [IMGPROXY_REPORT_IO_ERRORS](https://docs.imgproxy.net/latest/configuration/options#IMGPROXY_REPORT_IO_ERRORS) config. - (pro) Add [colorize](https://docs.imgproxy.net/latest/usage/processing#colorize) processing option. - ### Changed - Automatically add `http://` scheme to the `IMGPROXY_S3_ENDPOINT` value if it has no scheme. - Trim redundant slashes in the S3 object key. diff --git a/config/config.go b/config/config.go index 5109a9ee..78090d6f 100644 --- a/config/config.go +++ b/config/config.go @@ -23,6 +23,7 @@ var ( Bind string ReadTimeout int WriteTimeout int + WriteResponseTimeout int KeepAliveTimeout int ClientKeepAliveTimeout int DownloadTimeout int @@ -188,6 +189,7 @@ var ( AirbrakeEnv string ReportDownloadingErrors bool + ReportIOErrors bool EnableDebugHeaders bool @@ -217,6 +219,7 @@ func Reset() { Bind = ":8080" ReadTimeout = 10 WriteTimeout = 10 + WriteResponseTimeout = 10 KeepAliveTimeout = 10 ClientKeepAliveTimeout = 90 DownloadTimeout = 5 @@ -380,6 +383,7 @@ func Reset() { AirbrakeEnv = "production" ReportDownloadingErrors = true + ReportIOErrors = false EnableDebugHeaders = false @@ -401,6 +405,7 @@ func Configure() error { configurators.String(&Bind, "IMGPROXY_BIND") configurators.Int(&ReadTimeout, "IMGPROXY_READ_TIMEOUT") configurators.Int(&WriteTimeout, "IMGPROXY_WRITE_TIMEOUT") + configurators.Int(&WriteResponseTimeout, "IMGPROXY_WRITE_RESPONSE_TIMEOUT") configurators.Int(&KeepAliveTimeout, "IMGPROXY_KEEP_ALIVE_TIMEOUT") configurators.Int(&ClientKeepAliveTimeout, "IMGPROXY_CLIENT_KEEP_ALIVE_TIMEOUT") configurators.Int(&DownloadTimeout, "IMGPROXY_DOWNLOAD_TIMEOUT") @@ -597,6 +602,7 @@ func Configure() error { configurators.String(&AirbrakeProjecKey, "IMGPROXY_AIRBRAKE_PROJECT_KEY") configurators.String(&AirbrakeEnv, "IMGPROXY_AIRBRAKE_ENVIRONMENT") configurators.Bool(&ReportDownloadingErrors, "IMGPROXY_REPORT_DOWNLOADING_ERRORS") + configurators.Bool(&ReportIOErrors, "IMGPROXY_REPORT_IO_ERRORS") configurators.Bool(&EnableDebugHeaders, "IMGPROXY_ENABLE_DEBUG_HEADERS") configurators.Int(&FreeMemoryInterval, "IMGPROXY_FREE_MEMORY_INTERVAL") diff --git a/processing_handler.go b/processing_handler.go index ab901201..ef46a237 100644 --- a/processing_handler.go +++ b/processing_handler.go @@ -143,10 +143,21 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, sta rw.Header().Set("Content-Length", strconv.Itoa(len(resultData.Data))) rw.WriteHeader(statusCode) - rw.Write(resultData.Data) + _, err := rw.Write(resultData.Data) + + var ierr *ierrors.Error + if err != nil { + ierr = ierrors.New(statusCode, fmt.Sprintf("Failed to write response: %s", err), "Failed to write response") + ierr.Unexpected = true + + if config.ReportIOErrors { + sendErr(r.Context(), "IO", ierr) + errorreport.Report(ierr, r) + } + } router.LogResponse( - reqID, r, statusCode, nil, + reqID, r, statusCode, ierr, log.Fields{ "image_url": originURL, "processing_options": po, @@ -204,14 +215,6 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if queueSem != nil { - acquired := queueSem.TryAcquire(1) - if !acquired { - panic(ierrors.New(429, "Too many requests", "Too many requests")) - } - defer queueSem.Release(1) - } - path := r.RequestURI if queryStart := strings.IndexByte(path, '?'); queryStart >= 0 { path = path[:queryStart] @@ -282,6 +285,14 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { } } + if queueSem != nil { + acquired := queueSem.TryAcquire(1) + if !acquired { + panic(ierrors.New(429, "Too many requests", "Too many requests")) + } + defer queueSem.Release(1) + } + // The heavy part starts here, so we need to restrict worker number func() { defer metrics.StartQueueSegment(ctx)() diff --git a/router/router.go b/router/router.go index e8cff6ba..98dabaea 100644 --- a/router/router.go +++ b/router/router.go @@ -95,6 +95,8 @@ func (r *Router) ServeHTTP(rw http.ResponseWriter, req *http.Request) { req, timeoutCancel := startRequestTimer(req) defer timeoutCancel() + rw = newTimeoutResponse(rw) + reqID := req.Header.Get(xRequestIDHeader) if len(reqID) == 0 || !requestIDRe.MatchString(reqID) { diff --git a/router/timeout_response.go b/router/timeout_response.go new file mode 100644 index 00000000..1a1ca2c0 --- /dev/null +++ b/router/timeout_response.go @@ -0,0 +1,43 @@ +package router + +import ( + "net/http" + "time" + + "github.com/imgproxy/imgproxy/v3/config" +) + +type timeoutResponse struct { + http.ResponseWriter + controller *http.ResponseController +} + +func newTimeoutResponse(rw http.ResponseWriter) http.ResponseWriter { + return &timeoutResponse{ + ResponseWriter: rw, + controller: http.NewResponseController(rw), + } +} + +func (rw *timeoutResponse) WriteHeader(statusCode int) { + rw.withWriteDeadline(func() { + rw.ResponseWriter.WriteHeader(statusCode) + }) +} + +func (rw *timeoutResponse) Write(b []byte) (int, error) { + var ( + n int + err error + ) + rw.withWriteDeadline(func() { + n, err = rw.ResponseWriter.Write(b) + }) + return n, err +} + +func (rw *timeoutResponse) withWriteDeadline(f func()) { + rw.controller.SetWriteDeadline(time.Now().Add(time.Duration(config.WriteResponseTimeout) * time.Second)) + defer rw.controller.SetWriteDeadline(time.Time{}) + f() +}