-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feature: Return errors separately #17
Comments
This is something I've wanted to do for a while but never got around to doing. I think we need to handle 2 different error scenarios: 1) When the evaluation of the task throws an error (as you noted) and 2) When the task fails to evaluate (ie. network issues, etc.) What do you think about the below? It would introduce a var engine = require("engine.js").engine;
var client = engine.client.create();
var task = client.createTask();
task.setContext("(function(locals){ return { add: function(a,b){ return a+b } } })");
task.setLocals({});
task.setCode('sub(1,2)');
task.on('eval', function(err, response){
/*
* err: An Error object that holds information relating to the failed execution
* of running the task. A user can trap this variable when the task fails to run.
*
* response: A TaskResponse object that holds information about the successful execution
* of the task.
* - response.getEvaluation() : The evaluation of the last statement in the task
* - response.isError() : Did the task throw an exception (SyntaxError,
* ReferenceError, SecurityError, etc.)?
* - response.getGlobals() : The context globals (after task execution)
* - response.getLocals() : The context locals (after task execution)
* - ...
* - (other properties can be added later, like execution time, etc. )
*/
});
task.run(); |
Oooh, yea I like this a lot. It avoids the confusion about the |
It would be nice if it were possible to tell the difference between when code resulted in an error and when it successfully returned a value. I'm thinking of something like this:
Or maybe a different event:
I think I should be able to implement this feature and will submit a pull request when I get a chance.
The text was updated successfully, but these errors were encountered: