From 61765d20e1dc4c03260fc2155aed7d5eae060223 Mon Sep 17 00:00:00 2001
From: Raul Ochoa <rochoaf@gmail.com>
Date: Fri, 13 May 2016 12:10:05 +0200
Subject: [PATCH] Fail on turbo-carto specific errors

This will try to fallback on postcss errors so it still targets
carto parser in those cases.

Closes #434
---
 .../utils/style/turbo-carto-adapter.js        | 14 ++-
 test/acceptance/turbo-cartocss/error-cases.js | 91 +++++++++++++++++++
 2 files changed, 101 insertions(+), 4 deletions(-)
 create mode 100644 test/acceptance/turbo-cartocss/error-cases.js

diff --git a/lib/cartodb/utils/style/turbo-carto-adapter.js b/lib/cartodb/utils/style/turbo-carto-adapter.js
index 1c197ac7..7475c44a 100644
--- a/lib/cartodb/utils/style/turbo-carto-adapter.js
+++ b/lib/cartodb/utils/style/turbo-carto-adapter.js
@@ -38,12 +38,18 @@ TurboCartoAdapter.prototype._parseCartoCss = function (username, layer, callback
     }
 
     this.turboCartoParser.process(username, layer.options.cartocss, layer.options.sql, function (err, cartocss) {
-        // Ignore turbo-carto errors and continue
-        if (!err && cartocss) {
-            layer.options.cartocss = cartocss;
+        // Only return turbo-carto errors
+        if (err && err.name === 'TurboCartoError') {
+            err = new Error('turbo-carto: ' + err.message);
+            err.http_status = 400;
+            return callback(err);
         }
 
-        callback(null, layer);
+        // Try to continue in the rest of the cases
+        if (cartocss) {
+            layer.options.cartocss = cartocss;
+        }
+        return callback(null, layer);
     });
 };
 
diff --git a/test/acceptance/turbo-cartocss/error-cases.js b/test/acceptance/turbo-cartocss/error-cases.js
new file mode 100644
index 00000000..877b3770
--- /dev/null
+++ b/test/acceptance/turbo-cartocss/error-cases.js
@@ -0,0 +1,91 @@
+require('../../support/test_helper');
+
+var assert = require('../../support/assert');
+var TestClient = require('../../support/test-client');
+
+function makeMapconfig(markerWidth, markerFill) {
+    return {
+        "version": "1.4.0",
+        "layers": [
+            {
+                "type": 'mapnik',
+                "options": {
+                    "cartocss_version": '2.3.0',
+                    "sql": 'SELECT * FROM populated_places_simple_reduced',
+                    "cartocss": createCartocss(markerWidth, markerFill)
+                }
+            }
+        ]
+    };
+}
+
+function createCartocss(markerWidth, markerFill) {
+    return [
+        "#populated_places_simple_reduced {",
+        "  marker-fill-opacity: 0.9;",
+        "  marker-line-color: #FFF;",
+        "  marker-line-width: 1;",
+        "  marker-line-opacity: 1;",
+        "  marker-placement: point;",
+        "  marker-type: ellipse;",
+        "  marker-allow-overlap: true;",
+        "  marker-width: " + (markerWidth || '10') + ";",
+        "  marker-fill: " + (markerFill || 'red') + ";",
+        "}"
+    ].join('\n');
+}
+
+var ERROR_RESPONSE = {
+    status: 400,
+    headers: {
+        'Content-Type': 'application/json; charset=utf-8'
+    }
+};
+
+describe('turbo-carto error cases', function() {
+    afterEach(function (done) {
+        if (this.testClient) {
+            this.testClient.drain(done);
+        }
+    });
+
+    it('should return invalid number of ramp error', function(done) {
+        this.testClient = new TestClient(makeMapconfig('ramp([pop_max], (8,24,96), (8,24,96,128))'));
+        this.testClient.getLayergroup(ERROR_RESPONSE, function(err, layergroup) {
+            assert.ok(!err, err);
+
+            assert.ok(layergroup.hasOwnProperty('errors'));
+            assert.equal(layergroup.errors.length, 1);
+            assert.ok(layergroup.errors[0].match(/invalid\sramp\slength/i));
+
+            done();
+        });
+    });
+
+    it('should return invalid column from datasource', function(done) {
+        this.testClient = new TestClient(makeMapconfig(null, 'ramp([wadus_column], (red, green, blue))'));
+        this.testClient.getLayergroup(ERROR_RESPONSE, function(err, layergroup) {
+            assert.ok(!err, err);
+
+            assert.ok(layergroup.hasOwnProperty('errors'));
+            assert.equal(layergroup.errors.length, 1);
+            assert.ok(layergroup.errors[0].match(/datasource/));
+            assert.ok(layergroup.errors[0].match(/wadus_column/));
+
+            done();
+        });
+    });
+
+    it('should fail by falling back to normal carto parser', function(done) {
+        this.testClient = new TestClient(makeMapconfig('ramp([price], (8,24,96), (8,24,96));//(red, green, blue))'));
+        this.testClient.getLayergroup(ERROR_RESPONSE, function(err, layergroup) {
+            assert.ok(!err, err);
+
+            assert.ok(layergroup.hasOwnProperty('errors'));
+            assert.equal(layergroup.errors.length, 1);
+            assert.ok(layergroup.errors[0].match(/invalid\scode/i));
+
+            done();
+        });
+    });
+});