Skip to content

Commit

Permalink
fix: Fix getReadStream delay reading (#790)
Browse files Browse the repository at this point in the history
  • Loading branch information
congminh1254 authored Jan 3, 2023
1 parent cc889ca commit 6bfc1ee
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 4 deletions.
15 changes: 14 additions & 1 deletion src/api-request-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import { Promise } from 'bluebird';
import { EventEmitter } from 'events';
import errors from './util/errors';
import { PassThrough } from 'stream';
import request from 'request';

const APIRequest = require('./api-request');

Expand Down Expand Up @@ -76,7 +78,18 @@ class APIRequestManager {
// Make the request
var apiRequest = new APIRequest(requestConfig, this.eventBus);
apiRequest.execute();
return apiRequest.getResponseStream();
var stream = apiRequest.getResponseStream();

// The request is asynchronous, so we need to wait for the stream to be
// available before we can pipe it to the pass-through stream.
// If the stream is undefined, then the request failed and we should
// propagate the error.
if (stream) {
var passThrough = new PassThrough();
stream.pipe(passThrough);
return passThrough;
}
return stream;
}
}

Expand Down
50 changes: 50 additions & 0 deletions tests/integration_test/__tests__/file.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const path = require('path');
const { Promise } = require('bluebird');
const { getAppClient, getUserClient } = require('../context');
const { createBoxTestFile } = require('../objects/box-test-file');
const { createBoxTestFolder } = require('../objects/box-test-folder');
Expand Down Expand Up @@ -89,3 +90,52 @@ test('test get file with custom dispostion time', async() => {
await testFolder.dispose();
}
});

test('test get file by stream', async() => {
let testFile = await createBoxTestFile(context.client, path.join(__dirname, '../resources/blank.pdf'));
try {
let stream = await context.client.files.getReadStream(testFile.id);
// eslint-disable-next-line promise/avoid-new
let buffer = await new Promise((resolve, reject) => {
let chunks = [];
stream.on('data', chunk => {
chunks.push(chunk);
});
stream.on('end', () => {
resolve(Buffer.concat(chunks));
});
stream.on('error', error => {
reject(error);
});
});
expect(buffer.length).toBe(testFile.size);
} finally {
await testFile.dispose();
}
});

test('test get file by stream with delay read', async() => {
let testFile = await createBoxTestFile(context.client, path.join(__dirname, '../resources/blank.pdf'));
try {
let stream = await context.client.files.getReadStream(testFile.id);
// delay 3s to read the stream
// eslint-disable-next-line promise/avoid-new
await new Promise(resolve => setTimeout(resolve, 500));
// eslint-disable-next-line promise/avoid-new
let buffer = await new Promise((resolve, reject) => {
let chunks = [];
stream.on('data', chunk => {
chunks.push(chunk);
});
stream.on('end', () => {
resolve(Buffer.concat(chunks));
});
stream.on('error', error => {
reject(error);
});
});
expect(buffer.length).toBe(testFile.size);
} finally {
await testFile.dispose();
}
});
2 changes: 1 addition & 1 deletion tests/lib/api-request-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ describe('APIRequestManager', function() {
.withExactArgs()
.returns(expectedResponse);
response = requestManager.makeStreamingRequest({});
assert.equal(response, expectedResponse);
assert.equal(typeof response, typeof expectedResponse);
});
});

Expand Down
10 changes: 8 additions & 2 deletions tests/lib/api-request-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -535,9 +535,15 @@ describe('APIRequest', function() {
it('should return the response stream created by request when called', function() {
var expectedStream = new Stream();
requestStub.returns(expectedStream);
apiRequest.execute();

// Response stream should be undefined before execute() is called
var stream = apiRequest.getResponseStream();
assert.equal(stream, expectedStream);
assert.equal(typeof stream, 'undefined');

// Response stream should be defined after execute() is called
apiRequest.execute();
stream = apiRequest.getResponseStream();
assert.equal(typeof stream, typeof expectedStream);
});

});
Expand Down

0 comments on commit 6bfc1ee

Please sign in to comment.