diff --git a/asyncbuffer/buffer.go b/asyncbuffer/buffer.go index abdc023e..f9b3d139 100644 --- a/asyncbuffer/buffer.go +++ b/asyncbuffer/buffer.go @@ -22,6 +22,7 @@ import ( "github.com/sirupsen/logrus" + "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/ioutil" ) @@ -108,6 +109,17 @@ func (ab *AsyncBuffer) callFinishFn() { }) } +func (ab *AsyncBuffer) setErr(err error) { + if err == nil { + return + } + + // If the error is already set, we do not overwrite it + if ab.err.Load() == nil { + ab.err.Store(ierrors.Wrap(err, 1)) + } +} + // addChunk adds a new chunk to the AsyncBuffer, increments bytesRead // and signals that a chunk is ready func (ab *AsyncBuffer) addChunk(chunk *byteChunk) { @@ -139,10 +151,10 @@ func (ab *AsyncBuffer) readChunks() { logrus.WithField("source", "asyncbuffer.AsyncBuffer.readChunks").Warningf("error closing upstream reader: %s", err) } - if ab.bytesRead.Load() < int64(ab.dataLen) && ab.err.Load() == nil { + if ab.bytesRead.Load() < int64(ab.dataLen) { // If the reader has finished reading and we have not read enough data, // set err to io.ErrUnexpectedEOF - ab.err.Store(io.ErrUnexpectedEOF) + ab.setErr(io.ErrUnexpectedEOF) } ab.callFinishFn() @@ -171,7 +183,7 @@ func (ab *AsyncBuffer) readChunks() { // If the pool is empty, it will create a new byteChunk with ChunkSize chunk, ok := chunkPool.Get().(*byteChunk) if !ok { - ab.err.Store(errors.New("asyncbuffer.AsyncBuffer.readChunks: failed to get chunk from pool")) + ab.setErr(errors.New("asyncbuffer.AsyncBuffer.readChunks: failed to get chunk from pool")) return } @@ -182,7 +194,7 @@ func (ab *AsyncBuffer) readChunks() { // If it's not the EOF, we need to store the error if err != nil && err != io.EOF { - ab.err.Store(err) + ab.setErr(err) chunkPool.Put(chunk) return } diff --git a/imagedata/errors.go b/imagedata/errors.go new file mode 100644 index 00000000..e719361a --- /dev/null +++ b/imagedata/errors.go @@ -0,0 +1,14 @@ +package imagedata + +import ( + "fmt" + + "github.com/imgproxy/imgproxy/v3/ierrors" +) + +func wrapDownloadError(err error, desc string) error { + return ierrors.Wrap( + err, 0, + ierrors.WithPrefix(fmt.Sprintf("can't download %s", desc)), + ) +} diff --git a/imagedata/factory.go b/imagedata/factory.go index 4d75ac5c..da897b6b 100644 --- a/imagedata/factory.go +++ b/imagedata/factory.go @@ -4,13 +4,11 @@ import ( "bytes" "context" "encoding/base64" - "fmt" "io" "net/http" "os" "github.com/imgproxy/imgproxy/v3/asyncbuffer" - "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/imagefetcher" "github.com/imgproxy/imgproxy/v3/imagetype" "github.com/imgproxy/imgproxy/v3/security" @@ -105,7 +103,7 @@ func sendRequest(ctx context.Context, url string, opts DownloadOptions) (*imagef } // DownloadSync downloads the image synchronously and returns the ImageData and HTTP headers. -func downloadSync(ctx context.Context, imageURL string, opts DownloadOptions) (ImageData, http.Header, error) { +func DownloadSync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { if opts.DownloadFinished != nil { defer opts.DownloadFinished() } @@ -120,26 +118,26 @@ func downloadSync(ctx context.Context, imageURL string, opts DownloadOptions) (I } if err != nil { - return nil, h, err + return nil, h, wrapDownloadError(err, desc) } b, err := io.ReadAll(res.Body) if err != nil { - return nil, h, err + return nil, h, wrapDownloadError(err, desc) } format, err := imagetype.Detect(bytes.NewReader(b)) if err != nil { - return nil, h, err + return nil, h, wrapDownloadError(err, desc) } d := NewFromBytesWithFormat(format, b) - return d, h, err + return d, h, nil } -// downloadAsync downloads the image asynchronously and returns the ImageData +// DownloadAsync downloads the image asynchronously and returns the ImageData // backed by AsyncBuffer and HTTP headers. -func downloadAsync(ctx context.Context, imageURL string, opts DownloadOptions) (ImageData, http.Header, error) { +func DownloadAsync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { // We pass this responsibility to AsyncBuffer //nolint:bodyclose req, res, h, err := sendRequest(ctx, imageURL, opts) @@ -147,7 +145,7 @@ func downloadAsync(ctx context.Context, imageURL string, opts DownloadOptions) ( if opts.DownloadFinished != nil { defer opts.DownloadFinished() } - return nil, h, err + return nil, h, wrapDownloadError(err, desc) } b := asyncbuffer.New(res.Body, int(res.ContentLength), opts.DownloadFinished) @@ -156,43 +154,16 @@ func downloadAsync(ctx context.Context, imageURL string, opts DownloadOptions) ( if err != nil { b.Close() req.Cancel() - return nil, h, err + return nil, h, wrapDownloadError(err, desc) } d := &imageDataAsyncBuffer{ b: b, format: format, + desc: desc, cancel: nil, } d.AddCancel(req.Cancel) // request will be closed when the image data is consumed - return d, h, err -} - -// DownloadSyncWithDesc downloads the image synchronously and returns the ImageData, but -// wraps errors with desc. -func DownloadSync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { - imgdata, h, err := downloadSync(ctx, imageURL, opts) - if err != nil { - return nil, h, ierrors.Wrap( - err, 0, - ierrors.WithPrefix(fmt.Sprintf("can't download %s", desc)), - ) - } - - return imgdata, h, nil -} - -// DownloadSyncWithDesc downloads the image synchronously and returns the ImageData, but -// wraps errors with desc. -func DownloadAsync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { - imgdata, h, err := downloadAsync(ctx, imageURL, opts) - if err != nil { - return nil, h, ierrors.Wrap( - err, 0, - ierrors.WithPrefix(fmt.Sprintf("can't download %s", desc)), - ) - } - - return imgdata, h, nil + return d, h, nil } diff --git a/imagedata/image_data.go b/imagedata/image_data.go index 39e557a7..b40a1f5e 100644 --- a/imagedata/image_data.go +++ b/imagedata/image_data.go @@ -47,6 +47,7 @@ type imageDataBytes struct { type imageDataAsyncBuffer struct { b *asyncbuffer.AsyncBuffer format imagetype.Type + desc string cancel []context.CancelFunc cancelOnce sync.Once } @@ -123,7 +124,10 @@ func (d *imageDataAsyncBuffer) AddCancel(cancel context.CancelFunc) { // Error returns any error that occurred during reading data from // async buffer or the underlying source. func (d *imageDataAsyncBuffer) Error() error { - return d.b.Error() + if err := d.b.Error(); err != nil { + return wrapDownloadError(err, d.desc) + } + return nil } func Init() error { diff --git a/processing_handler.go b/processing_handler.go index 334a7d2c..4a8d16cd 100644 --- a/processing_handler.go +++ b/processing_handler.go @@ -462,8 +462,8 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) err } // First, check if the processing error wasn't caused by an image data error - if originData.Error() != nil { - return ierrors.Wrap(originData.Error(), 0, ierrors.WithCategory(categoryDownload)) + if derr := originData.Error(); derr != nil { + return ierrors.Wrap(derr, 0, ierrors.WithCategory(categoryDownload)) } // If it wasn't, than it was a processing error