diff --git a/lib/common/util.js b/lib/common/util.js index f95c9ce990b..e5ed5cf3e53 100644 --- a/lib/common/util.js +++ b/lib/common/util.js @@ -165,12 +165,14 @@ function ApiError(errorBody) { Error.captureStackTrace(this, arguments.callee); this.errors = errorBody.errors; this.code = errorBody.code; - this.message = errorBody.message; + this.message = errorBody.message || 'Error during request.'; this.response = errorBody.response; } nodeutil.inherits(ApiError, Error); +util.ApiError = ApiError; + /** * Uniformly process an API response. * @@ -211,10 +213,10 @@ function parseApiResp(err, resp, body) { if (resp.statusCode < 200 || resp.statusCode > 299) { // Unknown error. Format according to ApiError standard. - parsedResp.err = new ApiError({ + parsedResp.err = new util.ApiError({ errors: [], code: resp.statusCode, - message: 'Error during request.', + message: resp.statusMessage, response: resp }); } @@ -227,7 +229,7 @@ function parseApiResp(err, resp, body) { if (parsedResp.body && parsedResp.body.error) { // Error from JSON API. - parsedResp.err = new ApiError(parsedResp.body.error); + parsedResp.err = new util.ApiError(parsedResp.body.error); } return parsedResp; @@ -716,9 +718,12 @@ function makeRequest(reqOpts, config, callback) { }; if (config.stream) { + var requestStream = retryRequest(reqOpts, options); + config.stream.abort = requestStream.abort; + // `streamForward` is used to re-emit the events the request stream receives // on to the stream the user is holding (config.stream). - streamForward(retryRequest(reqOpts, options)).pipe(config.stream); + streamForward(requestStream).pipe(config.stream); } else { retryRequest(reqOpts, options, function(err, response, body) { util.handleResp(err, response, body, callback); diff --git a/lib/storage/file.js b/lib/storage/file.js index 8497d061ecc..5e6b940b236 100644 --- a/lib/storage/file.js +++ b/lib/storage/file.js @@ -538,51 +538,58 @@ File.prototype.createReadStream = function(options) { .on('response', throughStream.emit.bind(throughStream, 'response')) .on('complete', function(res) { - if (rangeRequest) { - // Range requests can't receive data integrity checks. - endThroughStream(null, res); - return; - } + util.handleResp(null, res, null, function(err) { + if (err) { + endThroughStream(err); + return; + } - var failed = false; - var crcFail = true; - var md5Fail = true; + if (rangeRequest) { + // Range requests can't receive data integrity checks. + endThroughStream(null, res); + return; + } - var hashes = {}; - res.headers['x-goog-hash'].split(',').forEach(function(hash) { - var hashType = hash.split('=')[0].trim(); - hashes[hashType] = hash.substr(hash.indexOf('=') + 1); - }); + var failed = false; + var crcFail = true; + var md5Fail = true; - var remoteMd5 = hashes.md5; - var remoteCrc = hashes.crc32c && hashes.crc32c.substr(4); + var hashes = {}; + res.headers['x-goog-hash'].split(',').forEach(function(hash) { + var hashType = hash.split('=')[0].trim(); + hashes[hashType] = hash.substr(hash.indexOf('=') + 1); + }); - if (crc32c) { - crcFail = localCrcHash !== remoteCrc; - failed = crcFail; - } + var remoteMd5 = hashes.md5; + var remoteCrc = hashes.crc32c && hashes.crc32c.substr(4); - if (md5) { - md5Fail = localMd5Hash !== remoteMd5; - failed = md5Fail; - } + if (crc32c) { + crcFail = localCrcHash !== remoteCrc; + failed = crcFail; + } - if (validation === 'all') { - failed = remoteMd5 ? md5Fail : crcFail; - } + if (md5) { + md5Fail = localMd5Hash !== remoteMd5; + failed = md5Fail; + } - if (failed) { - var mismatchError = new Error([ - 'The downloaded data did not match the data from the server.', - 'To be sure the content is the same, you should download the', - 'file again.' - ].join(' ')); - mismatchError.code = 'CONTENT_DOWNLOAD_MISMATCH'; + if (validation === 'all') { + failed = remoteMd5 ? md5Fail : crcFail; + } - endThroughStream(mismatchError, res); - } else { - endThroughStream(null, res); - } + if (failed) { + var mismatchError = new Error([ + 'The downloaded data did not match the data from the server.', + 'To be sure the content is the same, you should download the', + 'file again.' + ].join(' ')); + mismatchError.code = 'CONTENT_DOWNLOAD_MISMATCH'; + + endThroughStream(mismatchError, res); + } else { + endThroughStream(null, res); + } + }); }) .pipe(throughStream) diff --git a/system-test/storage.js b/system-test/storage.js index 66a5f741012..735977f5a60 100644 --- a/system-test/storage.js +++ b/system-test/storage.js @@ -526,6 +526,15 @@ describe('storage', function() { }); }); + it('should handle non-network errors', function(done) { + var file = bucket.file('hi.jpg'); + file.download(function(err) { + assert.strictEqual(err.code, 404); + assert.strictEqual(err.message, 'Not Found'); + done(); + }); + }); + it('should upload a gzipped file and download it', function(done) { var options = { metadata: { diff --git a/test/common/util.js b/test/common/util.js index 0953f1d3adb..38376f5bcce 100644 --- a/test/common/util.js +++ b/test/common/util.js @@ -132,6 +132,36 @@ describe('common/util', function() { }); }); + describe('ApiError', function() { + it('should build correct ApiError', function() { + var error = { + errors: [ new Error(), new Error() ], + code: 100, + message: 'Uh oh', + response: { a: 'b', c: 'd' } + }; + + var apiError = new util.ApiError(error); + + assert.strictEqual(apiError.errors, error.errors); + assert.strictEqual(apiError.code, error.code); + assert.strictEqual(apiError.message, error.message); + assert.strictEqual(apiError.response, error.response); + }); + + it('should build ApiError with default status message', function() { + var error = { + errors: [], + code: 100, + response: { a: 'b', c: 'd' } + }; + + var apiError = new util.ApiError(error); + + assert.strictEqual(apiError.message, 'Error during request.'); + }); + }); + describe('extendGlobalConfig', function() { it('should favor `keyFilename` when `credentials` is global', function() { var globalConfig = { credentials: {} }; @@ -208,11 +238,20 @@ describe('common/util', function() { }); describe('parseApiResp', function() { - it('should return err code if there are not other errors', function() { - var parsedApiResp = util.parseApiResp(null, { statusCode: 400 }); + describe('non-200s response status', function() { + it('should build ApiError with status and message', function(done) { + var error = { statusCode: 400, statusMessage: 'Not Good' }; + + utilOverrides.ApiError = function(error_) { + assert.strictEqual(error_.code, error.statusCode); + assert.strictEqual(error_.message, error.statusMessage); + assert.strictEqual(error_.response, error); - assert.strictEqual(parsedApiResp.err.code, 400); - assert.strictEqual(parsedApiResp.err.message, 'Error during request.'); + done(); + }; + + util.parseApiResp(null, error); + }); }); it('should detect body errors', function() { @@ -1004,6 +1043,19 @@ describe('common/util', function() { util.makeRequest(reqOpts, {}); }); + it('should expose the abort method from retryRequest', function(done) { + var userStream = new stream.Stream(); + + retryRequestOverride = function() { + var requestStream = new stream.Stream(); + requestStream.abort = done; + return requestStream; + }; + + util.makeRequest(reqOpts, { stream: userStream }); + userStream.abort(); + }); + it('should allow turning off retries to retryRequest', function(done) { retryRequestOverride = testNoRetryRequestConfig(done); util.makeRequest(reqOpts, noRetryRequestConfig); diff --git a/test/storage/file.js b/test/storage/file.js index 96f083d5be7..ac14eb5bdd7 100644 --- a/test/storage/file.js +++ b/test/storage/file.js @@ -585,6 +585,27 @@ describe('File', function() { }); }); + it('should let util.handleResp handle the response', function(done) { + var response = { a: 'b', c: 'd' }; + + handleRespOverride = function(err, response_, body) { + assert.strictEqual(err, null); + assert.strictEqual(response_, response); + assert.strictEqual(body, null); + done(); + }; + + file.bucket.storage.makeAuthorizedRequest_ = function() { + var stream = through(); + setImmediate(function() { + stream.emit('complete', response); + }); + return stream; + }; + + file.createReadStream(); + }); + describe('errors', function() { var ERROR = new Error('Error.');