Continue middleware chain after download error
Stop sending constructing the error response directly inside the storage component and send the error back to the application to handle it. As a result: - `File.afterRemoteError` hook is trigered now - App-provided error handling strategy is used to build error responses, e.g. using the production mode of strong-error-handler to hide sensitive information.
This commit is contained in:
parent
9997041093
commit
6cb5a294c5
|
@ -204,16 +204,6 @@ exports.upload = function(provider, req, res, options, cb) {
|
|||
});
|
||||
};
|
||||
|
||||
function handleError(res, err) {
|
||||
if (err.code === 'ENOENT') {
|
||||
res.type('application/json');
|
||||
res.status(404).send({error: err});
|
||||
return;
|
||||
}
|
||||
res.type('application/json');
|
||||
res.status(500).send({error: err});
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle download from a container/file.
|
||||
* @param {Object} provider The storage service provider
|
||||
|
@ -248,7 +238,7 @@ exports.download = function(provider, req, res, container, file, cb) {
|
|||
|
||||
provider.getFile(params.container, params.remote, function(err, stats) {
|
||||
if (err) {
|
||||
return handleError(res, err);
|
||||
return cb(processError(err, params.remote));
|
||||
}
|
||||
|
||||
setupPartialDownload(params, stats, res);
|
||||
|
@ -261,11 +251,15 @@ exports.download = function(provider, req, res, container, file, cb) {
|
|||
res.type(fileName);
|
||||
|
||||
reader.pipe(res);
|
||||
|
||||
reader.on('error', function onReaderError(err) {
|
||||
handleError(res, err);
|
||||
cb(processError(err, params.remote));
|
||||
cb = function() {}; // avoid double-callback
|
||||
});
|
||||
|
||||
reader.on('end', function onReaderEnd() {
|
||||
cb();
|
||||
cb = function() {}; // avoid double-callback
|
||||
});
|
||||
}
|
||||
};
|
||||
|
@ -287,3 +281,13 @@ function setupPartialDownload(params, stats, res) {
|
|||
res.set('Accept-Ranges', 'bytes');
|
||||
res.set('Content-Length', chunksize);
|
||||
};
|
||||
|
||||
function processError(err, fileName) {
|
||||
if (err.code === 'ENOENT') {
|
||||
err.statusCode = err.status = 404;
|
||||
// Hide the original message reported e.g. by FS provider, as it may
|
||||
// contain sensitive information.
|
||||
err.message = 'File not found: ' + fileName;
|
||||
}
|
||||
return err;
|
||||
}
|
||||
|
|
|
@ -314,6 +314,24 @@ describe('storage service', function() {
|
|||
});
|
||||
});
|
||||
|
||||
it('should run a function after a download failed', function(done) {
|
||||
var hookCalled = false;
|
||||
var Container = app.models.Container;
|
||||
|
||||
Container.afterRemoteError('download', function(ctx, cb) {
|
||||
hookCalled = true;
|
||||
cb();
|
||||
});
|
||||
|
||||
request('http://localhost:' + app.get('port'))
|
||||
.get('/containers/album1/download/does-not-exist')
|
||||
.expect(404, function(err, res) {
|
||||
if (err) return done(err);
|
||||
assert(hookCalled, 'afterRemoteEror hook was not called');
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('should delete a file', function(done) {
|
||||
request('http://localhost:' + app.get('port'))
|
||||
.del('/containers/album1/files/test.jpg')
|
||||
|
|
Loading…
Reference in New Issue