diff --git a/lib/api/completeMultipartUpload.js b/lib/api/completeMultipartUpload.js index 88197d1bfc..7f076fef7a 100644 --- a/lib/api/completeMultipartUpload.js +++ b/lib/api/completeMultipartUpload.js @@ -1,6 +1,6 @@ const async = require('async'); const { parseString } = require('xml2js'); -const { errors, versioning, s3middleware, storage } = require('arsenal'); +const { errors, errorInstances, versioning, s3middleware, storage } = require('arsenal'); const convertToXml = s3middleware.convertToXml; const { pushMetric } = require('../utapi/utilities'); @@ -538,11 +538,14 @@ function completeMultipartUpload(authInfo, request, log, callback) { services.batchDeleteObjectMetadata(mpuBucket.getName(), keysToDelete, log, err => { if (err) { - // Handle specific error cases according to retry strategy if (err.is?.DeleteConflict) { // DeleteConflict should trigger automatic retry // Convert to InternalError to make it retryable - return next(errors.InternalError, extraPartLocations, + const customErr = errorInstances.InternalError.customizeDescription( + 'conflict deleting MPU parts metadata' + ); + + return next(customErr, extraPartLocations, destinationBucket, aggregateETag, generatedVersionId, droppedMPUSize); } @@ -551,6 +554,7 @@ function completeMultipartUpload(authInfo, request, log, callback) { return next(err, extraPartLocations, destinationBucket, aggregateETag, generatedVersionId, droppedMPUSize); } + return next(null, extraPartLocations, destinationBucket, aggregateETag, generatedVersionId, droppedMPUSize); }); diff --git a/lib/services.js b/lib/services.js index ce7c89358b..cbf8740170 100644 --- a/lib/services.js +++ b/lib/services.js @@ -985,7 +985,7 @@ const services = { assert.strictEqual(typeof mpuBucketName, 'string'); async.eachLimit(keysToDelete, 5, (key, callback) => { metadata.deleteObjectMD(mpuBucketName, key, { overheadField: constants.overheadField }, log, callback); - }, err => cb(err)); + }, cb); }, }; diff --git a/package.json b/package.json index 5a8d35efbb..f5bfc9602e 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "dependencies": { "@azure/storage-blob": "^12.28.0", "@hapi/joi": "^17.1.1", - "arsenal": "git+https://github.com/scality/Arsenal#8.2.35", + "arsenal": "git+https://github.com/scality/Arsenal#8.2.37", "async": "2.6.4", "aws-sdk": "^2.1692.0", "bucketclient": "scality/bucketclient#8.2.7", diff --git a/tests/functional/aws-node-sdk/test/object/copyPart.js b/tests/functional/aws-node-sdk/test/object/copyPart.js index f87780b706..87f8f6b0c9 100644 --- a/tests/functional/aws-node-sdk/test/object/copyPart.js +++ b/tests/functional/aws-node-sdk/test/object/copyPart.js @@ -582,85 +582,93 @@ describe('Object Part Copy', () => { it('should not corrupt object if overwriting an existing part by copying a part ' + 'while the MPU is being completed', async () => { - const finalObjETag = '"db77ebbae9e9f5a244a26b86193ad818-1"'; - process.stdout.write('Putting first part in MPU test"'); - const randomDestObjName = `copycatobject${Math.floor(Math.random() * 100000)}`; + const finalObjETag = '"db77ebbae9e9f5a244a26b86193ad818-1"'; + process.stdout.write('Putting first part in MPU test"'); + const randomDestObjName = `copycatobject${Math.floor(Math.random() * 100000)}`; - const initiateRes = await s3 - .createMultipartUpload({ + const initiateRes = await s3 + .createMultipartUpload({ Bucket: destBucketName, Key: randomDestObjName, - }) - .promise(); - const uploadId = initiateRes.UploadId; + }) + .promise(); + const uploadId = initiateRes.UploadId; - const res = await s3 - .uploadPartCopy({ + const res = await s3 + .uploadPartCopy({ Bucket: destBucketName, Key: randomDestObjName, CopySource: `${sourceBucketName}/${sourceObjName}`, PartNumber: 1, UploadId: uploadId, - }) - .promise(); - assert.strictEqual(res.ETag, etag); - assert(res.LastModified); + }) + .promise(); + assert.strictEqual(res.ETag, etag); + assert(res.LastModified); + + process.stdout.write( + 'Overwriting first part in MPU test and completing MPU at the same time', + ); - process.stdout.write( - 'Overwriting first part in MPU test and completing MPU at the same time', - ); - const [completeRes, uploadRes] = await Promise.all([ - s3 + const [completeRes, uploadRes] = await Promise.all([ + s3 .completeMultipartUpload({ - Bucket: destBucketName, - Key: randomDestObjName, - UploadId: uploadId, - MultipartUpload: { + Bucket: destBucketName, + Key: randomDestObjName, + UploadId: uploadId, + MultipartUpload: { Parts: [{ ETag: etag, PartNumber: 1 }], - }, - }) - .promise() - .catch(err => { - throw err; + }, + }).promise() + .catch(async err => { + const raceConditionOccurred = err?.code === 'InternalError' + && err?.message === 'conflict deleting MPU parts metadata'; + + if (raceConditionOccurred) { + return Promise.resolve(null); + } + + throw err; }), - s3 + s3 .uploadPartCopy({ - Bucket: destBucketName, - Key: randomDestObjName, - CopySource: `${sourceBucketName}/${sourceObjName}`, - PartNumber: 1, - UploadId: uploadId, + Bucket: destBucketName, + Key: randomDestObjName, + CopySource: `${sourceBucketName}/${sourceObjName}`, + PartNumber: 1, + UploadId: uploadId, }) .promise() .catch(err => { - const completeMPUFinishedEarlier = - err && err.code === 'NoSuchKey'; - if (completeMPUFinishedEarlier) { - return Promise.resolve(null); - } + const completeMPUFinishedEarlier = err && err.code === 'NoSuchKey'; + if (completeMPUFinishedEarlier) { + return Promise.resolve(null); + } - throw err; + throw err; }), - ]); + ], + ); - if (uploadRes !== null) { - assert.strictEqual(uploadRes.ETag, etag); - assert(uploadRes.LastModified); - } + if (uploadRes !== null) { + assert.strictEqual(uploadRes.ETag, etag); + assert(uploadRes.LastModified); + } + if (completeRes !== null) { assert.strictEqual(completeRes.Bucket, destBucketName); assert.strictEqual(completeRes.Key, randomDestObjName); assert.strictEqual(completeRes.ETag, finalObjETag); - process.stdout.write( - 'Getting object put by MPU with ' + 'overwrite part', - ); - const resGet = await s3 - .getObject({ + } + + process.stdout.write('Getting object put by MPU with overwrite part'); + const resGet = await s3 + .getObject({ Bucket: destBucketName, Key: randomDestObjName, - }) - .promise(); - assert.strictEqual(resGet.ETag, finalObjETag); + }) + .promise(); + assert.strictEqual(resGet.ETag, finalObjETag); }); }); diff --git a/yarn.lock b/yarn.lock index 9edededc50..e1b4a475a6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1451,9 +1451,9 @@ arraybuffer.prototype.slice@^1.0.4: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/Arsenal#8.2.35": - version "8.2.35" - resolved "git+https://github.com/scality/Arsenal#51640ec484e521925e2d3a8b7ce5912f61590b8d" +"arsenal@git+https://github.com/scality/Arsenal#8.2.37": + version "8.2.37" + resolved "git+https://github.com/scality/Arsenal#a000b510e641b819f86d91371f638f1cbc57144f" dependencies: "@azure/identity" "^4.13.0" "@azure/storage-blob" "^12.28.0"