Return 413 HTTP status for S3 uploads if max upload limit is reached

When an upload (e.g. a CI artifact) reaches the maximum file size limit,
uploads via S3 would return a 500 error to the user. This made it
difficult to understand why the upload failed.

This was happening because the `hardLimitReader` was aborting the
transfer with `ErrEntityTooLarge`, but this error was wrapped in layers
of AWS errors. Since none of these AWS errors were understood by the
file handler, a 500 error was returned.

To fix this, AWS has a way to retrieve the original error. We now
recursively go down the error stack to find the root cause.

Note that there is an open issue in the AWS SDK to make this easier with
Golang (https://github.com/aws/aws-sdk-go/issues/2820).

changelogs/unreleased/sh-return-413-for-s3-artifact-uploads.yml

+---
+title: Return 413 HTTP status for S3 uploads if max upload limit is reached
+merge_request: 655
+author:
+type: fixed

internal/objectstore/s3_object.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/aws/aws-sdk-go/service/s3/s3manager"
 	"gitlab.com/gitlab-org/labkit/log"
@@ -63,7 +64,8 @@ func (s *S3Object) Upload(ctx context.Context, r io.Reader) error {
 	_, err = uploader.UploadWithContext(ctx, input)
 	if err != nil {
 		log.WithError(err).Error("error uploading S3 session")
-		return err
+		// Get the root cause, such as ErrEntityTooLarge, so we can return the proper HTTP status code
+		return unwrapAWSError(err)
 	}
 
 	s.uploaded = true
@@ -106,3 +108,12 @@ func (s *S3Object) Delete() {
 		log.WithError(err).Error("error deleting S3 object", err)
 	}
 }
+
+// This is needed until https://github.com/aws/aws-sdk-go/issues/2820 is closed.
+func unwrapAWSError(e error) error {
+	if awsErr, ok := e.(awserr.Error); ok {
+		return unwrapAWSError(awsErr.OrigErr())
+	}
+
+	return e
+}

internal/objectstore/s3_object_test.go

@@ -141,4 +141,5 @@ func TestS3ObjectUploadCancel(t *testing.T) {
 
 	_, err = object.Consume(ctx, strings.NewReader(test.ObjectContent), deadline)
 	require.Error(t, err)
+	require.Equal(t, "context canceled", err.Error())
 }