Skip to content

Commit

Permalink
Improve module error handling. Output exception details in debug and …
Browse files Browse the repository at this point in the history
…error placeholder or nothing in release.
  • Loading branch information
Denis Rechkunov committed Mar 26, 2014
1 parent a54420f commit 6610044
Show file tree
Hide file tree
Showing 12 changed files with 164 additions and 65 deletions.
11 changes: 10 additions & 1 deletion lib/PageRendererBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,23 @@ var moduleContextHelper = require('./helpers/moduleContextHelper');
* Creates new instance of base page renderer.
* @param {ModuleLoader} $moduleLoader Module loader to get modules.
* @param {Logger} $logger Logger to log messages.
* @param {boolean} isRelease Is current application mode release.
* @constructor
*/
function PageRendererBase($moduleLoader, $logger) {
function PageRendererBase($moduleLoader, $logger, isRelease) {
this._modulesByNames = $moduleLoader.getModulesByNames();
this._logger = $logger;
this._initializeTemplates();
this._isRelease = Boolean(isRelease);
}

/**
* Is current application mode release.
* @type {boolean}
* @private
*/
PageRendererBase.prototype._isRelease = false;

/**
* Set of modules by names.
* @type {Object}
Expand Down
11 changes: 10 additions & 1 deletion lib/client/ModuleLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,20 @@ ModuleLoader.prototype.getModulesByNames = function () {
self._templateProvider.registerCompiled(fullName,
placeholder.compiledSource);

module.placeholders[placeholder.name] = placeholder;
if (moduleContextHelper.isRootPlaceholder(placeholder.name)) {
return;
}

placeholder.getTemplateStream = function (data) {
return self._templateProvider.getStream(fullName, data);
};

if (moduleContextHelper.isErrorPlaceholder(placeholder.name)) {
module.errorPlaceholder = placeholder;
} else {
module.placeholders[placeholder.name] = placeholder;
}

self._logger.info(util.format(INFO_PLACEHOLDER_LOADED,
placeholder.name,
placeholder.moduleName
Expand Down
66 changes: 44 additions & 22 deletions lib/client/PageRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,20 @@ util.inherits(PageRenderer, PageRendererBase);
var TRACE_RENDER_PLACEHOLDER = 'Render placeholder "%s" of module "%s"',
ID_ATTRIBUTE_NAME = 'id',
LOADING_CLASS_NAME = 'loading',
BODY_ELEMENT_NAME = 'BODY';
BODY_ELEMENT_NAME = 'BODY',
ERROR_FORMAT = '%s<br/>%s';

/**
* Creates instance of client-side page renderer.
* @param {ModuleLoader} $moduleLoader Module loader to load modules set.
* @param {Window} $window Browser window to render content.
* @param {Logger} $logger Logger to log messages.
* @param {boolean} isRelease Is application mode release.
* @constructor
* @extends PageRendererBase
*/
function PageRenderer($moduleLoader, $window, $logger) {
PageRendererBase.call(this, $moduleLoader, $logger);
function PageRenderer($moduleLoader, $window, $logger, isRelease) {
PageRendererBase.call(this, $moduleLoader, $logger, isRelease);
this._window = $window;
}

Expand Down Expand Up @@ -126,6 +128,9 @@ PageRenderer.prototype.renderModule =
if (moduleContextHelper.isRootPlaceholder(placeholderName)) {
return;
}
if (moduleContextHelper.isErrorPlaceholder(placeholderName)) {
return;
}
// now search root of rendering for each placeholder
// of changed module
var id = moduleContextHelper.joinModuleNameAndContext(
Expand Down Expand Up @@ -215,6 +220,38 @@ PageRenderer.prototype._getIterationAction =
currentParameters = parameters[currentItem.moduleName] || {},
currentModule = self._modulesByNames[currentItem.moduleName];

var errorHandler = function (error) {
self._logger.error(error);
if (!self._isRelease && error instanceof Error) {
currentItem.element.empty();
currentItem.element.html(util.format(ERROR_FORMAT,
error.message, error.stack));
currentItem.element.removeClass(LOADING_CLASS_NAME);
endCheckAction();
} else if (currentModule.errorPlaceholder) {
streamHandler(
currentModule.errorPlaceholder.getTemplateStream(error));
} else {
currentItem.element.empty();
currentItem.element.removeClass(LOADING_CLASS_NAME);
}
};

var streamHandler = function (stream) {
currentItem.element.empty();
stream.on('data', function (chunk) {
currentItem.element.append(chunk);
});
stream.on('error', function (error) {
errorHandler(error);
endCheckAction();
});
stream.on('end', function () {
currentItem.element.removeClass(LOADING_CLASS_NAME);
checkInner();
});
};

// checks if placeholder has something to render inside itself
var checkInner = function () {
self._placeholderIds
Expand All @@ -240,14 +277,12 @@ PageRenderer.prototype._getIterationAction =
// module render data handler
var dataHandler = function (error, data) {
if (error) {
self._logger.error(error);
currentItem.element.removeClass(LOADING_CLASS_NAME);
endCheckAction();
errorHandler(error);
return;
}

// if module do not wish to render itself again
if (!data) {
if (!error && !data) {
currentItem.element.removeClass(LOADING_CLASS_NAME);
checkInner();
return;
Expand All @@ -256,20 +291,7 @@ PageRenderer.prototype._getIterationAction =
self._logger.trace(util.format(TRACE_RENDER_PLACEHOLDER,
currentItem.name, currentItem.moduleName));

currentItem.element.empty();
var stream = currentItem.getTemplateStream(data);
stream.on('data', function (chunk) {
currentItem.element.append(chunk);
});
stream.on('error', function (error) {
self._logger.error(error);
currentItem.element.removeClass(LOADING_CLASS_NAME);
endCheckAction();
});
stream.on('end', function () {
currentItem.element.removeClass(LOADING_CLASS_NAME);
checkInner();
});
streamHandler(currentItem.getTemplateStream(data));
};

try {
Expand All @@ -283,7 +305,7 @@ PageRenderer.prototype._getIterationAction =
currentModule.implementation.render(currentItem.name,
currentParameters, dataHandler);
} catch (e) {
self._logger.error(e);
errorHandler(e);
endCheckAction();
}
};
Expand Down
12 changes: 11 additions & 1 deletion lib/helpers/moduleContextHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ var url = require('url');
var SLASH_REPLACE_REGEXP = /(^(\/|\\))|((\/|\\)$)/g,
MODULE_CONTEXT_PARAMETER_REGEXP = /^\w+_.+$/i,
MODULE_CONTEXT_PREFIX_SEPARATOR = '_',
ROOT_PLACEHOLDER_NAME = '__index';
ROOT_PLACEHOLDER_NAME = '__index',
ERROR_PLACEHOLDER_NAME = '__error';

module.exports = {

Expand All @@ -48,6 +49,15 @@ module.exports = {
return placeholderName.toLowerCase() === ROOT_PLACEHOLDER_NAME;
},

/**
* Determines if specified placeholder name is a error placeholder name.
* @param {string} placeholderName Name of placeholder.
* @returns {boolean}
*/
isErrorPlaceholder: function (placeholderName) {
return placeholderName.toLowerCase() === ERROR_PLACEHOLDER_NAME;
},

/**
* Parses query string parameters grouping by modules and globals.
* @param {string} urlText HTTP request URL.
Expand Down
35 changes: 22 additions & 13 deletions lib/server/ClientBundleBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var path = require('path'),
UglifyJS = require('uglify-js'),
fs = require('fs'),
browserify = require('browserify'),
moduleContextHelper = require('../helpers/moduleContextHelper'),
packageDescriptionPath = path.join(process.cwd(), 'package.json'),
packageDescription;

Expand Down Expand Up @@ -81,7 +82,7 @@ function ClientBundleBuilder($config, $logger, $templateProvider, $moduleLoader,
this._templateProvider = $templateProvider;
this._clientTemplateEnginePath = $config.clientTemplateEnginePath;
this._moduleLoader = $moduleLoader;
this._isRelease = $config.isRelease;
this._isRelease = Boolean($config.isRelease);
this._serviceLocator = $serviceLocator;
if (!fs.existsSync(this._publicPath)) {
fs.mkdirSync(this._publicPath);
Expand Down Expand Up @@ -232,22 +233,30 @@ ClientBundleBuilder.prototype._generateRequiresForModules = function () {
* @private
*/
ClientBundleBuilder.prototype._generatePlaceholders = function () {
var placeholders = Object
var self = this,
results = [];
Object
.keys(this._templateProvider.compiledSources)
.map(function (fullName) {
var underscoreIndex = fullName.indexOf('_'),
moduleName = fullName.substring(0, underscoreIndex),
placeholderName = fullName.substring(underscoreIndex + 1);
return {
moduleName: moduleName,
name: placeholderName,
compiledSource: this._templateProvider
.forEach(function (fullName) {
var moduleAndContext =
moduleContextHelper.splitModuleNameAndContext(fullName);

// we do not need root templates at client-side
if (!moduleAndContext ||
moduleContextHelper.isRootPlaceholder(moduleAndContext.context)) {
return;
}

results.push({
moduleName: moduleAndContext.moduleName,
name: moduleAndContext.context,
compiledSource: self._templateProvider
.compiledSources[fullName].toString()
};
}, this);
});
});

return JSON
.stringify(placeholders)
.stringify(results)
.replace(BRACKETS_REGEXP, '');
};

Expand Down
5 changes: 5 additions & 0 deletions lib/server/ModuleLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ function loadModule(modulePath, context) {
return;
}

if (moduleContextHelper.isErrorPlaceholder(name)) {
module.errorPlaceholder = placeholder;
return;
}

module.placeholders[name] = placeholder;
});

Expand Down
8 changes: 4 additions & 4 deletions lib/server/PageRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,12 @@ util.inherits(PageRenderer, PageRendererBase);
* @param {ModuleLoader} $moduleLoader Module loader to get modules.
* @param {ResourceBuilder} $resourceBuilder Resource builder for assets.
* @param {Logger} $logger Logger to log errors and trace.
* @param {boolean} isRelease Is application mode release.
* @constructor
* @extends PageRendererBase
*/
function PageRenderer($moduleLoader, $resourceBuilder, $logger) {
PageRendererBase.call(this, $moduleLoader, $logger);
function PageRenderer($moduleLoader, $resourceBuilder, $logger, isRelease) {
PageRendererBase.call(this, $moduleLoader, $logger, isRelease);
initializePlaceholders(this);
$resourceBuilder.on('built', function () {
$logger.info(INFO_READY_TO_HANDLE_REQUESTS);
Expand All @@ -83,6 +84,7 @@ PageRenderer.prototype.render =
util.format(TRACE_RENDERING_PAGE, pageName));
try {
context = {
isRelease: this._isRelease,
parameters: parameters,
modulesByNames: this._modulesByNames,
placeholderIds: this._placeholderIds,
Expand All @@ -94,12 +96,10 @@ PageRenderer.prototype.render =

renderStream.on('error', function (error) {
self._logger.error(error);
next(error);
});

transformStream.on('error', function (error) {
self._logger.error(error);
next(error);
});

renderStream.render();
Expand Down
48 changes: 43 additions & 5 deletions lib/server/streams/ModuleReadable.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,33 @@ var stream = require('stream'),

util.inherits(ModuleReadable, stream.PassThrough);

var ERROR_FORMAT = '%s<br/>%s';

/**
* Creates new instance of module placeholder rendering stream.
* @param {Object} module Module which will render placeholder.
* @param {Object} placeholder Placeholder to render.
* @param {Object} parameters Set of request parameters.
* @param {boolean} isRelease Is application mode release.
* @constructor
* @extends PassThrough
*/
function ModuleReadable(module, placeholder, parameters) {
function ModuleReadable(module, placeholder, parameters, isRelease) {
stream.PassThrough.call(this);

this._placeholder = placeholder;
this._parameters = parameters;
this._module = module;
this._isRelease = Boolean(isRelease);
}

/**
* Is current application mode release.
* @type {boolean}
* @private
*/
ModuleReadable.prototype._isRelease = false;

/**
* If module rendering in progress this value is true.
* @type {boolean}
Expand Down Expand Up @@ -100,10 +111,11 @@ ModuleReadable.prototype.render = function () {
ownParameters,
function (error, content) {
if (error) {
self.emit('error', error);
self._errorHandler(error);
return;
}

if (error || !content) {
if (!content) {
self.end();
return;
}
Expand All @@ -115,7 +127,33 @@ ModuleReadable.prototype.render = function () {
contentStream.pipe(self);
});
} catch (e) {
this.emit('error', e);
this.end();
this._errorHandler(e);
}
};

/**
* Handles all errors.
* @param {Error} error
* @private
*/
ModuleReadable.prototype._errorHandler = function (error) {
var self = this;
setImmediate(function () {
self.emit('error', error);
// if application in debug mode then render
// error text in placeholder
if (!self._isRelease && error instanceof Error) {
var errorStream = new ContentReadable(
util.format(ERROR_FORMAT,
error.message, error.stack));
errorStream.pipe(self);
} else if (self._module.errorPlaceholder) {
var errorPlaceholderStream =
self._module.errorPlaceholder.getTemplateStream(error);
errorPlaceholderStream.pipe(self);
} else {
self.end();
}
});

};
1 change: 0 additions & 1 deletion lib/server/streams/ParserDuplex.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ ParserDuplex.prototype._read = function () {
}
this._replaceStream = this.foundTagIdHandler(tagId);
if (this._replaceStream) {
this._replaceStream.on('error', this._errorHandler.bind(this));
this._replaceStream.on('readable',
this._replaceHandler.bind(this));
this._replaceStream.on('end',
Expand Down

0 comments on commit 6610044

Please sign in to comment.