Skip to content

Commit

Permalink
Throw if templating a file fails
Browse files Browse the repository at this point in the history
Previously, if an error was thrown during the template rendering
process, the error was simply swallowed. This was likely to avoid
errors when attempting to template a binary file, since the byte
stream, when converted to a string, could contain template markers.

Adds the isbinaryfile package to detect if the file is binary. If
so, it skips templating entirely. If not, it attempts to template
the file, and allows exceptions to reject the render promise.

This happened to expose some tests that should have been failing
around the http-proxy generators (the tests were not supplying
enough arguments). Fixes those tests too.
  • Loading branch information
davewasmer committed Apr 4, 2015
1 parent 2d2e21c commit e5e374b
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 11 deletions.
14 changes: 9 additions & 5 deletions lib/models/file-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
var fs = require('fs');
var Promise = require('../ext/promise');
var readFile = Promise.denodeify(fs.readFile);
var lstat = Promise.denodeify(fs.stat);
var chalk = require('chalk');
var EditFileDiff = require('./edit-file-diff');
var EOL = require('os').EOL;
var isBinaryFile = require('isbinaryfile');

function processTemplate(content, context) {
return require('lodash/string/template')(content)(context);
Expand Down Expand Up @@ -84,11 +86,13 @@ FileInfo.prototype.render = function() {
context = this.templateVariables;
if (!this.rendered) {
this.rendered = readFile(path).then(function(content){
try {
return processTemplate(content.toString(), context);
} catch (error) {
return content.toString();
}
return lstat(path).then(function(fileStat) {
if (isBinaryFile(content, fileStat.size)) {
return content;
} else {
return processTemplate(content.toString(), context);
}
});
});
}
return this.rendered;
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
"chai-as-promised": "^4.3.0",
"ember-cli-ncp": "1.0.2",
"github": "^0.2.3",
"isbinaryfile": "^2.0.3",
"mocha": "^2.2.1",
"mocha-jshint": "1.0.0",
"multiline": "^1.0.2",
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/destroy-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ describe('Acceptance: ember destroy', function() {

it('http-proxy foo', function() {
this.timeout(20000);
var commandArgs = ['http-proxy', 'foo'];
var commandArgs = ['http-proxy', 'foo', 'bar'];
var files = ['server/proxies/foo.js'];

return assertDestroyAfterGenerate(commandArgs, files);
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/pods-destroy-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ describe('Acceptance: ember destroy pod', function() {
});

it('http-proxy foo --pod', function() {
var commandArgs = ['http-proxy', 'foo', '--pod'];
var commandArgs = ['http-proxy', 'foo', 'bar', '--pod'];
var files = ['server/proxies/foo.js'];

return assertDestroyAfterGenerate(commandArgs, files);
Expand Down
11 changes: 7 additions & 4 deletions tests/unit/models/file-info-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,19 @@ describe('Unit - FileInfo', function(){
});
});

it('falls back to raw text if template throws', function(){
it('rejects if templating throws', function(){
var templateWithUndefinedVariable = path.resolve(__dirname,
'../../fixtures/blueprints/with-templating/files/with-undefined-variable.txt');
var options = {};
assign(options, validOptions, { inputPath: templateWithUndefinedVariable });
var fileInfo = new FileInfo(options);

return fileInfo.render().then(function(output){
expect(output.trim()).to.equal('Howdy <%= enemy %>',
'expects to fall back to raw text');
return fileInfo.render().then(function() {
throw new Error('FileInfo.render should reject if templating throws');
}).catch(function(e) {
if (!e.toString().match(/ReferenceError/)) {
throw e;
}
});
});

Expand Down

0 comments on commit e5e374b

Please sign in to comment.