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

Implementation opt-in error handling #8? #65

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
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
19 changes: 19 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,25 @@ user.save(function(err){
Destroy and invoke optional `fn(err)`.

Emits "destroy" when successfully deleted.

### .toError(fn)

Register a custom error handler for the model class and/or model instances.
If called on the Model class, the error handler is used for static methods as well as the methods of the model
instances. If called on a model instance, the error handler is only used for methods of this instance.

```js
var handler1 = function(err, res) {...};
var handler2 = function(err, res) {...};

var Car = model('Car')
.toError(handler1);

var car1 = new Car(); // uses handler1

var car2 = new Car();
car2.toError(handler2); // uses handler2
```

## Links

Expand Down
42 changes: 32 additions & 10 deletions lib/proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ exports.destroy = function(fn){
this.request
.del(url)
.set(this.model._headers)
.end(function(res){
if (res.error) return fn(error(res), res);
.end(function(err, res){
if (err || res.error)
return fn(self._toError(err,res), res);
self.destroyed = true;
self.model.emit('destroy', self, res);
self.emit('destroy');
Expand Down Expand Up @@ -178,8 +179,10 @@ exports.save = function(fn){
.post(url)
.set(this.model._headers)
.send(self)
.end(function(res){
if (res.error) return fn(error(res), res);
.end(function(err, res){
if (err || res.error)
return fn(self._toError(err,res), res);

if (res.body) self.primary(res.body[key]);
self.dirty = {};
self.model.emit('save', self, res);
Expand All @@ -206,8 +209,9 @@ exports.update = function(fn){
.put(url)
.set(this.model._headers)
.send(self)
.end(function(res){
if (res.error) return fn(error(res), res);
.end(function(err,res){
if (err || res.error)
return fn(self._toError(err, res), res);
self.dirty = {};
self.model.emit('save', self, res);
self.emit('save');
Expand Down Expand Up @@ -288,13 +292,31 @@ exports.toJSON = function(){
};

/**
* Response error helper.
* Register a custom error handler for the model instance.
*
* @param {Function} fn
* @return {Object} self
* @api public
*/

exports.toError = function(fn) {
if (fn) {
this._toError = fn;
}

return this;
}

/**
* Standard error handler.
*
* @param {Response} er
* @param {Error} err
* @param {Response} res
* @return {Error}
* @api private
*/

function error(res) {
return new Error('got ' + res.status + ' response');
exports._toError = function(err, res) {
if (err) return err;
else return new Error('got ' + res.status + ' response');
}
47 changes: 35 additions & 12 deletions lib/static.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
* Module dependencies.
*/

var Collection = require('collection');
try {
var Collection = require('collection');
} catch (e) {
var Collection = require('component-collection');
}
var request = require('superagent');
var noop = function(){};

Expand Down Expand Up @@ -146,8 +150,9 @@ exports.destroyAll = function(fn){
this.request
.del(url)
.set(this._headers)
.end(function(res){
if (res.error) return fn(error(res), null, res);
.end(function(err, res){
if (err || res.error)
return fn(self._toError(err,res), null, res);
fn(null, [], res);
});
};
Expand All @@ -165,8 +170,9 @@ exports.all = function(fn){
this.request
.get(url)
.set(this._headers)
.end(function(res){
if (res.error) return fn(error(res), null, res);
.end(function(err, res){
if (err || res.error)
return fn(self._toError(err,res), null, res);
var col = new Collection;
for (var i = 0, len = res.body.length; i < len; ++i) {
col.push(new self(res.body[i]));
Expand All @@ -189,21 +195,38 @@ exports.get = function(id, fn){
this.request
.get(url)
.set(this._headers)
.end(function(res){
if (res.error) return fn(error(res), null, res);
.end(function(err, res){
if (err || res.error)
return fn(self._toError(err,res), null, res);
var model = new self(res.body);
fn(null, model, res);
});
};

/**
* Response error helper.
* Register a custom error handler for the model class and its instances.
*
* @param {Response} er
* @param {Function} fn
* @return {Object} self
* @api public
*/
exports.toError = function(fn) {
if (fn) {
this._toError = this.prototype._toError = fn;
}

return this;
}

/**
* Standard error handler.
*
* @param {Error} err
* @param {Response} res
* @return {Error}
* @api private
*/

function error(res) {
return new Error('got ' + res.status + ' response');
exports._toError = function(err, res) {
if (err) return err;
else return new Error('got ' + res.status + ' response');
}
7 changes: 6 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@
"main": "lib/index.js",
"dependencies": {
"component-emitter": "~1.1.3",
"collection": "git://github.com/component/collection#0.0.2",
"component-collection": "~0.0.2",
"component-each": "~0.2.5",
"superagent": "~0.21.0"
},
"browser": {
"emitter": "component-emitter",
"collection": "component-collection",
"each": "component-each"
},
"devDependencies": {
"express": "3.0.0"
}
Expand Down
60 changes: 60 additions & 0 deletions test/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,3 +441,63 @@ describe('Model#isValid()', function(){
assert(0 == user.errors.length);
})
})

describe('Model#toError()', function() {
function getErrorHandler(static) {
return function(err, res) {
return {error: err, response: res, static: static};
};
}

it('should be called on errors', function(done) {
var Car = model('Car')
.attr('id')
.attr('color');

var car = new Car({color: 'blue'});

car.toError(getErrorHandler(false));

car.save(function(err) {
assert(null == err.error);
assert(500 == err.response.status);
assert(false == err.static);

done();
});
});

it('should overwrite the static error handler', function(done) {
var Car = model('Car')
.attr('id')
.attr('color')
.toError(getErrorHandler(true));

var car = new Car({color: 'red'});

car.toError(getErrorHandler(false));

car.save(function(err) {
assert(null == err.error);
assert(500 == err.response.status);
assert(false == err.static);

done();
});
});

it('should use the default error handler if not called', function(done) {
var Car = model('Car')
.attr('id')
.attr('color');

var car = new Car({color: 'yellow'});

car.save(function(err) {
assert(err instanceof Error);
assert('got 500 response' == err.message);

done();
});
});
});
16 changes: 16 additions & 0 deletions test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,21 @@ app.post('/users', function(req, res){
res.send({ id: id });
});

app.get('/cars', function(req, res) {
setTimeout(function() {
res.send([]);
}, 500);
});

app.get('/cars/:id', function(req, res) {
setTimeout(function() {
res.send({id: req.params.id, color: 'silver'});
}, 500);
});

app.post('/cars', function(req, res) {
res.send(500);
});

app.listen(4000);
console.log('test server listening on port 4000');
56 changes: 56 additions & 0 deletions test/statics.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,59 @@ describe('Model.route(string)', function(){
assert('/api/u/edit' == User.url('edit'));
})
})

describe('Model.toError()', function() {
var Car = model('Car')
.attr('id')
.attr('color')
.toError(function(err, res) {
return {error: err, response: res};
});

var _request_get = Car.request.get;

before( function(done) {
Car.request.get = function(url, data, fn) {
var req = _request_get(url, data, fn);
req.timeout(1);
return req;
};
done();
});

after( function(done) {
Car.request.get = _request_get;
done();
});


it('should be called on errors (e.g. request timeout)', function(done) {
Car.all(function(err, res) {
assert(err.error instanceof Error);
assert(-1 != err.error.message.indexOf('timeout'));
done();
});
});

it('should be inherited by model instances', function(done) {
var car = new Car({color: 'silver'});

car.save(function(err) {
assert(500 == err.response.status);
done();
});
});

it('should use the default error handler if not called', function(done) {
var Car = model('Car')
.attr('id')
.attr('color');

Car.all(function(err) {
assert(err instanceof Error);
assert(-1 != err.message.indexOf('timeout'));

done();
});
});
});