Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: during the sharding upload process, a request id is thrown during abort #1219

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/browser/managed-upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ proto._resumeMultipart = async function _resumeMultipart(checkpoint, options) {
try {
result = await self._uploadPart(name, uploadId, partNo, data, options);
} catch (error) {
if (error.status === 404) {
throw self._makeAbortEvent();
if (error.status === 404 && self.isCancel()) {
throw self._makeAbortEvent(error);
}
throw error;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/common/multipart-copy.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ proto._resumeMultipartCopy = async function _resumeMultipartCopy(checkpoint, sou
try {
result = await self.uploadPartCopy(name, uploadId, partNo, range, source, uploadPartCopyOptions);
} catch (error) {
if (error.status === 404) {
throw self._makeAbortEvent();
if (error.status === 404 && self.isCancel()) {
throw self._makeAbortEvent(error);
}
throw error;
}
Expand Down
5 changes: 3 additions & 2 deletions lib/common/parallel.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,12 @@ proto._makeCancelEvent = function _makeCancelEvent() {
};

// abort is not error , so create an object
proto._makeAbortEvent = function _makeAbortEvent() {
proto._makeAbortEvent = function _makeAbortEvent(error) {
const abortEvent = {
status: 0,
name: 'abort',
message: 'upload task has been abort'
message: 'upload task has been abort',
requestId: error ? error.requestId : undefined
};
return abortEvent;
};
4 changes: 2 additions & 2 deletions lib/managed-upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ proto._resumeMultipart = async function _resumeMultipart(checkpoint, options) {
result = await self._uploadPart(name, uploadId, partNo, data, options);
} catch (error) {
removeStreamFromMultipartUploadStreams();
if (error.status === 404) {
throw self._makeAbortEvent();
if (error.status === 404 && self.isCancel()) {
throw self._makeAbortEvent(error);
}
throw error;
}
Expand Down
9 changes: 8 additions & 1 deletion test/browser/browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1792,9 +1792,13 @@ describe('browser', () => {
const file = new File([fileContent], 'multipart-upload-file');
const name = `${prefix}multipart/upload-file`;
const uploadPart = store._uploadPart;
const requestId = 'KDJSJJSHDEEEEEEWWW';

store._uploadPart = () => {
store._stop();
const e = new Error('TEST Not Found');
e.status = 404;
e.requestId = requestId;
throw e;
};
let netErrs;
Expand All @@ -1803,8 +1807,11 @@ describe('browser', () => {
} catch (err) {
netErrs = err;
}
store.resetCancelFlag();

assert.strictEqual(netErrs.status, 0);
assert.strictEqual(netErrs.name, 'abort');
assert.strictEqual(netErrs.requestId, requestId);
store._uploadPart = uploadPart;
});
});
Expand Down Expand Up @@ -2430,7 +2437,7 @@ describe('browser', () => {
.fill('a')
.join('');
const fileName = new File([fileContent], 'multipart-upload-kms');
const objectKey = 'multipart-upload-file-set-header-browser-test';
const objectKey = `multipart-upload-file-set-header-browser-test-${Date.now()}`;
const req = store.urllib.request;
let header;
mm(store.urllib, 'request', (url, args) => {
Expand Down
26 changes: 17 additions & 9 deletions test/node/multipart.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,21 +641,29 @@ describe('test/multipart.test.js', () => {
it('should request throw abort event', async () => {
const fileName = await utils.createTempFile('multipart-upload-file', 1024 * 1024); // 1m
const name = `${prefix}multipart/upload-file`;
const stubNetError = sinon.stub(store, '_uploadPart');
const netErr = new Error('Not Found');
netErr.status = 404;
netErr.code = 'Not Found';
netErr.name = 'Not Found';
stubNetError.throws(netErr);
const requestId = 'KDJSJJSHDEEEEEEWWW';
mm(store, '_uploadPart', () => {
store._stop();
const netErr = new Error('Not Found');
netErr.status = 404;
netErr.code = 'Not Found';
netErr.name = 'Not Found';
netErr.requestId = requestId;
throw netErr;
});

let netErrs;
try {
await store.multipartUpload(name, fileName);
} catch (err) {
netErrs = err;
}
store.resetCancelFlag();
mm.restore();

assert.strictEqual(netErrs.status, 0);
assert.strictEqual(netErrs.name, 'abort');
store._uploadPart.restore();
assert.strictEqual(netErrs.requestId, requestId);
});
});

Expand Down Expand Up @@ -977,8 +985,8 @@ describe('test/multipart.test.js', () => {
afterEach(mm.restore);

it('Test whether the speed limit setting for sharded upload is effective', async () => {
const file = await utils.createTempFile('multipart-upload-file-set-header', 101 * 1024);
const objectKey = `${prefix}multipart/upload-file-set-header`;
const file = await utils.createTempFile(`multipart-upload-file-set-header-${Date.now()}`, 101 * 1024);
const objectKey = `${prefix}multipart/upload-file-set-header-${Date.now()}`;
const req = store.urllib.request;
let header;
mm(store.urllib, 'request', (url, args) => {
Expand Down
40 changes: 40 additions & 0 deletions test/node/multiversion.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const utils = require('./utils');
const oss = require('../..');
const config = require('../config').oss;
const fs = require('fs');
const mm = require('mm');
const ms = require('humanize-ms');
const { metaSyncTime } = require('../config');

Expand Down Expand Up @@ -284,6 +285,45 @@ describe('test/multiversion.test.js', () => {
assert(false);
}
});

it('should request throw abort event', async () => {
const file = await utils.createTempFile(`multipart-upload-file-abort-${Date.now()}`, 102410);
const objectKey = `${prefix}multipart-copy-source-abort.js`;
const { res: sourceRes } = await store.multipartUpload(objectKey, file);
const versionId = sourceRes.headers['x-oss-version-id'];
store.delete(objectKey);
const copyName = `${prefix}multipart-copy-target-abort.js`;
const requestId = 'KDJSJJSHDEEEEEEWWW';
mm(store, 'uploadPartCopy', () => {
store._stop();
const netErr = new Error('Not Found');
netErr.status = 404;
netErr.requestId = requestId;
throw netErr;
});

let netErrs;
try {
await store.multipartUploadCopy(
copyName,
{
sourceKey: objectKey,
sourceBucketName: bucket
},
{
versionId
}
);
} catch (error) {
netErrs = error;
}
store.resetCancelFlag();
mm.restore();

assert.strictEqual(netErrs.status, 0);
assert.strictEqual(netErrs.name, 'abort');
assert.strictEqual(netErrs.requestId, requestId);
});
});

describe('deleteMulti()', () => {
Expand Down
1 change: 0 additions & 1 deletion test/node/object.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,6 @@ describe('test/object.test.js', () => {
await store.getStream(`${name}not-exists`);
throw new Error('should not run this');
} catch (err) {
console.log('error is', err);
assert.equal(err.name, 'NoSuchKeyError');
assert(Object.keys(store.agent.freeSockets).length === 0);
await utils.sleep(ms(metaSyncTime));
Expand Down
5 changes: 2 additions & 3 deletions test/node/rtmp.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,12 @@ describe('test/rtmp.test.js', () => {
createVodConf.Description = 'this is live channel 4';
const result = await store.putChannel(createVodCid, createVodConf);
assert.equal(result.res.status, 200);
const url = store.getRtmpUrl(createVodCid, {
store.getRtmpUrl(createVodCid, {
params: {
playlistName: 'vod.m3u8'
},
expires: 3600
});
console.log(url);
});

after(async () => {
Expand All @@ -214,7 +213,7 @@ describe('test/rtmp.test.js', () => {

assert.equal(result.res.status, 200);
} catch (err) {
console.error(err);
assert.fail(err);
}
});
});
Expand Down
Loading