From ef5d78d93d86fd5be6d11e80f992e4f89b0b0efd Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 25 Jun 2026 09:20:15 +0000 Subject: [PATCH 1/4] fix(storage): destroy local read stream on upload write failure to prevent resource leaks --- handwritten/storage/src/bucket.ts | 11 ++++++-- handwritten/storage/test/bucket.ts | 44 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 5c796789ebd3..09907e82a92a 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4501,17 +4501,22 @@ class Bucket extends ServiceObject { ) { newFile.storage.retryOptions.autoRetry = false; } + const readStream = fs.createReadStream(pathString); const writable = newFile.createWriteStream(options); if (options.onUploadProgress) { writable.on('progress', options.onUploadProgress); } - fs.createReadStream(pathString) - .on('error', bail) + readStream + .on('error', err => { + readStream.destroy(); + bail(err); + }) .pipe(writable) .on('error', err => { + readStream.destroy(); if ( this.storage.retryOptions.autoRetry && - this.storage.retryOptions.retryableErrorFn!(err) + this.storage.retryOptions.retryableErrorFn!(err as any) ) { return reject(err); } else { diff --git a/handwritten/storage/test/bucket.ts b/handwritten/storage/test/bucket.ts index 555d8e8c1c9c..3333b38a96d3 100644 --- a/handwritten/storage/test/bucket.ts +++ b/handwritten/storage/test/bucket.ts @@ -100,11 +100,18 @@ class FakeNotification { } let fsStatOverride: Function | null; +let fsCreateReadStreamOverride: Function | null; const fakeFs = { ...fs, stat: (filePath: string, callback: Function) => { return (fsStatOverride || fs.stat)(filePath, callback); }, + createReadStream: (filePath: string, options?: any) => { + return (fsCreateReadStreamOverride || fs.createReadStream)( + filePath, + options + ); + }, }; let pLimitOverride: Function | null; @@ -3231,6 +3238,43 @@ describe('Bucket', () => { }); }); + it('should destroy the local read stream if write stream fails', done => { + const fakeFile = new FakeFile(bucket, 'file-name'); + const options = {destination: fakeFile, resumable: false}; + const originalCreateReadStream = fs.createReadStream; + let readStream: fs.ReadStream; + fsCreateReadStreamOverride = (path: string, opts: any) => { + readStream = originalCreateReadStream(path, opts); + return readStream; + }; + + fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => { + const ws = new stream.Writable({ + write(chunk, encoding, callback) { + callback(new Error('write error')); + }, + }); + return ws; + }; + + const textfilepath = path.join( + getDirName(), + '../../../test/testdata/textfile.txt' + ); + + bucket.upload(textfilepath, options, (err: Error) => { + try { + assert.strictEqual(err.message, 'write error'); + assert.ok(readStream.destroyed); + done(); + } catch (e) { + done(e); + } finally { + fsCreateReadStreamOverride = null; + } + }); + }); + it('should allow overriding content type', done => { const fakeFile = new FakeFile(bucket, 'file-name'); const metadata = {contentType: 'made-up-content-type'}; From 1529d3328285c269cef836ed84a50591b0122514 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 25 Jun 2026 09:58:00 +0000 Subject: [PATCH 2/4] fix(storage): destroy writable stream on read error and reset read stream override in tests --- handwritten/storage/src/bucket.ts | 1 + handwritten/storage/test/bucket.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 09907e82a92a..169a7797f8a3 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4509,6 +4509,7 @@ class Bucket extends ServiceObject { readStream .on('error', err => { readStream.destroy(); + writable.destroy(); bail(err); }) .pipe(writable) diff --git a/handwritten/storage/test/bucket.ts b/handwritten/storage/test/bucket.ts index 3333b38a96d3..1f80a317afa0 100644 --- a/handwritten/storage/test/bucket.ts +++ b/handwritten/storage/test/bucket.ts @@ -241,6 +241,7 @@ describe('Bucket', () => { beforeEach(() => { fsStatOverride = null; + fsCreateReadStreamOverride = null; pLimitOverride = null; bucket = new Bucket(STORAGE, BUCKET_NAME); }); @@ -3265,6 +3266,7 @@ describe('Bucket', () => { bucket.upload(textfilepath, options, (err: Error) => { try { assert.strictEqual(err.message, 'write error'); + assert.ok(readStream); assert.ok(readStream.destroyed); done(); } catch (e) { From d0e8fe37afe4333d5887a50d44306141d32befa7 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 25 Jun 2026 11:36:48 +0000 Subject: [PATCH 3/4] fix: reorder read stream initialization to prevent premature stream destruction in upload flow --- handwritten/storage/src/bucket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 169a7797f8a3..76decb6cd95d 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4501,11 +4501,11 @@ class Bucket extends ServiceObject { ) { newFile.storage.retryOptions.autoRetry = false; } - const readStream = fs.createReadStream(pathString); const writable = newFile.createWriteStream(options); if (options.onUploadProgress) { writable.on('progress', options.onUploadProgress); } + const readStream = fs.createReadStream(pathString); readStream .on('error', err => { readStream.destroy(); From 221122803259306d39bad61f658d28aa14c31c64 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 26 Jun 2026 06:46:12 +0000 Subject: [PATCH 4/4] fix: update error type to ApiError for retryable error check in bucket operations --- handwritten/storage/src/bucket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 76decb6cd95d..527e7396f87f 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4517,7 +4517,7 @@ class Bucket extends ServiceObject { readStream.destroy(); if ( this.storage.retryOptions.autoRetry && - this.storage.retryOptions.retryableErrorFn!(err as any) + this.storage.retryOptions.retryableErrorFn!(err as ApiError) ) { return reject(err); } else {