🐛 fix image size timeout (#8283)

closes #8041

- destroy socket is required, see https://nodejs.org/api/http.html#http_server_settimeout_msecs_callback
- optimise error messages in general
- make timeouts configureable
This commit is contained in:
Katharina Irrgang 2017-04-05 22:58:26 +02:00 committed by Hannah Wolfe
parent 1bcd25fdbf
commit 64d2a94073
4 changed files with 20 additions and 9 deletions

View File

@ -57,7 +57,8 @@
},
"times": {
"cannotScheduleAPostBeforeInMinutes": 2,
"publishAPostBySchedulerToleranceInMinutes": 2
"publishAPostBySchedulerToleranceInMinutes": 2,
"getImageSizeTimeoutInMS": 5000
},
"maintenance": {
"enabled": false

View File

@ -19,7 +19,7 @@ function getCachedImageSizeFromUrl(url) {
// image size is not in cache
if (!imageSizeCache[url]) {
return getImageSizeFromUrl(url, 6000).then(function (res) {
return getImageSizeFromUrl(url).then(function (res) {
imageSizeCache[url] = res;
return Promise.resolve(imageSizeCache[url]);

View File

@ -19,6 +19,7 @@ var sizeOf = require('image-size'),
Promise = require('bluebird'),
http = require('http'),
https = require('https'),
config = require('../config'),
utils = require('../utils'),
errors = require('../errors'),
dimensions,
@ -28,16 +29,13 @@ var sizeOf = require('image-size'),
/**
* @description read image dimensions from URL
* @param {String} imagePath
* @param {Number} timeout (optional)
* @returns {Promise<Object>} imageObject or error
*/
module.exports.getImageSizeFromUrl = function getImageSizeFromUrl(imagePath, timeout) {
module.exports.getImageSizeFromUrl = function getImageSizeFromUrl(imagePath) {
return new Promise(function imageSizeRequest(resolve, reject) {
var imageObject = {},
options;
// set default timeout if called without option. Otherwise node will use default timeout of 120 sec.
timeout = timeout ? timeout : 10000;
options,
timeout = config.get('times:getImageSizeTimeoutInMS') || 10000;
imageObject.url = imagePath;
@ -83,6 +81,7 @@ module.exports.getImageSizeFromUrl = function getImageSizeFromUrl(imagePath, tim
}
} else {
return reject(new errors.InternalServerError({
message: res.statusCode === 404 ? 'Image not found.' : 'Unknown Request error.',
code: 'IMAGE_SIZE',
statusCode: res.statusCode,
context: imagePath
@ -92,8 +91,16 @@ module.exports.getImageSizeFromUrl = function getImageSizeFromUrl(imagePath, tim
}).on('socket', function (socket) {
if (timeout) {
socket.setTimeout(timeout);
/**
* https://nodejs.org/api/http.html
* "...if a callback is assigned to the Server's 'timeout' event, timeouts must be handled explicitly"
*
* socket.destroy will jump to the error listener
*/
socket.on('timeout', function () {
request.abort();
socket.destroy(new Error('Request timed out.'));
});
}
}).on('error', function (err) {

View File

@ -3,6 +3,7 @@ var should = require('should'),
Promise = require('bluebird'),
rewire = require('rewire'),
nock = require('nock'),
configUtils = require('../../utils/configUtils'),
utils = require('../../../server/utils'),
// Stuff we are testing
@ -21,6 +22,7 @@ describe('Image Size', function () {
afterEach(function () {
sandbox.restore();
configUtils.restore();
});
it('should have an image size function', function () {
@ -173,7 +175,8 @@ describe('Image Size', function () {
.socketDelay(11)
.reply(408);
result = Promise.resolve(imageSize.getImageSizeFromUrl(url, 10))
configUtils.set('times:getImageSizeTimeoutInMS', 10);
result = Promise.resolve(imageSize.getImageSizeFromUrl(url))
.catch(function (err) {
requestMock.isDone().should.be.true();
should.exist(err);