use invalid arg code for upload failures (#692)

frostbyte73 · web-flow · commit 7c2ce61a6657 · 2024-06-07T12:41:25.000-07:00
diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go
@@ -143,7 +143,7 @@ func ErrParticipantNotFound(identity string) error {
 // This can have many reasons, some related to invalid parameters, other because of system failure.
 // Do not provide an error code until we have code to analyze the error from the underlying upload library further.
 func ErrUploadFailed(location string, err error) error {
-	return psrpc.NewErrorf(psrpc.Unknown, "%s upload failed: %v", location, err)
+	return psrpc.NewErrorf(psrpc.InvalidArgument, "%s upload failed: %v", location, err)
 }
 
 func ErrCPUExhausted(usage float64) error {
diff --git a/pkg/pipeline/sink/uploader/alioss.go b/pkg/pipeline/sink/uploader/alioss.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/aliyun/aliyun-oss-go-sdk/oss"
 
+	"github.com/livekit/egress/pkg/errors"
 	"github.com/livekit/egress/pkg/types"
 	"github.com/livekit/protocol/livekit"
 )
@@ -37,22 +38,22 @@ func newAliOSSUploader(conf *livekit.AliOSSUpload) (uploader, error) {
 func (u *AliOSSUploader) upload(localFilePath, requestedPath string, _ types.OutputType) (string, int64, error) {
 	stat, err := os.Stat(localFilePath)
 	if err != nil {
-		return "", 0, wrap("AliOSS", err)
+		return "", 0, errors.ErrUploadFailed("AliOSS", err)
 	}
 
 	client, err := oss.New(u.conf.Endpoint, u.conf.AccessKey, u.conf.Secret)
 	if err != nil {
-		return "", 0, wrap("AliOSS", err)
+		return "", 0, errors.ErrUploadFailed("AliOSS", err)
 	}
 
 	bucket, err := client.Bucket(u.conf.Bucket)
 	if err != nil {
-		return "", 0, wrap("AliOSS", err)
+		return "", 0, errors.ErrUploadFailed("AliOSS", err)
 	}
 
 	err = bucket.PutObjectFromFile(requestedPath, localFilePath)
 	if err != nil {
-		return "", 0, wrap("AliOSS", err)
+		return "", 0, errors.ErrUploadFailed("AliOSS", err)
 	}
 
 	return fmt.Sprintf("https://%s.%s/%s", u.conf.Bucket, u.conf.Endpoint, requestedPath), stat.Size(), nil
diff --git a/pkg/pipeline/sink/uploader/azure.go b/pkg/pipeline/sink/uploader/azure.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/Azure/azure-storage-blob-go/azblob"
 
+	"github.com/livekit/egress/pkg/errors"
 	"github.com/livekit/egress/pkg/types"
 	"github.com/livekit/protocol/livekit"
 )
@@ -44,12 +45,12 @@ func (u *AzureUploader) upload(localFilepath, storageFilepath string, outputType
 		u.conf.AccountKey,
 	)
 	if err != nil {
-		return "", 0, wrap("Azure", err)
+		return "", 0, errors.ErrUploadFailed("Azure", err)
 	}
 
 	azUrl, err := url.Parse(u.container)
 	if err != nil {
-		return "", 0, wrap("Azure", err)
+		return "", 0, errors.ErrUploadFailed("Azure", err)
 	}
 
 	pipeline := azblob.NewPipeline(credential, azblob.PipelineOptions{
@@ -65,15 +66,15 @@ func (u *AzureUploader) upload(localFilepath, storageFilepath string, outputType
 
 	file, err := os.Open(localFilepath)
 	if err != nil {
-		return "", 0, wrap("Azure", err)
+		return "", 0, errors.ErrUploadFailed("Azure", err)
 	}
 	defer func() {
 		_ = file.Close()
 	}()
 
 	stat, err := file.Stat()
 	if err != nil {
-		return "", 0, wrap("Azure", err)
+		return "", 0, errors.ErrUploadFailed("Azure", err)
 	}
 
 	// upload blocks in parallel for optimal performance
@@ -84,7 +85,7 @@ func (u *AzureUploader) upload(localFilepath, storageFilepath string, outputType
 		Parallelism:     16,
 	})
 	if err != nil {
-		return "", 0, wrap("Azure", err)
+		return "", 0, errors.ErrUploadFailed("Azure", err)
 	}
 
 	return fmt.Sprintf("%s/%s", u.container, storageFilepath), stat.Size(), nil
diff --git a/pkg/pipeline/sink/uploader/gcp.go b/pkg/pipeline/sink/uploader/gcp.go
@@ -27,6 +27,7 @@ import (
 	"github.com/googleapis/gax-go/v2"
 	"google.golang.org/api/option"
 
+	"github.com/livekit/egress/pkg/errors"
 	"github.com/livekit/egress/pkg/types"
 	"github.com/livekit/protocol/livekit"
 )
@@ -77,15 +78,15 @@ func newGCPUploader(conf *livekit.GCPUpload) (uploader, error) {
 func (u *GCPUploader) upload(localFilepath, storageFilepath string, _ types.OutputType) (string, int64, error) {
 	file, err := os.Open(localFilepath)
 	if err != nil {
-		return "", 0, wrap("GCP", err)
+		return "", 0, errors.ErrUploadFailed("GCP", err)
 	}
 	defer func() {
 		_ = file.Close()
 	}()
 
 	stat, err := file.Stat()
 	if err != nil {
-		return "", 0, wrap("GCP", err)
+		return "", 0, errors.ErrUploadFailed("GCP", err)
 	}
 
 	wc := u.client.Bucket(u.conf.Bucket).Object(storageFilepath).Retryer(
@@ -100,11 +101,11 @@ func (u *GCPUploader) upload(localFilepath, storageFilepath string, _ types.Outp
 	wc.ChunkRetryDeadline = 0
 
 	if _, err = io.Copy(wc, file); err != nil {
-		return "", 0, wrap("GCP", err)
+		return "", 0, errors.ErrUploadFailed("GCP", err)
 	}
 
 	if err = wc.Close(); err != nil {
-		return "", 0, wrap("GCP", err)
+		return "", 0, errors.ErrUploadFailed("GCP", err)
 	}
 
 	return fmt.Sprintf("https://%s.storage.googleapis.com/%s", u.conf.Bucket, storageFilepath), stat.Size(), nil
diff --git a/pkg/pipeline/sink/uploader/s3.go b/pkg/pipeline/sink/uploader/s3.go
@@ -30,6 +30,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/s3/s3manager"
 
 	"github.com/livekit/egress/pkg/config"
+	"github.com/livekit/egress/pkg/errors"
 	"github.com/livekit/egress/pkg/types"
 	"github.com/livekit/protocol/logger"
 	"github.com/livekit/psrpc"
@@ -159,11 +160,11 @@ func (u *S3Uploader) getBucketLocation() (string, error) {
 	svc := s3.New(sess)
 	resp, err := svc.GetBucketLocation(req)
 	if err != nil {
-		return "", psrpc.NewErrorf(psrpc.Unknown, "failed to retrieve upload bucket region: %v", err)
+		return "", psrpc.NewErrorf(psrpc.InvalidArgument, "failed to retrieve upload bucket region: %v", err)
 	}
 
 	if resp.LocationConstraint == nil {
-		return "", psrpc.NewErrorf(psrpc.MalformedResponse, "invalid upload bucket region returned by provider. Try specifying the region manually in the request")
+		return "", psrpc.NewErrorf(psrpc.InvalidArgument, "invalid upload bucket region returned by provider. Try specifying the region manually in the request")
 	}
 
 	return *resp.LocationConstraint, nil
@@ -172,20 +173,20 @@ func (u *S3Uploader) getBucketLocation() (string, error) {
 func (u *S3Uploader) upload(localFilepath, storageFilepath string, outputType types.OutputType) (string, int64, error) {
 	sess, err := session.NewSession(u.awsConfig)
 	if err != nil {
-		return "", 0, wrap("S3", err)
+		return "", 0, errors.ErrUploadFailed("S3", err)
 	}
 
 	file, err := os.Open(localFilepath)
 	if err != nil {
-		return "", 0, wrap("S3", err)
+		return "", 0, errors.ErrUploadFailed("S3", err)
 	}
 	defer func() {
 		_ = file.Close()
 	}()
 
 	stat, err := file.Stat()
 	if err != nil {
-		return "", 0, wrap("S3", err)
+		return "", 0, errors.ErrUploadFailed("S3", err)
 	}
 
 	_, err = s3manager.NewUploader(sess).Upload(&s3manager.UploadInput{
@@ -198,7 +199,7 @@ func (u *S3Uploader) upload(localFilepath, storageFilepath string, outputType ty
 		ContentDisposition: u.contentDisposition,
 	})
 	if err != nil {
-		return "", 0, wrap("S3", err)
+		return "", 0, errors.ErrUploadFailed("S3", err)
 	}
 
 	endpoint := "s3.amazonaws.com"
diff --git a/pkg/pipeline/sink/uploader/uploader.go b/pkg/pipeline/sink/uploader/uploader.go
@@ -15,13 +15,10 @@
 package uploader
 
 import (
-	"fmt"
 	"os"
 	"path"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/livekit/egress/pkg/config"
 	"github.com/livekit/egress/pkg/stats"
 	"github.com/livekit/egress/pkg/types"
@@ -82,11 +79,11 @@ type remoteUploader struct {
 
 func (u *remoteUploader) Upload(localFilepath, storageFilepath string, outputType types.OutputType, deleteAfterUpload bool, fileType string) (string, int64, error) {
 	start := time.Now()
-	location, size, err := u.upload(localFilepath, storageFilepath, outputType)
+	location, size, uploadErr := u.upload(localFilepath, storageFilepath, outputType)
 	elapsed := time.Since(start)
 
 	// success
-	if err == nil {
+	if uploadErr == nil {
 		u.monitor.IncUploadCountSuccess(fileType, float64(elapsed.Milliseconds()))
 		if deleteAfterUpload {
 			_ = os.Remove(localFilepath)
@@ -117,7 +114,7 @@ func (u *remoteUploader) Upload(localFilepath, storageFilepath string, outputTyp
 		return backupFilepath, stat.Size(), nil
 	}
 
-	return "", 0, err
+	return "", 0, uploadErr
 }
 
 type localUploader struct{}
@@ -130,7 +127,3 @@ func (u *localUploader) Upload(localFilepath, _ string, _ types.OutputType, _ bo
 
 	return localFilepath, stat.Size(), nil
 }
-
-func wrap(name string, err error) error {
-	return errors.Wrap(err, fmt.Sprintf("%s upload failed", name))
-}

Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ func ErrParticipantNotFound(identity string) error {`
`143`	`143`	`// This can have many reasons, some related to invalid parameters, other because of system failure.`
`144`	`144`	`// Do not provide an error code until we have code to analyze the error from the underlying upload library further.`
`145`	`145`	`func ErrUploadFailed(location string, err error) error {`
`146`		`- return psrpc.NewErrorf(psrpc.Unknown, "%s upload failed: %v", location, err)`
	`146`	`+ return psrpc.NewErrorf(psrpc.InvalidArgument, "%s upload failed: %v", location, err)`
`147`	`147`	`}`
`148`	`148`
`149`	`149`	`func ErrCPUExhausted(usage float64) error {`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ import (`
`20`	`20`
`21`	`21`	`"github.com/aliyun/aliyun-oss-go-sdk/oss"`
`22`	`22`
	`23`	`+ "github.com/livekit/egress/pkg/errors"`
`23`	`24`	`"github.com/livekit/egress/pkg/types"`
`24`	`25`	`"github.com/livekit/protocol/livekit"`
`25`	`26`	`)`
`@@ -37,22 +38,22 @@ func newAliOSSUploader(conf *livekit.AliOSSUpload) (uploader, error) {`
`37`	`38`	`func (u *AliOSSUploader) upload(localFilePath, requestedPath string, _ types.OutputType) (string, int64, error) {`
`38`	`39`	`stat, err := os.Stat(localFilePath)`
`39`	`40`	`if err != nil {`
`40`		`- return "", 0, wrap("AliOSS", err)`
	`41`	`+ return "", 0, errors.ErrUploadFailed("AliOSS", err)`
`41`	`42`	`}`
`42`	`43`
`43`	`44`	`client, err := oss.New(u.conf.Endpoint, u.conf.AccessKey, u.conf.Secret)`
`44`	`45`	`if err != nil {`
`45`		`- return "", 0, wrap("AliOSS", err)`
	`46`	`+ return "", 0, errors.ErrUploadFailed("AliOSS", err)`
`46`	`47`	`}`
`47`	`48`
`48`	`49`	`bucket, err := client.Bucket(u.conf.Bucket)`
`49`	`50`	`if err != nil {`
`50`		`- return "", 0, wrap("AliOSS", err)`
	`51`	`+ return "", 0, errors.ErrUploadFailed("AliOSS", err)`
`51`	`52`	`}`
`52`	`53`
`53`	`54`	`err = bucket.PutObjectFromFile(requestedPath, localFilePath)`
`54`	`55`	`if err != nil {`
`55`		`- return "", 0, wrap("AliOSS", err)`
	`56`	`+ return "", 0, errors.ErrUploadFailed("AliOSS", err)`
`56`	`57`	`}`
`57`	`58`
`58`	`59`	`return fmt.Sprintf("https://%s.%s/%s", u.conf.Bucket, u.conf.Endpoint, requestedPath), stat.Size(), nil`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@ import (`
`22`	`22`
`23`	`23`	`"github.com/Azure/azure-storage-blob-go/azblob"`
`24`	`24`
	`25`	`+ "github.com/livekit/egress/pkg/errors"`
`25`	`26`	`"github.com/livekit/egress/pkg/types"`
`26`	`27`	`"github.com/livekit/protocol/livekit"`
`27`	`28`	`)`
`@@ -44,12 +45,12 @@ func (u *AzureUploader) upload(localFilepath, storageFilepath string, outputType`
`44`	`45`	`u.conf.AccountKey,`
`45`	`46`	`)`
`46`	`47`	`if err != nil {`
`47`		`- return "", 0, wrap("Azure", err)`
	`48`	`+ return "", 0, errors.ErrUploadFailed("Azure", err)`
`48`	`49`	`}`
`49`	`50`
`50`	`51`	`azUrl, err := url.Parse(u.container)`
`51`	`52`	`if err != nil {`
`52`		`- return "", 0, wrap("Azure", err)`
	`53`	`+ return "", 0, errors.ErrUploadFailed("Azure", err)`
`53`	`54`	`}`
`54`	`55`
`55`	`56`	`pipeline := azblob.NewPipeline(credential, azblob.PipelineOptions{`
`@@ -65,15 +66,15 @@ func (u *AzureUploader) upload(localFilepath, storageFilepath string, outputType`
`65`	`66`
`66`	`67`	`file, err := os.Open(localFilepath)`
`67`	`68`	`if err != nil {`
`68`		`- return "", 0, wrap("Azure", err)`
	`69`	`+ return "", 0, errors.ErrUploadFailed("Azure", err)`
`69`	`70`	`}`
`70`	`71`	`defer func() {`
`71`	`72`	`_ = file.Close()`
`72`	`73`	`}()`
`73`	`74`
`74`	`75`	`stat, err := file.Stat()`
`75`	`76`	`if err != nil {`
`76`		`- return "", 0, wrap("Azure", err)`
	`77`	`+ return "", 0, errors.ErrUploadFailed("Azure", err)`
`77`	`78`	`}`
`78`	`79`
`79`	`80`	`// upload blocks in parallel for optimal performance`
`@@ -84,7 +85,7 @@ func (u *AzureUploader) upload(localFilepath, storageFilepath string, outputType`
`84`	`85`	`Parallelism: 16,`
`85`	`86`	`})`
`86`	`87`	`if err != nil {`
`87`		`- return "", 0, wrap("Azure", err)`
	`88`	`+ return "", 0, errors.ErrUploadFailed("Azure", err)`
`88`	`89`	`}`
`89`	`90`
`90`	`91`	`return fmt.Sprintf("%s/%s", u.container, storageFilepath), stat.Size(), nil`