Merge pull request #198 from opentable/error-messages

More details on errors from node client

client/src/get-components-data.js

@@ -1,5 +1,6 @@
 'use strict';
 
+var format = require('stringformat');
 var request = require('minimal-request');
 
 var settings = require('./settings');
@@ -50,8 +51,12 @@ module.exports = function(config){
     makePostRequest(serverRendering.components, options, function(error, responses){
       if(!!error || !responses || _.isEmpty(responses)){
         responses = [];
+        var errorDetails = !!error ? error.toString() : settings.emptyResponse;
         _.each(serverRendering.components, function(){
-          responses.push({ status: -1 });
+          responses.push({
+            response: { error: format(settings.connectionError, errorDetails, config.registries.serverRendering) },
+            status: 500
+          });
         });
       }
 
@@ -60,7 +65,8 @@ module.exports = function(config){
 
         if(action.render === 'server'){
           if(response.status !== 200){
-            action.result.error = serverRenderingFail;
+            var errorDetails = format('{0} ({1})', (response.response && response.response.error) || '', response.status);
+            action.result.error = new Error(format(serverRenderingFail, errorDetails));
             if(!!options.disableFailoverRendering){
               action.result.html = '';
               action.done = true;

client/src/render-components.js

@@ -1,5 +1,7 @@
 'use strict';
 
+var format = require('stringformat');
+
 var GetCompiledTemplate = require('./get-compiled-template');
 var settings = require('./settings');
 var _ = require('./utils/helpers');
@@ -48,7 +50,8 @@ module.exports = function(cache, renderTemplate){
     _.eachAsync(toRender, function(action, next){
       fetchTemplateAndRender(action.apiResponse, options, function(err, html){
         if(!!err){
-          action.result.error = settings.serverRenderingFail;
+          var errorDetails = format('{0} ({1})', (err.response && err.response.error), err.status);
+          action.result.error = new Error(format(settings.serverRenderingFail, errorDetails));
           if(!!options.disableFailoverRendering){
             action.result.html = '';
             action.done = true;

client/src/settings.js

@@ -3,8 +3,10 @@
 module.exports = {
   clientSideRenderingFail: 'Client-side rendering failed',
   configurationNotValid: 'Configuration is not valid: ',
+  connectionError: '{0} when connecting to {1}',
+  emptyResponse: 'Empty response',
   genericError: 'Internal client error',
   registriesEmpty: 'registries must contain at least one endpoint',
   registriesIsNotObject: 'registries must be an object',
-  serverSideRenderingFail: 'Server-side rendering failed'
+  serverSideRenderingFail: 'Server-side rendering failed: {0}'
 };

test/acceptance/client.js

@@ -137,7 +137,7 @@ describe('The node.js OC client', function(){
         });
 
         it('should return an error for the component with error', function(){
-          expect($errs[0]).to.not.be.null;
+          expect($errs[0].toString()).to.be.equal('Error: Server-side rendering failed: Component "hello-world-i-dont-exist" not found on local repository (404)');
           expect($errs[1]).to.be.null;
         });
       });
@@ -207,7 +207,7 @@ describe('The node.js OC client', function(){
         });
 
         it('should contain the error details', function(){
-          expect(error).to.eql('Server-side rendering failed');
+          expect(error.toString()).to.match(/Error\: Server-side rendering failed: Error: (.*?) when connecting to http\:\/\/localhost\:1234 \(500\)/g);
         });
       });
 
@@ -240,7 +240,7 @@ describe('The node.js OC client', function(){
         });
 
         it('should contain the error details', function(){
-          expect(error).to.equal('Server-side rendering failed');
+          expect(error.toString()).to.match(/Error\: Server-side rendering failed: Error: (.*?) when connecting to http\:\/\/localhost\:1234 \(500\)/g);
         });
       });
 
@@ -279,7 +279,7 @@ describe('The node.js OC client', function(){
         });
 
         it('should contain the error details', function(){
-          expect(error).to.eql('Server-side rendering failed');
+          expect(error.toString()).to.match(/Error\: Server-side rendering failed: Error: (.*?) when connecting to http\:\/\/localhost\:1234 \(500\)/g);
         });
       });
 
@@ -313,16 +313,18 @@ describe('The node.js OC client', function(){
         });
 
         it('should contain the error details', function(){
-          expect(error).to.eql('Server-side rendering failed');
+          expect(error.toString()).to.match(/Error\: Server-side rendering failed: Error: (.*?) when connecting to http\:\/\/localhost\:1234 \(500\)/g);
         });
       });
     });
 
     describe('when server-side rendering an existing component linked to a responsive registry', function(){
 
       describe('when container option = true', function(){
+        var error;
         before(function(done){
           client.renderComponent('hello-world', { container: true }, function(err, html){
+            error = err;
             $component = cheerio.load(html)('oc-component');
             done();
           });
@@ -344,6 +346,10 @@ describe('The node.js OC client', function(){
         it('should contain the component version', function(){
           expect($component.data('version')).to.equal('1.0.0');
         });
+
+        it('should contain a null error', function(){
+          expect(error).to.be.null;
+        });
       });
 
       describe('when container option = false', function(){