From 6dba06130bbe09f6851b5bac4eff2450ee4cf268 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 18 Apr 2017 11:40:37 +0200
Subject: [PATCH] Retur a proper error message when ogr2ogr command fails

---
 app/models/formats/ogr.js           | 37 +++++++++++------------------
 test/acceptance/export/shapefile.js | 10 ++++----
 2 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/app/models/formats/ogr.js b/app/models/formats/ogr.js
index 7310e080..38cdb948 100644
--- a/app/models/formats/ogr.js
+++ b/app/models/formats/ogr.js
@@ -173,34 +173,25 @@ OgrFormat.prototype.toOGR = function(options, out_format, out_filename, callback
         next(err);
       });
 
-      var stdout = '';
-      child.stdout.on('data', function(data) {
-        stdout += data;
-        //console.log('stdout: ' + data);
-      });
-
-      var stderr;
-      var logErrPat = new RegExp(/^ERROR/);
-      child.stderr.on('data', function(data) {
-        data = data.toString(); // know of a faster way ?
-        // Store only the first ERROR line
-        if ( ! stderr && data.match(logErrPat) ) {
-            stderr = data;
-        }
-        console.log('ogr2ogr stderr: ' + data);
+      var stderrData = [];
+      child.stderr.setEncoding('utf8');
+      child.stderr.on('data', function (data) {
+        stderrData.push(data);
       });
 
       child.on('exit', function(code) {
-        if ( code ) {
-          console.log("ogr2ogr exited with code " + code);
-          var emsg = stderr ? stderr.split('\n')[0] : ( "unknown ogr2ogr error (code " + code + ")" );
-          // TODO: add more info about this error ?
-          //if ( RegExp(/attempt to write non-.*geometry.*to.*type shapefile/i).exec(emsg) )
-          next(new Error(emsg));
-        } else {
-          next(null);
+        if (code !== 0) {
+          var errMessage = 'ogr2ogr command return code ' + code;
+          if (stderrData.length > 0) {
+            errMessage += ', Error: ' + stderrData.join('\n');
+          }
+
+          return next(new Error(errMessage));
         }
+
+        return next();
       });
+
     },
     function finish(err) {
       callback(err, out_filename);
diff --git a/test/acceptance/export/shapefile.js b/test/acceptance/export/shapefile.js
index ec04a389..fb516084 100644
--- a/test/acceptance/export/shapefile.js
+++ b/test/acceptance/export/shapefile.js
@@ -197,8 +197,9 @@ it('mixed type geometry', function(done){
         assert.deepEqual(res.headers['content-disposition'], 'inline');
         assert.equal(res.statusCode, 400, res.statusCode + ': ' +res.body);
         var parsedBody = JSON.parse(res.body);
-        var expectedBody = {"error":["ERROR 1: Attempt to write non-point (LINESTRING) geometry to point shapefile."]};
-        assert.deepEqual(parsedBody, expectedBody);
+        var error = parsedBody.error[0];
+        var expectedError = /Attempt to write non-point \(LINESTRING\) geometry to point shapefile/g;
+        assert.ok(expectedError.test(error), error);
         done();
     });
 });
@@ -222,8 +223,9 @@ it('errors are not confused with warnings', function(done){
         assert.deepEqual(res.headers['content-disposition'], 'inline');
         assert.equal(res.statusCode, 400, res.statusCode + ': ' +res.body);
         var parsedBody = JSON.parse(res.body);
-        var expectedBody = {"error":["ERROR 1: Attempt to write non-point (LINESTRING) geometry to point shapefile."]};
-        assert.deepEqual(parsedBody, expectedBody);
+        var error = parsedBody.error[0];
+        var expectedError = /Attempt to write non-point \(LINESTRING\) geometry to point shapefile/g;
+        assert.ok(expectedError.test(error), error);
         done();
     });
 });