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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

fix: truncated createReadStream through early end event #2056

Merged
merged 3 commits into from Sep 1, 2022
Merged

fix: truncated createReadStream through early end event #2056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8620b8d
test: read entire file when range is `{start: 0}`
danielbankhead Sep 1, 2022
d50bb5e
fix: `ReturnType<F>` regression
danielbankhead Sep 1, 2022
577dac3
fix: Remove premature `.end()` calls
danielbankhead Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 5 additions & 10 deletions src/file.ts
View file
Open in desktop
Expand Up @@ -1417,9 +1417,7 @@ class File extends ServiceObject<File> {
// Authenticate the request, then pipe the remote API request to the stream
// returned to the user.
const makeRequest = () => {
const query = {
alt: 'media',
} as FileQuery;
const query: FileQuery = {alt: 'media'};

if (this.generation) {
query.generation = this.generation;
Expand Down Expand Up @@ -1460,8 +1458,7 @@ class File extends ServiceObject<File> {
})
.on('response', res => {
throughStream.emit('response', res);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
util.handleResp(null, res, null, onResponse as any);
util.handleResp(null, res, null, onResponse);
})
.resume();

Expand Down Expand Up @@ -1525,7 +1522,7 @@ class File extends ServiceObject<File> {
const handoffStream = new PassThrough({
final: async cb => {
// Preserving `onComplete`'s ability to
// close `throughStream` before pipeline
// destroy `throughStream` before pipeline
// attempts to.
await onComplete(null);
cb();
Expand Down Expand Up @@ -1558,7 +1555,6 @@ class File extends ServiceObject<File> {
}

if (rangeRequest || !shouldRunValidation) {
throughStream.end();
return;
}

Expand All @@ -1576,7 +1572,6 @@ class File extends ServiceObject<File> {
return;
}
if (this.metadata.contentEncoding === 'gzip') {
throughStream.end();
return;
}
}
Expand Down Expand Up @@ -1611,14 +1606,14 @@ class File extends ServiceObject<File> {

throughStream.destroy(mismatchError);
} else {
throughStream.end();
return;
}
};
};

throughStream.on('reading', makeRequest);

return throughStream as Readable;
return throughStream;
}

createResumableUpload(
Expand Down
10 changes: 9 additions & 1 deletion system-test/storage.ts
View file
Open in desktop
Expand Up @@ -1719,7 +1719,7 @@ describe('storage', () => {

// Validate the desired functionality
const results = await testFunction(USER_PROJECT_OPTIONS);
return results;
return results as ReturnType<F>;
}

it('bucket#combine', async () => {
Expand Down Expand Up @@ -2119,6 +2119,14 @@ describe('storage', () => {
assert.strictEqual(String(fileContents), String(remoteContents));
});

it('should download an entire file if range `start:0` is provided', async () => {
const fileContents = fs.readFileSync(FILES.big.path);
const [file] = await bucket.upload(FILES.big.path);
const [result] = await file.download({start: 0});

assert.strictEqual(result.toString(), fileContents.toString());
});

it('should download an empty file', async () => {
const fileContents = fs.readFileSync(FILES.empty.path);
const [file] = await bucket.upload(FILES.empty.path);
Expand Down