Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More details on errors from node client #198

Merged
merged 4 commits into from
Mar 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions client/src/get-components-data.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

var format = require('stringformat');
var request = require('minimal-request');

var settings = require('./settings');
Expand Down Expand Up @@ -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
});
});
}

Expand All @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion client/src/render-components.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

var format = require('stringformat');

var GetCompiledTemplate = require('./get-compiled-template');
var settings = require('./settings');
var _ = require('./utils/helpers');
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion client/src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}'
};
16 changes: 11 additions & 5 deletions test/acceptance/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});
Expand Down Expand Up @@ -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);
});
});

Expand Down Expand Up @@ -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);
});
});

Expand Down Expand Up @@ -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);
});
});

Expand Down Expand Up @@ -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();
});
Expand All @@ -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(){
Expand Down