diff --git a/src/bucket.ts b/src/bucket.ts index 6e7d562dc..06743be8d 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -3725,7 +3725,6 @@ class Bucket extends ServiceObject { callback?: UploadCallback ): Promise | void { const upload = () => { - const isMultipart = options.resumable === false; const returnValue = retry( async (bail: (err: Error) => void) => { await new Promise((resolve, reject) => { @@ -3737,7 +3736,6 @@ class Bucket extends ServiceObject { .pipe(writable) .on('error', err => { if ( - isMultipart && this.storage.retryOptions.autoRetry && this.storage.retryOptions.retryableErrorFn!(err) ) { diff --git a/src/file.ts b/src/file.ts index 294f5497e..f430337bf 100644 --- a/src/file.ts +++ b/src/file.ts @@ -3618,14 +3618,12 @@ class File extends ServiceObject { typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; const options = typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; - const isMultipart = options.resumable === false; const returnValue = retry( async (bail: (err: Error) => void) => { await new Promise((resolve, reject) => { const writable = this.createWriteStream(options) .on('error', err => { if ( - isMultipart && this.storage.retryOptions.autoRetry && this.storage.retryOptions.retryableErrorFn!(err) ) { diff --git a/test/bucket.ts b/test/bucket.ts index 9314e8e48..77a697a29 100644 --- a/test/bucket.ts +++ b/test/bucket.ts @@ -2440,6 +2440,24 @@ describe('Bucket', () => { }); describe('resumable uploads', () => { + class DelayedStream500Error extends Transform { + retryCount: number; + constructor(retryCount: number) { + super(); + this.retryCount = retryCount; + } + _transform(chunk: string | Buffer, _encoding: string, done: Function) { + this.push(chunk); + setTimeout(() => { + if (this.retryCount === 1) { + done(new HTTPError('first error', 500)); + } else { + done(); + } + }, 5); + } + } + beforeEach(() => { fsStatOverride = (path: string, callback: Function) => { callback(null, {size: 1}); // Small size to guarantee simple upload @@ -2494,6 +2512,62 @@ describe('Bucket', () => { }; bucket.upload(filepath, options, assert.ifError); }); + + it('should not retry a nonretryable error code', done => { + const fakeFile = new FakeFile(bucket, 'file-name'); + const options = {destination: fakeFile, resumable: true}; + let retryCount = 0; + fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => { + class DelayedStream403Error extends Transform { + _transform( + chunk: string | Buffer, + _encoding: string, + done: Function + ) { + this.push(chunk); + setTimeout(() => { + retryCount++; + if (retryCount === 1) { + done(new HTTPError('first error', 403)); + } else { + done(); + } + }, 5); + } + } + setImmediate(() => { + assert.strictEqual(options_.resumable, true); + retryCount++; + done(); + }); + return new DelayedStream403Error(); + }; + + bucket.upload(filepath, options, (err: Error) => { + assert.strictEqual(err.message, 'first error'); + assert.ok(retryCount === 2); + done(); + }); + }); + + it('resumable upload should retry', done => { + const fakeFile = new FakeFile(bucket, 'file-name'); + const options = {destination: fakeFile, resumable: true}; + let retryCount = 0; + fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => { + setImmediate(() => { + assert.strictEqual(options_.resumable, true); + retryCount++; + done(); + }); + return new DelayedStream500Error(retryCount); + }; + bucket.upload(filepath, options, (err: Error) => { + assert.strictEqual(err.message, 'first error'); + assert.ok(retryCount === 1); + done(); + }); + }); }); describe('multipart uploads', () => { diff --git a/test/file.ts b/test/file.ts index 5f44f6b60..3903629ae 100644 --- a/test/file.ts +++ b/test/file.ts @@ -240,7 +240,7 @@ describe('File', () => { totalTimeout: 600, maxRetryDelay: 60, retryableErrorFn: (err: HTTPError) => { - return err.code === 500; + return err?.code === 500; }, }, }; @@ -4107,19 +4107,16 @@ describe('File', () => { assert.ok(retryCount === 2); }); - it('non-multipart upload should not retry', async () => { + it('resumable upload should retry', async () => { const options = {resumable: true}; let retryCount = 0; file.createWriteStream = () => { retryCount++; return new DelayedStream500Error(retryCount); }; - try { - await file.save(DATA, options); - throw Error('unreachable'); - } catch (e) { - assert.strictEqual(e.message, 'first error'); - } + + await file.save(BUFFER_DATA, options); + assert.ok(retryCount === 2); }); });