Skip to content

Commit 175e6f9

Browse files
stephenplusplusstephenplusplus
committed
use raw response payload as error message
1 parent a25a18c commit 175e6f9

3 files changed

Lines changed: 113 additions & 64 deletions

File tree

‎packages/storage/src/file.js‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,8 @@ File.prototype.createReadStream = function(options) {
449449
var crc32c = true;
450450
var md5 = false;
451451

452+
var refreshedMetadata = false;
453+
452454
if (is.string(options.validation)) {
453455
options.validation = options.validation.toLowerCase();
454456
crc32c = options.validation === 'crc32c';
@@ -470,13 +472,11 @@ File.prototype.createReadStream = function(options) {
470472
// returned to the user.
471473
function makeRequest() {
472474
var reqOpts = {
473-
uri: format('{downloadBaseUrl}/{bucketName}/{fileName}', {
474-
downloadBaseUrl: STORAGE_DOWNLOAD_BASE_URL,
475-
bucketName: self.bucket.name,
476-
fileName: encodeURIComponent(self.name)
477-
}),
475+
uri: '',
478476
gzip: true,
479-
qs: {}
477+
qs: {
478+
alt: 'media'
479+
}
480480
};
481481

482482
if (self.generation) {
@@ -509,6 +509,13 @@ File.prototype.createReadStream = function(options) {
509509
function onResponse(err, body, res) {
510510
if (err) {
511511
requestStream.unpipe(throughStream);
512+
513+
// Get error message from the body.
514+
res.pipe(concat(function(body) {
515+
err.message = body.toString();
516+
throughStream.destroy(err);
517+
}));
518+
512519
return;
513520
}
514521

@@ -525,21 +532,29 @@ File.prototype.createReadStream = function(options) {
525532
// This is hooked to the `complete` event from the request stream. This is
526533
// our chance to validate the data and let the user know if anything went
527534
// wrong.
528-
function onComplete(err, body, res) {
535+
function onComplete(err) {
529536
if (err) {
530-
throughStream.destroy(err);
531537
return;
532538
}
533539

534540
if (rangeRequest) {
535541
return;
536542
}
537543

538-
var hashes = {};
539-
res.headers['x-goog-hash'].split(',').forEach(function(hash) {
540-
var hashType = hash.split('=')[0].trim();
541-
hashes[hashType] = hash.substr(hash.indexOf('=') + 1);
542-
});
544+
if (!refreshedMetadata) {
545+
refreshedMetadata = true;
546+
547+
self.getMetadata(function() {
548+
onComplete(err);
549+
});
550+
551+
return;
552+
}
553+
554+
var hashes = {
555+
crc32c: self.metadata.crc32c,
556+
md5: self.metadata.md5Hash
557+
};
543558

544559
// If we're doing validation, assume the worst-- a data integrity
545560
// mismatch. If not, these tests won't be performed, and we can assume the

‎packages/storage/system-test/storage.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ describe('storage', function() {
904904
file.copy('new-file.txt', options, done);
905905
}));
906906

907-
it.only('file#createReadStream', doubleTest(function(options, done) {
907+
it('file#createReadStream', doubleTest(function(options, done) {
908908
file.createReadStream(options)
909909
.on('error', done)
910910
.on('end', done)
@@ -915,7 +915,7 @@ describe('storage', function() {
915915
file.createResumableUpload(options, done);
916916
}));
917917

918-
it.skip('file#download', doubleTest(function(options, done) {
918+
it('file#download', doubleTest(function(options, done) {
919919
file.download(options, done);
920920
}));
921921

@@ -1256,7 +1256,10 @@ describe('storage', function() {
12561256

12571257
it('should not download from the unencrypted file', function(done) {
12581258
unencryptedFile.download(function(err) {
1259-
assert.strictEqual(err.message, 'Bad Request');
1259+
assert.strictEqual(err.message, [
1260+
'The target object is encrypted by a',
1261+
'customer-supplied encryption key.'
1262+
].join(' '));
12601263
done();
12611264
});
12621265
});

‎packages/storage/test/file.js‎

Lines changed: 79 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,12 @@ describe('File', function() {
513513
return FakeFailedRequest;
514514
}
515515

516+
beforeEach(function() {
517+
requestOverride = function() {
518+
return through();
519+
};
520+
});
521+
516522
it('should throw if both a range and validation is given', function() {
517523
assert.throws(function() {
518524
file.createReadStream({
@@ -606,14 +612,14 @@ describe('File', function() {
606612

607613
describe('authenticating', function() {
608614
it('should create an authenticated request', function(done) {
609-
var expectedPath = format('https://{host}/{b}/{o}', {
610-
host: 'storage.googleapis.com',
611-
b: file.bucket.name,
612-
o: encodeURIComponent(file.name)
613-
});
614-
615615
file.requestStream = function(opts) {
616-
assert.equal(opts.uri, expectedPath);
616+
assert.deepEqual(opts, {
617+
uri: '',
618+
gzip: true,
619+
qs: {
620+
alt: 'media'
621+
}
622+
});
617623
setImmediate(function() {
618624
done();
619625
});
@@ -640,7 +646,7 @@ describe('File', function() {
640646

641647
beforeEach(function() {
642648
file.requestStream = function(opts) {
643-
var stream = (requestOverride || request)(opts);
649+
var stream = requestOverride(opts);
644650

645651
setImmediate(function() {
646652
stream.emit('error', ERROR);
@@ -711,6 +717,7 @@ describe('File', function() {
711717
});
712718

713719
it('should unpipe stream from an error on the response', function(done) {
720+
var rawResponseStream = through();
714721
var requestStream = through();
715722
var readStream = file.createReadStream();
716723

@@ -728,7 +735,7 @@ describe('File', function() {
728735
assert.strictEqual(requestStream._readableState.pipes, readStream);
729736

730737
// Triggers the unpipe.
731-
callback(new Error());
738+
callback(new Error(), null, rawResponseStream);
732739

733740
setImmediate(function() {
734741
assert.strictEqual(requestStream._readableState.pipesCount, 0);
@@ -776,40 +783,65 @@ describe('File', function() {
776783
})
777784
.resume();
778785
});
786+
787+
it('should parse a response stream for a better error', function(done) {
788+
var rawResponsePayload = 'error message from body';
789+
var rawResponseStream = through();
790+
var requestStream = through();
791+
792+
handleRespOverride = function(err, res, body, callback) {
793+
callback(ERROR, null, res);
794+
795+
setImmediate(function() {
796+
rawResponseStream.end(rawResponsePayload);
797+
});
798+
};
799+
800+
file.requestStream = function() {
801+
setImmediate(function() {
802+
requestStream.emit('response', rawResponseStream);
803+
});
804+
return requestStream;
805+
};
806+
807+
file.createReadStream()
808+
.once('error', function(err) {
809+
assert.strictEqual(err, ERROR);
810+
assert.strictEqual(err.message, rawResponsePayload);
811+
done();
812+
})
813+
.resume();
814+
});
779815
});
780816
});
781817

782818
describe('validation', function() {
783819
var data = 'test';
784820

785-
var fakeResponse = {
786-
crc32c: {
787-
headers: { 'x-goog-hash': 'crc32c=####wA==' }
788-
},
789-
md5: {
790-
headers: { 'x-goog-hash': 'md5=CY9rzUYh03PK3k6DJie09g==' }
791-
}
792-
};
793-
794821
beforeEach(function() {
795822
file.metadata.mediaLink = 'http://uri';
796823

797-
file.request = function(opts, cb) {
798-
if (cb) {
799-
(cb.onAuthenticated || cb)(null, {});
800-
} else {
801-
return (requestOverride || requestCached)(opts);
802-
}
824+
file.getMetadata = function(callback) {
825+
file.metadata = {
826+
crc32c: '####wA==',
827+
md5Hash: 'CY9rzUYh03PK3k6DJie09g=='
828+
};
829+
callback();
803830
};
804831
});
805832

806833
it('should destroy the stream on error', function(done) {
834+
var rawResponseStream = through();
807835
var error = new Error('Error.');
808836

809837
file.requestStream = getFakeSuccessfulRequest('data');
810838

811839
handleRespOverride = function(err, resp, body, callback) {
812-
callback(error);
840+
callback(error, null, rawResponseStream);
841+
842+
setImmediate(function() {
843+
rawResponseStream.end();
844+
});
813845
};
814846

815847
file.createReadStream({ validation: 'crc32c' })
@@ -826,8 +858,7 @@ describe('File', function() {
826858
});
827859

828860
it('should validate with crc32c', function(done) {
829-
file.requestStream =
830-
getFakeSuccessfulRequest(data, fakeResponse.crc32c);
861+
file.requestStream = getFakeSuccessfulRequest(data, {});
831862

832863
file.createReadStream({ validation: 'crc32c' })
833864
.on('error', done)
@@ -836,10 +867,7 @@ describe('File', function() {
836867
});
837868

838869
it('should emit an error if crc32c validation fails', function(done) {
839-
file.requestStream = getFakeSuccessfulRequest(
840-
'bad-data',
841-
fakeResponse.crc32c
842-
);
870+
file.requestStream = getFakeSuccessfulRequest('bad-data', {});
843871

844872
file.createReadStream({ validation: 'crc32c' })
845873
.on('error', function(err) {
@@ -850,7 +878,7 @@ describe('File', function() {
850878
});
851879

852880
it('should validate with md5', function(done) {
853-
file.requestStream = getFakeSuccessfulRequest(data, fakeResponse.md5);
881+
file.requestStream = getFakeSuccessfulRequest(data, {});
854882

855883
file.createReadStream({ validation: 'md5' })
856884
.on('error', done)
@@ -859,8 +887,7 @@ describe('File', function() {
859887
});
860888

861889
it('should emit an error if md5 validation fails', function(done) {
862-
file.requestStream =
863-
getFakeSuccessfulRequest('bad-data', fakeResponse.md5);
890+
file.requestStream = getFakeSuccessfulRequest('bad-data', {});
864891

865892
file.createReadStream({ validation: 'md5' })
866893
.on('error', function(err) {
@@ -871,9 +898,14 @@ describe('File', function() {
871898
});
872899

873900
it('should default to crc32c validation', function(done) {
874-
file.requestStream = getFakeSuccessfulRequest(data, {
875-
headers: { 'x-goog-hash': 'crc32c=fakefakefake' }
876-
});
901+
file.getMetadata = function(callback) {
902+
file.metadata = {
903+
crc32c: file.metadata.crc32c
904+
};
905+
callback();
906+
};
907+
908+
file.requestStream = getFakeSuccessfulRequest(data, {});
877909

878910
file.createReadStream()
879911
.on('error', function(err) {
@@ -884,9 +916,7 @@ describe('File', function() {
884916
});
885917

886918
it('should ignore a data mismatch if validation: false', function(done) {
887-
file.requestStream = getFakeSuccessfulRequest(data, {
888-
headers: { 'x-goog-hash': 'crc32c=fakefakefake' }
889-
});
919+
file.requestStream = getFakeSuccessfulRequest(data, {});
890920

891921
file.createReadStream({ validation: false })
892922
.resume()
@@ -896,10 +926,7 @@ describe('File', function() {
896926

897927
describe('destroying the through stream', function() {
898928
it('should destroy after failed validation', function(done) {
899-
file.requestStream = getFakeSuccessfulRequest(
900-
'bad-data',
901-
fakeResponse.md5
902-
);
929+
file.requestStream = getFakeSuccessfulRequest('bad-data', {});
903930

904931
var readStream = file.createReadStream({ validation: 'md5' });
905932
readStream.destroy = function(err) {
@@ -910,10 +937,14 @@ describe('File', function() {
910937
});
911938

912939
it('should destroy if MD5 is requested but absent', function(done) {
913-
file.requestStream = getFakeSuccessfulRequest(
914-
'bad-data',
915-
fakeResponse.crc32c
916-
);
940+
file.getMetadata = function(callback) {
941+
file.metadata = {
942+
crc32c: file.metadata.crc32c
943+
};
944+
callback();
945+
};
946+
947+
file.requestStream = getFakeSuccessfulRequest('bad-data', {});
917948

918949
var readStream = file.createReadStream({ validation: 'md5' });
919950
readStream.destroy = function(err) {

0 commit comments

Comments
 (0)