diff --git a/test/api/unit/libs/webhooks.test.js b/test/api/unit/libs/webhooks.test.js index b3cf687c21..c8f745499f 100644 --- a/test/api/unit/libs/webhooks.test.js +++ b/test/api/unit/libs/webhooks.test.js @@ -1,4 +1,5 @@ import got from 'got'; +import moment from 'moment'; import { WebhookSender, taskScoredWebhook, @@ -13,6 +14,7 @@ import { import { generateUser, defer, + sleep, } from '../../../helpers/api-unit.helper'; @@ -322,6 +324,100 @@ describe('webhooks', () => { json: true, }); }); + + describe('failures', () => { + let sendWebhook; + + beforeEach(async () => { + sandbox.restore(); + sandbox.stub(got, 'post').returns(Promise.reject()); + + sendWebhook = new WebhookSender({ type: 'taskActivity' }); + user.webhooks = [{ + url: 'http://custom-url.com', enabled: true, type: 'taskActivity', + }]; + await user.save(); + + expect(user.webhooks[0].failures).to.equal(0); + expect(user.webhooks[0].lastFailureAt).to.equal(undefined); + }); + + it('does not increase failures counter if request is successfull', async () => { + sandbox.restore(); + sandbox.stub(got, 'post').returns(Promise.resolve()); + + const body = {}; + sendWebhook.send(user, body); + + expect(got.post).to.be.calledOnce; + expect(got.post).to.be.calledWithMatch('http://custom-url.com', { + json: true, + body, + }); + + await sleep(0.1); + user = await User.findById(user._id).exec(); + + expect(user.webhooks[0].failures).to.equal(0); + expect(user.webhooks[0].lastFailureAt).to.equal(undefined); + }); + + it('records failures', async () => { + const body = {}; + sendWebhook.send(user, body); + + expect(got.post).to.be.calledOnce; + expect(got.post).to.be.calledWithMatch('http://custom-url.com', { + json: true, + body, + }); + + await sleep(0.1); + user = await User.findById(user._id).exec(); + + expect(user.webhooks[0].failures).to.equal(1); + expect(user.webhooks[0].lastFailureAt.toLocaleString()) + .to.equal((new Date()).toLocaleString()); + }); + + it('disables a webhook after 10 failures', async () => { + const times = 10; + for (let i = 0; i < times; i += 1) { + sendWebhook.send(user, {}); + await sleep(0.1); // eslint-disable-line no-await-in-loop + user = await User.findById(user._id).exec(); // eslint-disable-line no-await-in-loop + } + + expect(got.post).to.be.callCount(10); + expect(got.post).to.be.calledWithMatch('http://custom-url.com'); + + await sleep(0.1); + user = await User.findById(user._id).exec(); + + expect(user.webhooks[0].enabled).to.equal(false); + expect(user.webhooks[0].failures).to.equal(0); + }); + + it('resets failures after a month ', async () => { + const oneMonthAgo = moment().subtract(1, 'months').subtract(1, 'days').toDate(); + user.webhooks[0].lastFailureAt = oneMonthAgo; + user.webhooks[0].failures = 9; + + await user.save(); + + sendWebhook.send(user, []); + + expect(got.post).to.be.calledOnce; + expect(got.post).to.be.calledWithMatch('http://custom-url.com'); + + await sleep(0.1); + user = await User.findById(user._id).exec(); + + expect(user.webhooks[0].failures).to.equal(1); + expect(user.webhooks[0].lastFailureAt.toLocaleString()) + .to.equal((new Date()).toLocaleString()); + }); + }); }); describe('taskScoredWebhook', () => { diff --git a/test/api/v3/integration/webhook/POST-user_add_webhook.test.js b/test/api/v3/integration/webhook/POST-user_add_webhook.test.js index 1bf8399a8d..aed4c7172b 100644 --- a/test/api/v3/integration/webhook/POST-user_add_webhook.test.js +++ b/test/api/v3/integration/webhook/POST-user_add_webhook.test.js @@ -81,6 +81,16 @@ describe('POST /user/webhook', () => { expect(webhook.type).to.eql('taskActivity'); }); + it('ignores protected fields', async () => { + body.failures = 3; + body.lastFailureAt = new Date(); + + const webhook = await user.post('/user/webhook', body); + + expect(webhook.failures).to.eql(0); + expect(webhook.lastFailureAt).to.eql(undefined); + }); + it('successfully adds the webhook', async () => { expect(user.webhooks).to.eql([]); diff --git a/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js b/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js index 571e084742..7745872984 100644 --- a/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js +++ b/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js @@ -63,6 +63,21 @@ describe('PUT /user/webhook/:id', () => { expect(webhook.options).to.eql(options); }); + it('ignores protected fields', async () => { + const failures = 3; + const lastFailureAt = new Date(); + + await user.put(`/user/webhook/${webhookToUpdate.id}`, { + failures, lastFailureAt, + }); + + await user.sync(); + const webhook = user.webhooks.find(hook => webhookToUpdate.id === hook.id); + + expect(webhook.failures).to.eql(0); + expect(webhook.lastFailureAt).to.eql(undefined); + }); + it('updates a webhook with empty label', async () => { const url = 'http://a-new-url.com'; const type = 'groupChatReceived'; diff --git a/website/server/controllers/api-v3/webhook.js b/website/server/controllers/api-v3/webhook.js index 2007099f49..3336f0eaec 100644 --- a/website/server/controllers/api-v3/webhook.js +++ b/website/server/controllers/api-v3/webhook.js @@ -108,7 +108,7 @@ api.addWebhook = { url: '/user/webhook', async handler (req, res) { const { user } = res.locals; - const webhook = new Webhook(req.body); + const webhook = new Webhook(Webhook.sanitize(req.body)); const existingWebhook = user.webhooks.find(hook => hook.id === webhook.id); diff --git a/website/server/controllers/top-level/dataexport.js b/website/server/controllers/top-level/dataexport.js index 715b8c2268..a9c5cecbfd 100644 --- a/website/server/controllers/top-level/dataexport.js +++ b/website/server/controllers/top-level/dataexport.js @@ -248,7 +248,7 @@ api.exportUserAvatarPng = { let response; try { - response = await got.head(s3url); + response = await got.head(s3url); // TODO add timeout and retries } catch (gotError) { // If the file does not exist AWS S3 can return a 403 error if (gotError.code !== 'ENOTFOUND' && gotError.statusCode !== 404 && gotError.statusCode !== 403) { diff --git a/website/server/libs/email.js b/website/server/libs/email.js index 4620f980fe..c5192e82d1 100644 --- a/website/server/libs/email.js +++ b/website/server/libs/email.js @@ -131,6 +131,8 @@ export async function sendTxn (mailingInfoArray, emailType, variables, personalV if (IS_PROD && mailingInfoArray.length > 0) { return got.post(`${EMAIL_SERVER.url}/job`, { + retry: 5, // retry the http request to the email server 5 times + timeout: 60000, // wait up to 60s before timing out auth: `${EMAIL_SERVER.auth.user}:${EMAIL_SERVER.auth.password}`, json: true, body: { diff --git a/website/server/libs/webhook.js b/website/server/libs/webhook.js index 64eae26526..64691996f5 100644 --- a/website/server/libs/webhook.js +++ b/website/server/libs/webhook.js @@ -1,6 +1,7 @@ import got from 'got'; import { isURL } from 'validator'; import nconf from 'nconf'; +import moment from 'moment'; import logger from './logger'; import { // eslint-disable-line import/no-cycle model as User, @@ -8,11 +9,59 @@ import { // eslint-disable-line import/no-cycle const IS_PRODUCTION = nconf.get('IS_PROD'); -function sendWebhook (url, body) { +function sendWebhook (webhook, body, user) { + const { url, lastFailureAt } = webhook; + got.post(url, { body, json: true, - }).catch(err => logger.error(err)); + timeout: 30000, // wait up to 30s before timing out + retry: 3, // retry the request up to 3 times + }).catch(webhookErr => { + // Log the error + logger.error(webhookErr); + + let _failuresReset = false; + + // Reset failures if the last one happened more than 1 month ago + const oneMonthAgo = moment().subtract(1, 'months'); + if (!lastFailureAt || moment(lastFailureAt).isBefore(oneMonthAgo)) { + webhook.failures = 0; + _failuresReset = true; + } + + // Increase the number of failures + webhook.failures += 1; + webhook.lastFailureAt = new Date(); + + // Disable a webhook with too many failures + if (webhook.failures >= 10) { + webhook.enabled = false; + webhook.failures = 0; + webhook.lastFailureAt = undefined; + _failuresReset = true; + } + + const update = { + $set: { + 'webhooks.$.lastFailureAt': webhook.lastFailureAt, + 'webhooks.$.enabled': webhook.enabled, + }, + }; + + if (_failuresReset) { + update.$set['webhooks.$.failures'] = webhook.failures; + } else { + update.$inc = { + 'webhooks.$.failures': 1, + }; + } + + return User.update({ + _id: user._id, + 'webhooks.id': webhook.id, + }, update).exec(); + }).catch(err => logger.error(err)); // log errors that might have happened in the previous catch } function isValidWebhook (hook) { @@ -60,7 +109,7 @@ export class WebhookSender { this.attachDefaultData(user, body); hooks.forEach(hook => { - sendWebhook(hook.url, body); + sendWebhook(hook, body, user); }); } } diff --git a/website/server/models/webhook.js b/website/server/models/webhook.js index 21b79e6791..7fcd0a975b 100644 --- a/website/server/models/webhook.js +++ b/website/server/models/webhook.js @@ -61,6 +61,10 @@ export const schema = new Schema({ }), shared.i18n.t('invalidUrl')], }, enabled: { $type: Boolean, required: true, default: true }, + // How many times this webhook has failed, disabled after 10 + failures: { $type: Number, default: 0 }, + // When the last failure happened, if older than 1 month the number of failures is reset + lastFailureAt: { $type: Date }, options: { $type: Schema.Types.Mixed, required: true, @@ -76,7 +80,7 @@ export const schema = new Schema({ }); schema.plugin(baseModel, { - noSet: ['_id'], + noSet: ['_id', 'failures', 'lastFailureAt'], timestamps: true, _id: false, });