diff --git a/.travis.yml b/.travis.yml index d02d844..b1b2ffb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,3 +2,6 @@ language: node_js node_js: - "0.12" + +services: + - redis-server diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f477310..53cfc4c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,7 +11,10 @@ the contribution workflow at this point. create a token 4. From the terminal, add your newly created GitHub personal access token to the Heroku config: heroku config:set OAUTH_TOKEN=####### -5. Create a test repository which you'll send invalid CSS or Stylus files to, +5. Enable the Heroku Redis add-on (you may need to enter a credit card for + verification): `heroku addons:create heroku-redis:hobby-dev` +6. Start a worker: `heroku ps:scale worker=1` +7. Create a test repository which you'll send invalid CSS or Stylus files to, which your Discord instance will evaluate. Within that repository: 1. Navigate to *Settings* > *Webhooks and Services* 2. Add a webhook @@ -27,9 +30,24 @@ GitHub. # Submitting a pull request -Before submitting a pull request, run the following commands to test your -changes and spruce things up. +Before submitting a pull request, follow these steps to spruce things up and +test your changes. -1. `npm install` -2. `./gulp test` -3. `./gulp beautify` +## Setup + +To get started, install all dependencies and set up Redis: + +1. Run `npm install` +2. Follow [these steps](http://redis.io/topics/quickstart#installing-redis) + to install a local Redis server + +## Beautify your code + +Run this command to format your code according to our conventions: + +`./gulp beautify` + +## Run tests + +1. Start the local Redis server +2. Run `./gulp test` diff --git a/Procfile b/Procfile new file mode 100644 index 0000000..4903810 --- /dev/null +++ b/Procfile @@ -0,0 +1,2 @@ +web: node bin/www.js +worker: node worker.js diff --git a/commenter.js b/commenter.js index 6666624..e766fb4 100644 --- a/commenter.js +++ b/commenter.js @@ -8,15 +8,13 @@ var sendRequest = require('request'); var config = require('./config'); -var logger = require('./logger'); /** * Post a line comment on a particular pull request. commentURL should be an * instance of review_comments_url. * https://developer.github.com/v3/activity/events/types/#pullrequestevent */ -function postPullRequestComment(commentURL, comment, filename, commitSHA, line) { - +function postPullRequestComment(commentURL, comment, filename, commitSHA, line, done) { sendRequest({ url: commentURL, method: 'POST', @@ -30,8 +28,14 @@ function postPullRequestComment(commentURL, comment, filename, commitSHA, line) commit_id: commitSHA, position: line }) - }, function(error) { - if (error) return logger.error('Error posting pull request comment to ' + commentURL + ' regarding commit ' + commitSHA + ':', error); + }, function(error, response) { + // If the comment could not be submitted, notify Redis so that the job + // can be re-attempted later. Otherwise, mark the job as done. + if (error || response.statusCode === 403) { + return done(new Error('Comment could not be submitted')); + } else { + done(); + } }); } diff --git a/config.js b/config.js index caee4bd..f245886 100644 --- a/config.js +++ b/config.js @@ -2,6 +2,8 @@ var config = { brand: 'Discord', token: process.env.OAUTH_TOKEN, trackingID: process.env.TRACKING_ID, + commentWait: process.env.COMMENT_WAIT || 250, + commentAttempts: process.env.COMMENT_ATTEMPTS || 10, // Used when no environment variable (process.env.PORT) is set port: 3000, diff --git a/hook.js b/hook.js index 151c19f..f6a99c7 100644 --- a/hook.js +++ b/hook.js @@ -3,12 +3,12 @@ var github = require('octonode'); var Q = require('q'); -var commenter = require('./commenter'); var diff = require('./diffParse'); var logger = require('./logger'); var processor = require('./processor'); var utils = require('./utils'); var config = require('./config'); +var redisQueue = require('./redisQueue'); var configFilename = '.doiuse'; var githubClient = github.client(config.token); @@ -61,7 +61,32 @@ function processPullRequest(destinationRepo, originRepo, originBranch, prNumber, // Callback for handling an incompatible line of code var line = diff.lineToIndex(file.patch, incompatibility.usage.source.start.line); var comment = incompatibility.featureData.title + ' not supported by: ' + incompatibility.featureData.missing; - commenter.postPullRequestComment(commentURL, comment, file.filename, currentCommit.sha, line); + + // Create a Redis job that will submit the comment + var commentJob = redisQueue.create('comment', { + commentURL: commentURL, + sha: currentCommit.sha, + filename: file.filename, + line: line, + comment: comment + }); + + // If the comment is rejected, re-attempt several times with + // exponentially longer waits between each attempt. + commentJob.attempts(config.commentAttempts); + commentJob.backoff({ + type: 'exponential' + }); + + // If the comment is rejected after several attempts, log an + // error. + commentJob.on('failed', function() { + logger.error('Error posting comment to line ' + line + ' of ' + file.filename + ' in ' + originRepo + ' pull request #' + prNumber); + }); + + commentJob.save(function(error) { + if (error) return logger.error(error); + }); }); }); diff --git a/package.json b/package.json index 1e133d4..2b5695e 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,6 @@ "main": "app.js", "private": true, "scripts": { - "start": "node ./bin/www", "postinstall": "./gulp --gulpfile gulpfile-prod.js build", "test": "./gulp test" }, @@ -34,6 +33,7 @@ "express": "^4.8.6", "gulp": "^3.8.11", "gulp-marked": "^1.0.0", + "kue": "^0.9.4", "newrelic": "^1.21.1", "nunjucks": "^1.3.4", "octonode": "^0.7.1", diff --git a/redisQueue.js b/redisQueue.js new file mode 100644 index 0000000..77c1241 --- /dev/null +++ b/redisQueue.js @@ -0,0 +1,32 @@ +/** + * Return an instance of the Kue queue singleton. + * + * This file is only needed to work around bug #681 of Kue: + * https://github.com/Automattic/kue/issues/681 + * + * Once the bugfix is shipped, we'll be able to get the queue with much less + * code, making this file unnecessary: + * + * var kue = require('kue'); + * var redisQueue = kue.createQueue({ + * redis: process.env.REDIS_URL + * }); + */ + +var kue = require('kue'); +var url = require('url'); + +var parsedRedisURL = url.parse(process.env.REDIS_URL); + +var config = { + redis: { + port: parseInt(parsedRedisURL.port), + host: parsedRedisURL.hostname + } +}; + +if (parsedRedisURL.auth) { + config.redis.auth = parsedRedisURL.auth.replace(/.*?:/, ''); +} + +module.exports = kue.createQueue(config); diff --git a/tests/tests.js b/tests/tests.js index 67171d8..3875348 100644 --- a/tests/tests.js +++ b/tests/tests.js @@ -1,15 +1,20 @@ 'use strict'; -// Set environment variables that will be read during testing. These values will -// be read in config.js, so they need to be set before the file is loaded. +// Set environment variables that will be needed during testing. These values +// will be read in config.js, so they need to be set before the file is loaded. var testedTrackingID = '654321'; process.env.TRACKING_ID = testedTrackingID; +process.env.REDIS_URL = 'redis://localhost'; +process.env.COMMENT_WAIT = 0; // The comment wait is only needed in production to avoid tripping a GitHub spam protection system // Pretend that we're running in production mode so that we can test // production-only features like Google Analytics tracking. This value is read // in app.js, so it needs to be set before the file is loaded. process.env.NODE_ENV = 'production'; +// Process Redis tasks +require('../worker'); + var fs = require('fs'); var path = require('path'); diff --git a/worker.js b/worker.js new file mode 100644 index 0000000..1b1c166 --- /dev/null +++ b/worker.js @@ -0,0 +1,25 @@ +'use strict'; + +var commenter = require('./commenter'); +var config = require('./config'); +var redisQueue = require('./redisQueue'); + +var lastCommentTimestamp = 0; + +// Process comment jobs +// Pause for commentWait milleseconds between comment submissions +redisQueue.process('comment', function(job, done) { + var now = Date.now(); + var waitRemaining = 0; + + var nextCommentTimestamp = lastCommentTimestamp + parseInt(config.commentWait); + + if (nextCommentTimestamp > now) { + waitRemaining = nextCommentTimestamp - now; + } + + setTimeout(function() { + commenter.postPullRequestComment(job.data.commentURL, job.data.comment, job.data.filename, job.data.sha, job.data.line, done); + lastCommentTimestamp = Date.now(); // Use a new, fresh timestamp + }, waitRemaining); +});