From 7eb412cf5921c3e582321d83d7a01de03768be43 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Wed, 2 Jan 2019 13:09:42 +0100 Subject: [PATCH 1/3] Test: Accept PG11 error messages --- test/acceptance/app.test.js | 14 ++++---------- test/acceptance/auth-api.js | 6 +++--- test/acceptance/copy-statements.js | 8 ++++---- test/acceptance/export/csv.js | 9 +++------ 4 files changed, 14 insertions(+), 23 deletions(-) diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 9f649815..2a6c8f1a 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -105,9 +105,7 @@ it('GET /api/v1/sql with INSERT. oAuth not used, so public user - should fail', assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body); assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8'); assert.deepEqual(res.headers['content-disposition'], 'inline'); - assert.deepEqual(JSON.parse(res.body), - {"error":["permission denied for relation untitle_table_4"]} - ); + assert.ok(JSON.parse(res.body).error[0].match(/permission denied for .+? untitle_table_4/)); done(); }); }); @@ -122,9 +120,7 @@ it('GET /api/v1/sql with DROP TABLE. oAuth not used, so public user - should fai assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8'); assert.deepEqual(res.headers['content-disposition'], 'inline'); - assert.deepEqual(JSON.parse(res.body), - {"error":["must be owner of relation untitle_table_4"]} - ); + assert.ok(JSON.parse(res.body).error[0].match(/must be owner of.+? untitle_table_4/)); done(); }); }); @@ -148,9 +144,7 @@ it('GET /api/v1/sql with SQL parameter on DROP TABLE. should fail', function(don assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8'); assert.deepEqual(res.headers['content-disposition'], 'inline'); - assert.deepEqual(JSON.parse(res.body), - {"error":["must be owner of relation untitle_table_4"]} - ); + assert.ok(JSON.parse(res.body).error[0].match(/must be owner of.+? untitle_table_4/)); done(); }); }); @@ -767,7 +761,7 @@ it('GET with callback must return 200 status error even if it is an error', func var didRunJsonCallback = false; // jshint ignore:start function foo_jsonp(body) { - assert.deepEqual(body, {"error":["must be owner of relation untitle_table_4"]}); + assert.ok(body.error[0].match(/must be owner of.+? untitle_table_4/)); didRunJsonCallback = true; } eval(res.body); diff --git a/test/acceptance/auth-api.js b/test/acceptance/auth-api.js index d7642749..82304981 100644 --- a/test/acceptance/auth-api.js +++ b/test/acceptance/auth-api.js @@ -46,7 +46,7 @@ describe('Auth API', function () { }; this.testClient.getResult(privateSQL, expectedResponse, (err, result) => { assert.ifError(err); - assert.equal(result.error, 'permission denied for relation private_table'); + assert.ok(result.error[0].match(/permission denied for .+? private_table/)); done(); }); }); @@ -88,7 +88,7 @@ describe('Auth API', function () { this.testClient.getResult(scopedSQL, expectedResponse, (err, result) => { assert.ifError(err); - assert.equal(result.error, 'permission denied for relation scoped_table_1'); + assert.ok(result.error[0].match(/permission denied for .+? scoped_table_1/)); done(); }); }); @@ -183,7 +183,7 @@ describe('Auth API', function () { this.testClient.getResult(scopedSQL, expectedResponse, (err, result) => { assert.ifError(err); - assert.equal(result.error, 'permission denied for relation scoped_table_1'); + assert.ok(result.error[0].match(/permission denied for .+? scoped_table_1/)); done(); }); }); diff --git a/test/acceptance/copy-statements.js b/test/acceptance/copy-statements.js index 4a2c37ad..6fb5c8f3 100644 --- a/test/acceptance/copy-statements.js +++ b/test/acceptance/copy-statements.js @@ -68,10 +68,10 @@ describe('copy-statements', function() { assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8'); assert.deepEqual(res.headers['content-disposition'], 'inline'); - assert.deepEqual(JSON.parse(res.body), { - error: ["must be superuser to COPY to or from a file"], - hint: "Anyone can COPY to stdout or from stdin. psql's \\copy command also works for anyone." - }); + const error_exp = /must be superuser.* to COPY.* a file/; + const hint_exp = /Anyone can COPY to stdout or from stdin. psql's \\copy command also works for anyone./; + assert.ok(JSON.parse(res.body).error[0].match(error_exp)); + assert.ok(JSON.parse(res.body).hint.match(hint_exp)); done(); }); }); diff --git a/test/acceptance/export/csv.js b/test/acceptance/export/csv.js index 9d75ac44..7f37b42c 100644 --- a/test/acceptance/export/csv.js +++ b/test/acceptance/export/csv.js @@ -126,8 +126,7 @@ it('GET /api/v1/sql as csv', function(done){ method: 'GET' },{ }, function(err, res){ assert.equal(res.statusCode, 200, res.body); - var expected = 'cartodb_id,geom\r\n1,"SRID=4326;POINT(-3.699732 40.423012)"\r\n'; - assert.equal(res.body, expected); + assert.ok(res.body.match(/cartodb_id,geom\r\n.?1.?,"SRID=4326;POINT(.*)"\r\n/)); done(); }); }); @@ -155,8 +154,7 @@ it('GET /api/v1/sql as csv, properly escaped', function(done){ method: 'GET' },{ }, function(err, res){ assert.equal(res.statusCode, 200, res.body); - var expected = 'cartodb_id,address\r\n1,"Calle de Pérez Galdós 9, Madrid, Spain"\r\n'; - assert.equal(res.body, expected); + assert.ok(res.body.match(/cartodb_id,address\r\n.?1.?,"Calle de Pérez Galdós 9, Madrid, Spain"\r\n/)); done(); }); }); @@ -166,8 +164,7 @@ it('GET /api/v1/sql as csv, concurrently', function(done){ var concurrency = 4; var waiting = concurrency; function validate(err, res){ - var expected = 'cartodb_id,address\r\n1,"Calle de Pérez Galdós 9, Madrid, Spain"\r\n'; - assert.equal(res.body, expected); + assert.ok(res.body.match(/cartodb_id,address\r\n.?1.?,"Calle de Pérez Galdós 9, Madrid, Spain"\r\n/)); if ( ! --waiting ) { done(); } From 4662077a02813d892845d038e958388971ad1a5e Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Wed, 2 Jan 2019 15:52:15 +0100 Subject: [PATCH 2/3] Doc: Document errors in streaming responses --- doc/making_calls.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/making_calls.md b/doc/making_calls.md index daa57e19..37d33832 100644 --- a/doc/making_calls.md +++ b/doc/making_calls.md @@ -2,7 +2,7 @@ CARTO is based on the rock solid PostgreSQL database. All of your tables reside in a single database, which means you can perform complex queries joining tables, or carrying out geospatial operations. The best place to learn about PostgreSQL's SQL language is the [official documentation](http://www.postgresql.org/docs/9.1/static/). -CARTO is also based on PostGIS, so you can view the [official PostGIS reference](http://postgis.refractions.net/docs/) to know what functionality we support in terms of geospatial operations. All of our tables include a column called *the_geom,* which is a geometry field that indexes geometries in the EPSG:4326 (WGS 1984) coordinate system. All tables also have an automatically generated and updated column called *the_geom_webmercator*. We use the column internally to quickly create tiles for maps. +CARTO is also based on PostGIS, so you can view the [official PostGIS reference](https://postgis.net/documentation/) to know what functionality we support in terms of geospatial operations. All of our tables include a column called *the_geom,* which is a geometry field that indexes geometries in the EPSG:4326 (WGS 1984) coordinate system. All tables also have an automatically generated and updated column called *the_geom_webmercator*. We use the column internally to quickly create tiles for maps. ## URL Endpoints @@ -172,6 +172,8 @@ To help you debug your SQL queries, the CARTO SQL API returns the full error pro } ``` +**WARNING**: If the database finds an error after it has started streaming the response, the error header will still be set but the HTTP status code will be **200**. + You can use these errors to help understand your SQL. If you encounter errors executing SQL, either through CARTO Builder, or through the SQL API, it is suggested to Google search the error for independent troubleshooting. ## Write Data to your CARTO Account From 03701ae69503c6cb88a62c92f18e05dc28ef54bf Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Wed, 2 Jan 2019 16:15:28 +0100 Subject: [PATCH 3/3] Set platform limits message on streaming responses too --- app/models/formats/pg/geojson.js | 3 +- app/models/formats/pg/json.js | 3 +- test/acceptance/app.test.js | 115 +++++++++++++++++-------------- 3 files changed, 69 insertions(+), 52 deletions(-) diff --git a/app/models/formats/pg/geojson.js b/app/models/formats/pg/geojson.js index fde457f9..b5241f71 100644 --- a/app/models/formats/pg/geojson.js +++ b/app/models/formats/pg/geojson.js @@ -3,6 +3,7 @@ var _ = require('underscore'); var pg = require('./../pg'); +const errorHandlerFactory = require('../../../services/error_handler_factory'); function GeoJsonFormat() { this.buffer = ''; @@ -72,7 +73,7 @@ GeoJsonFormat.prototype.handleQueryEnd = function(/*result*/) { this.buffer += ']'; // end of features if (this.error) { - this.buffer += ',"error":' + JSON.stringify([this.error.message]); + this.buffer += ',"error":' + JSON.stringify(errorHandlerFactory(this.error).getResponse().error); } this.buffer += '}'; // end of root object diff --git a/app/models/formats/pg/json.js b/app/models/formats/pg/json.js index acdb11c7..d4dff9e5 100644 --- a/app/models/formats/pg/json.js +++ b/app/models/formats/pg/json.js @@ -3,6 +3,7 @@ var _ = require('underscore'); var pg = require('./../pg'); +const errorHandlerFactory = require('../../../services/error_handler_factory'); function JsonFormat() { this.buffer = ''; @@ -131,7 +132,7 @@ JsonFormat.prototype.handleQueryEnd = function(result) { ]; if (this.error) { - out.push(',"error":', JSON.stringify([this.error.message])); + out.push(',"error":', JSON.stringify(errorHandlerFactory(this.error).getResponse().error)); } diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 2a6c8f1a..5e67d378 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -771,59 +771,74 @@ it('GET with callback must return 200 status error even if it is an error', func }); }); - it('GET with slow query exceeding statement timeout returns proper error message', function(done){ - assert.response(server, { - url: "/api/v1/sql?q=select%20pg_sleep(2.1)%20as%20sleep", - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - }, - { - status: 429, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }, - function(err, res) { - var error = JSON.parse(res.body); - assert.deepEqual(error, { - error: [ - 'You are over platform\'s limits: SQL query timeout error.' + - ' Refactor your query before running again or contact CARTO support for more details.', - ], - context: 'limit', - detail: 'datasource' - }); +it('GET with slow query exceeding statement timeout returns proper error message', function(done){ + assert.response(server, { + url: "/api/v1/sql?q=select%20pg_sleep(2.1)%20as%20sleep", + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + { + // status: 429, ---> Both 200 and 429 are valid + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }, + function(err, res) { + var error = JSON.parse(res.body); + assert.deepEqual(error.error, [ + 'You are over platform\'s limits: SQL query timeout error.' + + ' Refactor your query before running again or contact CARTO support for more details.' + ]); - done(); - }); - }); + done(); + }); +}); - it('GET with slow python script exceeding statement timeout returns proper error message', function(done){ - assert.response(server, { - url: "/api/v1/sql?q=select%20py_sleep(2.1)", - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - }, - { - status: 429, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }, - function(err, res) { - var error = JSON.parse(res.body); - assert.deepEqual(error, { - error: [ - 'You are over platform\'s limits: SQL query timeout error.' + - ' Refactor your query before running again or contact CARTO support for more details.', - ], - context: 'limit', - detail: 'datasource' - }); +it('GET with slow query exceeding statement timeout returns proper error message (streaming)', function(done){ + assert.response(server, { + url: "/api/v1/sql?q=SELECT%20pg_sleep(generate_series(2,10)/10.0)", + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + { + // status: 429, ---> Both 200 and 429 are valid + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }, + function(err, res) { + var error = JSON.parse(res.body); + assert.deepEqual(error.error, [ + 'You are over platform\'s limits: SQL query timeout error.' + + ' Refactor your query before running again or contact CARTO support for more details.' + ]); - done(); - }); - }); + done(); + }); +}); + +it('GET with slow python script exceeding statement timeout returns proper error message', function(done){ + assert.response(server, { + url: "/api/v1/sql?q=select%20py_sleep(2.1)", + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + { + // status: 429, ---> Both 200 and 429 are valid + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }, + function(err, res) { + var error = JSON.parse(res.body); + assert.deepEqual(error.error, [ + 'You are over platform\'s limits: SQL query timeout error.' + + ' Refactor your query before running again or contact CARTO support for more details.' + ]); + + done(); + }); +}); it('too large rows get into error log', function(done){