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

set errors on the chunk when catching errors so that the proper callbacks are invoked... #471

Merged
merged 4 commits into from
Jun 11, 2014
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
2 changes: 1 addition & 1 deletion lib/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
//
// for the common case (using compile and render) a name is required so that templates will be cached by name and rendered later, by name.
if (!name && name !== null) {
dust.log(new Error("Template name parameter cannot be undefined when calling dust.compile"), 'ERROR');
throw new Error('Template name parameter cannot be undefined when calling dust.compile');
}

try {
Expand Down
38 changes: 7 additions & 31 deletions lib/dust.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
loggerContext;

dust.debugLevel = NONE;
dust.silenceErrors = false;

// Try to find the console in global scope
if (root && root.console && root.console.log) {
Expand All @@ -39,7 +38,8 @@
} : function() { /* no op */ };

/**
* If dust.isDebug is true, Log dust debug statements, info statements, warning statements, and errors.
* Log dust debug statements, info statements, warning statements, and errors.
* Filters out the messages based on the dust.debuglevel.
* This default implementation will print to the console if it exists.
* @param {String|Error} message the message to print/throw
* @param {String} type the severity of the message(ERROR, WARN, INFO, or DEBUG)
Expand All @@ -54,31 +54,6 @@
dust.logQueue.push({message: message, type: type});
logger.log('[DUST ' + type + ']: ' + message);
}

if (!dust.silenceErrors && type === ERROR) {
if (typeof message === 'string') {
throw new Error(message);
} else {
throw message;
}
}
};

/**
* If debugging is turned on(dust.isDebug=true) log the error message and throw it.
* Otherwise try to keep rendering. This is useful to fail hard in dev mode, but keep rendering in production.
* @param {Error} error the error message to throw
* @param {Object} chunk the chunk the error was thrown from
* @public
*/
dust.onError = function(error, chunk) {
logger.log('[!!!DEPRECATION WARNING!!!]: dust.onError will no longer return a chunk object.');
dust.log(error.message || error, ERROR);
if(!dust.silenceErrors) {
throw error;
} else {
return chunk;
}
};

dust.helpers = {};
Expand All @@ -97,17 +72,18 @@
try {
dust.load(name, chunk, Context.wrap(context, name)).end();
} catch (err) {
dust.log(err, ERROR);
chunk.setError(err);
}
};

dust.stream = function(name, context) {
var stream = new Stream();
var stream = new Stream(),
chunk = stream.head;
dust.nextTick(function() {
try {
dust.load(name, stream.head, Context.wrap(context, name)).end();
} catch (err) {
dust.log(err, ERROR);
chunk.setError(err);
}
});
return stream;
Expand Down Expand Up @@ -787,7 +763,7 @@
return chunk;
}
} catch (err) {
dust.log(err, ERROR);
chunk.setError(err);
return chunk;
}
};
Expand Down
2 changes: 1 addition & 1 deletion test/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ function testRender(unit, source, context, expected, options, baseContext, error
}
});
} catch(err) {
unit.equals(err.message, error);
unit.equals(err.message || err, error);
}
unit.pass();
};
Expand Down
33 changes: 2 additions & 31 deletions test/jasmine-test/spec/coreTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1829,40 +1829,11 @@ var coreTests = [
message: "test the log messages for an existing not exists check."
},
{
name: "Errors should be propogated if the silenceErrors flag is not set",
source: "{#errorHelper}{/errorHelper}",
context: { "errorHelper" : function(chunk, context, bodies, params)
{
dust.debugLevel = 'NONE';
throw new Error('Error should be visible');
return chunk.write('');
}
},
error: "Error should be visible",
message: "test to make sure errors are not swallowed."
},
{
name: "Errors should not be propogated when the silenceErrors is set to true",
source: "Error should NOT be visible{#errorHelper}{/errorHelper}",
context: { "error": {
"errorHelper" : function(chunk, context, bodies, params)
{
dust.debugLevel = 'NONE';
dust.silenceErrors = true;
throw new Error('Error message');
return chunk.write('');
}
}
},
expected: "Error should NOT be visible",
message: "test to make sure non dust errors are swallowed when the silenceErrors flag is set."
},
{
name: "Errors should be throwable from helpers",
name: "Errors should be throwable from helpers and consumed in the render callback/stream onerror",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not backwards compatible. 2.4?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors are still propagated, but in the way that you can actually catch them :). I don't mind bumping to 2.4 though.

With this change do we still need dust.silenceErrors flag?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 deprecate silenceErrors and deprecate dust.log throwing errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the new dust.throwError. It's still useful if you want to continue rendering in certain cases when throwing an error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throwError is a misnomer.... maybe dust.fail instead? If so, it should fail pretty catastrophically.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or... chunk.fail so that it's scoped to within the helper user function code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed there isn't really a use case for trying to throw errors since they are meaningless for the async cases.

for consistency, if you want to throw use chunk.setError and if you want to catch look in the errorback

source: "{@error errorMessage=\"helper error\"}{/error}",
context: { },
error: "helper error",
message: "test to make sure errors are properly caught and thrown when thrown from helpers."
message: "test to make sure errors are properly caught and propogated to the callbacks."
}
]
}
Expand Down
15 changes: 10 additions & 5 deletions test/jasmine-test/spec/renderTestSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ function render(test) {
}
dust.render(test.name, context, function(err, output) {
var log = dust.logQueue;
expect(err).toBeNull();
if (test.error) {
expect(test.error).toEqual(err.message || err);
} else {
expect(err).toBeNull();
}
if (test.log) {
for(var i=0; i<log.length; i++) {
if(log[i].message === test.log) {
Expand Down Expand Up @@ -99,7 +103,8 @@ function stream(test) {
log = dust.logQueue;
})
.on('error', function(err) {
output = err.message;
flag = true;
output = err.message || err;
log = dust.logQueue;
})
} catch(error) {
Expand Down Expand Up @@ -193,9 +198,9 @@ function pipe(test) {
},
error: function (err) {
flag = true;
output = err.message;
output = err.message || err;
log = dust.logQueue;
}
}
});
// Pipe to a second stream to test multiple event-listeners
tpl.pipe({
Expand All @@ -209,7 +214,7 @@ function pipe(test) {
},
error: function (err) {
flagTwo = true;
outputTwo = err.message;
outputTwo = err.message || err;
logTwo = logTwo.concat(dust.logQueue);
}
});
Expand Down