Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

when session.save error should return error #755

Merged
merged 1 commit into from

5 participants

@coolme200

No description provided.

@fengmk2

Yep, we need to handle session.save() error.

@jifeng

+1

@tj tj merged commit 1da7800 into from
@tj
Owner
tj commented

sorry had to revert this, we can't next() there since it's deferred, a response is already mid-completion

@fengmk2

We should log the error?

@tj
Owner
tj commented

yeah that's about all we can do here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 26, 2013
  1. when session.save error should return error

    tangyao authored
    1
This page is out of date. Refresh to see the latest.
Showing with 40 additions and 1 deletion.
  1. +5 −1 lib/middleware/session.js
  2. +35 −0 test/session.js
View
6 lib/middleware/session.js
@@ -277,7 +277,11 @@ function session(options){
if (!req.session) return res.end(data, encoding);
debug('saving');
req.session.resetMaxAge();
- req.session.save(function(){
+ req.session.save(function(err){
+ if (err) {
+ next(err);
+ return;
+ }
debug('saved');
res.end(data, encoding);
});
View
35 test/session.js
@@ -540,6 +540,41 @@ describe('connect.session()', function(){
});
});
})
+
+ describe('when session.save error', function () {
+
+ var session = require('../lib/middleware/session');
+
+ var old = session.Session.prototype.save;
+
+ before(function () {
+ session.Session.prototype.save = function (fn) {
+ fn && fn(new Error('Mock save error.'));
+ };
+ });
+
+ after(function () {
+ session.Session.prototype.save = old;
+ });
+ it('should return error', function (done) {
+ var app = connect()
+ .use(connect.cookieParser('keyboard cat'))
+ .use(connect.session())
+ .use(function(req, res, next){
+ req.session.count = req.session.count || 0;
+ req.session.count++;
+ res.end(req.session.count.toString());
+ });
+
+ app.request()
+ .get('/')
+ .end(function(res){
+ res.body.toString().should.include('Mock save error.');
+ done();
+ });
+ });
+ });
+
})
})
Something went wrong with that request. Please try again.