Reduce work on aborted requests

Closes #129
Includes testcase

NEWS.md

@@ -16,6 +16,7 @@ Enhancements:
  * Upgrade node-zipfile to ~0.5.0
  * Add support for node-0.10 (#132)
  * Fix lack of response on backend crash (#135)
+ * Reduce work on aborted requests (#129)
 
 1.8.3 - 2014-02-10
 ------------------

app/controllers/app.js

@@ -160,6 +160,22 @@ function handleQuery(req, res) {
     var tableCacheItem;
     var requestProtocol = req.protocol;
 
+    req.aborted = false;
+    req.on("close", function() {
+      console.log("Request closed unexpectedly (aborted?)");
+      req.aborted = true; // TODO: there must be a builtin way to check this
+    });
+
+    function checkAborted(step) {
+      if ( req.aborted ) {
+        var err = new Error("Request aborted during " + step);
+        // We'll use status 499, same as ngnix in these cases
+        // see http://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_Client_Error
+        err.http_status = 499;
+        throw err;
+      }
+    }
+
     try {
 
         // sanitize and apply defaults to input
@@ -215,6 +231,7 @@ function handleQuery(req, res) {
         // 5. Send formatted results back
         Step(
             function getDatabaseName() {
+                checkAborted('getDatabaseName');
                 if (_.isNull(database)) {
                     Meta.getUserDBName(cdbuser, this);
                 } else {
@@ -268,6 +285,7 @@ function handleQuery(req, res) {
             },
             function queryExplain(err, data){
                 if (err) throw err;
+                checkAborted('queryExplain');
 
                 if ( authenticated ) {
                   if ( global.settings.hasOwnProperty('db_user_pass') ) {
@@ -292,6 +310,7 @@ function handleQuery(req, res) {
             },
             function setHeaders(err, result){
                 if (err) throw err;
+                checkAborted('setHeaders');
 
                 // store explain result in local Cache
                 if ( ! tableCacheItem ) {
@@ -367,6 +386,7 @@ function handleQuery(req, res) {
             },
             function generateFormat(err, result){
                 if (err) throw err;
+                checkAborted('generateFormat');
 
                 // TODO: drop this, fix UI!
                 sql = PSQL.window_sql(sql,limit,offset);
@@ -378,7 +398,8 @@ function handleQuery(req, res) {
                   dp: dp,
                   skipfields: skipfields,
                   sql: sql,
-                  filename: filename
+                  filename: filename,
+                  abortChecker: checkAborted
                 }
 
                 formatter.sendResponse(opts, this);

app/models/formats/pg.js

@@ -65,6 +65,9 @@ pg.prototype.handleQueryEnd = function(result) {
 
   Step (
     function packageResult() {
+      if ( that.opts.abortChecker ) {
+        that.opts.abortChecker('packageResult');
+      }
       that.transform(result, that.opts, this);
     },
     function sendResults(err, out){

test/acceptance/frontend_abort.js

@@ -0,0 +1,86 @@
+require('../helper');
+require('../support/assert');
+
+var assert = require('assert')
+    , App = require(global.settings.app_root + '/app/controllers/app')
+    , querystring = require('querystring')
+    , _ = require('underscore')
+    , Step = require('step')
+    , net = require('net')
+    , http = require('http')
+    ;
+
+var sql_server_data_handler;
+var sql_server_port = 5556;
+var sql_server = net.createServer(function(c) {
+  c.on('data', function(d) {
+    console.log("SQL Server got data: " + d);
+    if ( sql_server_data_handler ) {
+      console.log("Sending data to sql_server_data_handler");
+      sql_server_data_handler(null, d);
+    }
+    c.destroy();
+  });
+});
+
+suite('frontend abort', function() {
+
+suiteSetup(function(done){
+  sql_server.listen(sql_server_port, done);
+});
+
+// See https://github.com/CartoDB/CartoDB-SQL-API/issues/129
+test('aborts request', function(done){
+//console.log("settings:"); console.dir(global.settings);
+  var db_host_backup = global.settings.db_host;
+  var db_port_backup = global.settings.db_port;
+  global.settings.db_host = 'localhost';
+  global.settings.db_port = sql_server_port;
+  var app = App();
+  var timeout;
+  Step(
+    function sendQuery() {
+      var next = this;
+      assert.response(app, {
+          url: '/api/v1/sql?q=SELECT+1',
+          method: 'GET',
+          timeout: 1,
+          headers: {host: 'vizzuality.localhost' }
+      },{}, function(res, err) {
+          next(err, res);
+      });
+    },
+    function checkResponse(err, res) {
+      assert(err); // expect timeout
+      assert.ok((''+err).match(/socket/), err);
+      sql_server_data_handler = this;
+      var next = this;
+      // If a call does not arrive to the sql server within
+      // the given timeout we're confident it means the request
+      // was successfully aborted
+      timeout = setTimeout(function() { next(null); }, 500);
+    },
+    function checkSqlServerData(err, data) {
+      clearTimeout(timeout);
+      assert.ok(!data, "SQL Server was contacted no matter client abort");
+      // TODO: intercept logs ?
+      return null;
+    },
+    function finish(err) {
+      global.settings.db_host = db_host_backup;
+      global.settings.db_port = db_port_backup;
+      done(err);
+    }
+  );
+});
+
+suiteTeardown(function(done) {
+  try {
+    sql_server.close(done);
+  } catch (er) {
+    console.log(er);
+    done(); // error expected as server is probably closed already
+  }
+});
+
+});

test/support/assert.js

@@ -133,7 +133,7 @@ assert.response = function(server, req, res, msg){
             timer = setTimeout(function(){
                 check();
                 delete req.timeout;
-                assert.fail(msg + 'Request timed out after ' + requestTimeout + 'ms.');
+                request.destroy(); // will trigger 'error' event
             }, requestTimeout);
         }