Skip to content

Commit

Permalink
Merge pull request #31 from eschwartz/fix-handle-fetcher-sync-errors
Browse files Browse the repository at this point in the history 
Fix handle fetcher sync errors
  • Loading branch information
@four43
four43 authored Oct 25, 2016
2 parents a1d1eef + 7efa85d commit c8c24bb
Show file tree
Hide file tree
Showing 3 changed files with 318 additions and 16 deletions.
120 changes: 120 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,126 @@ crispCacheBasic.set("testC", "The Value C", {size: 5}, callback);
```
Will result in the cache containing just the `testC` entry. The `testA` entry was added, then the `testB` entry. These are both held in cache because their sizes meet the `maxSize` of `10` but don't exceed it yet. When `testC` is added however, the cache finds that `testA` is the oldest and removes it. Seeing that the cache is still too large (`testC`'s 5 + `testB`'s 8 > our `maxSize` of 10) it removes `testB` too, leaving us with just `testC` in the cache.

### Error Handling

CrispCache handles errors returned by the fetcher differently, depending on the state of your cache. The intent of this behavior to smooth out hiccups in flaky asynchronous services, using a valid cached value whenever possible.

While your cache is **empty**, fetcher errors will be **propagated**:

```js
var cache = new CrispCache({
fetcher: function(key, cb) {
cb(new Error());
}
});

cache.get('key', function(err, val) {
// err!
});
```

While your cache is **active** or **stale**, fetcher errors will be **ignored**, and the last available value will be used:

```js
var i = 0;
var cache = new CrispCache({
fetcher: function(key, cb) {
// Return a value, on the first request
if (i === 0) {
i++;
return cb(null, 'first value');
}
// Throw errors, after the first request
cb(null, new Error());
},
defaultStaleTtl: 1000 * 60,
defaultExpiresTtl: 1000 * 60 * 5,
});

cache.get('key', function(err, val) {
// val === 'first value';
});

//...anytime within the next 5 minutes...
cache.get('key', function(err, val) {
// val === 'first value'
});
```

While your cache is **expired**, fetcher errors will be **propagated**:

```js
var i = 0;
var cache = new CrispCache({
fetcher: function(key, cb) {
// Return a value, on the first request
if (i === 0) {
i++;
return cb(null, 'first value');
}
// Throw errors, after the first request
cb(null, new Error());
},
defaultStaleTtl: 1000 * 60,
defaultExpiresTtl: 1000 * 60 * 5,
});

cache.get('key', function(err, val) {
// val === 'first value';
});

// ...5 minutes later...
cache.get('key', function(err, val) {
// err!
});
```

### Caching errors

If you want *all* errors to be propagated, you could wrap CrispCache to cache errors like so:

```
function asyncFn(key, cb) {
// ...
}
var cache = new CrispCache({
fetcher: function(key, cb) {
asyncFn(key, function(err, val) {
// Cache the error, as though it were a value
if (err) {
return cb(null, err);
}
cb(null, val);
});
},
getOptions: function(val) {
// Only cache errors for 30 seconds
if (val instanceof Error) {
return {
expiresTtl: 1000 * 30
}
}
// Cache regular values for 5 minutes
return {
expiresTtl: 1000 * 60 * 5
};
}
});
function cachedAsyncFn(key, cb) {
cache.get(key, function(err, val) {
// Return error-type values as errors
if (val instanceof Error) {
return cb(val);
}
cb(err, val);
});
}
```

## Roadmap

* Add different caching backends (memory is the only one supported now)
9 changes: 8 additions & 1 deletion main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@ function CrispCache(options) {
if (!options.fetcher) {
throw new Error("Must pass a fetcher option, a fetcher is a function(key, callback) that can retrieve a key from a repository");
}
this.fetcher = options.fetcher;
// Wrap the fetcher with error handling,
// to catch synchronous errors
this.fetcher = function(key, cb) {
try {
return options.fetcher(key, cb);
}
catch (err) { cb(err); }
};

// Stale Control
this.defaultStaleTtl = options.defaultStaleTtl;
Expand Down
205 changes: 190 additions & 15 deletions test/crisp-cache-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ function fetcher(key, callback) {
return callback(null, data[key]);
}, 1);
}
function fetcherBad(key, callback) {
callback(new Error("There was a problem with the fetcher"));
}

describe("CrispCache", function () {
describe("Setup Sanity", function () {
Expand Down Expand Up @@ -140,6 +137,196 @@ describe("CrispCache", function () {
});
clock.tick(10);
});

it("Should propagate fetcher errors, if the cache is empty", function(done) {
var cache = new CrispCache({
fetcher: function(key, cb) {
cb(new Error('fetcher error'));
}
});

cache.get('key', function(err, value) {
try {
assert(err, 'cache should throw an error');
assert.strictEqual(err.message, 'fetcher error', 'error message');
done();
}
catch (err) { done(err); }
});
});

it("Should propagate fetcher errors (thrown), if the cache is empty", function(done) {
clock = sinon.useFakeTimers();
var cache = new CrispCache({
fetcher: function() {
throw new Error('fetcher error')
}
});

cache.get('key', function(err, value) {
try {
assert(err, 'cache should throw an error');
assert.strictEqual(err.message, 'fetcher error', 'error message');
done();
}
catch (err) { done(err); }
})
});

it("Should propagate fetcher errors, if the cache is expired", function(done) {
clock = sinon.useFakeTimers();
var fetcher = function(key, cb) {
cb(null, 'fetcher init value');
}
var cache = new CrispCache({
fetcher: function(key, cb) {
fetcher(key, cb);
},
defaultExpiresTtl: 100
});

// Grab the initial value
cache.get('key', function(err, value) {
try {
assert.strictEqual(value, 'fetcher init value');

// Expire the cache, then have the fetcher return an error
clock.tick(101);
fetcher = function(key, cb) {
cb(new Error('fetcher error'));
}

cache.get('key', function(err, value) {
try {
assert.strictEqual(err && err.message, 'fetcher error');
done();
}
catch (err) { done(err); }
});
}
catch (err) { done(err); }
});
});

it("Should propagate fetcher errors (thrown), if the cache is expired", function(done) {
clock = sinon.useFakeTimers();
var fetcher = function(key, cb) {
cb(null, 'fetcher init value');
}
var cache = new CrispCache({
fetcher: function(key, cb) {
fetcher(key, cb);
},
defaultExpiresTtl: 100
});

// Grab the initial value
cache.get('key', function(err, value) {
try {
assert.strictEqual(value, 'fetcher init value');

// Expire the cache, then have the fetcher throw an error
clock.tick(101);
fetcher = function(key, cb) {
throw new Error('fetcher error');
}

cache.get('key', function(err, value) {
try {
assert.strictEqual(err && err.message, 'fetcher error');
done();
}
catch (err) {
done(err);
}
});
}
catch (err) {
done(err);
}
});
});

it("Should ignore fetcher errors, if the cache is stale", function (done) {
clock = sinon.useFakeTimers();
var fetcher = function(key, cb) {
cb(null, 'fetcher init value');
}
var cache = new CrispCache({
fetcher: function(key, cb) {
fetcher(key, cb);
},
defaultStaleTtl: 100,
defaultExpiresTtl: 500
});

// Grab the initial value
cache.get('key', function(err, value) {
try {
assert.strictEqual(value, 'fetcher init value');

// Expire the cache, then have the fetcher return an error
clock.tick(101);
fetcher = function(key, cb) {
cb(new Error('fetcher error'));
}

cache.get('key', function(err, value) {
try {
assert.strictEqual(value, 'fetcher init value', 'should return last valid value');
done();
}
catch (err) {
done(err);
}
});
}
catch (err) {
done(err);
}
});
});

it("Should ignore fetcher errors (thrown), if the cache is stale", function (done) {
clock = sinon.useFakeTimers();
var fetcher = function(key, cb) {
cb(null, 'fetcher init value');
}
var cache = new CrispCache({
fetcher: function(key, cb) {
fetcher(key, cb);
},
defaultStaleTtl: 100,
defaultExpiresTtl: 500
});

// Grab the initial value
cache.get('key', function(err, value) {
try {
assert.strictEqual(value, 'fetcher init value');

// Expire the cache, then have the fetcher return an error
clock.tick(101);
fetcher = function(key, cb) {
throw new Error('fetcher error');
}

cache.get('key', function(err, value) {
try {
assert.strictEqual(value, 'fetcher init value', 'should return last valid value');
done();
}
catch (err) {
done(err);
}
});
}
catch (err) {
done(err);
}
});
});

});

describe("Get - Advanced", function () {
Expand Down Expand Up @@ -241,18 +428,6 @@ describe("CrispCache", function () {
clock.tick(10);
});

it("Should propagate the error from the fetcher", function (done) {
crispCacheBasic = new CrispCache({
fetcher: fetcherBad
});
crispCacheBasic.get('hello', function (err, value) {
assert.ok(err);
assert.equal(err.message, "There was a problem with the fetcher");
assert.equal(value, undefined);
done();
});
});

it("Should assign varying staleTTLs based on variance", function (done) {
seed('foo', {global: true});
crispCacheBasic = new CrispCache({
Expand Down

0 comments on commit c8c24bb

Please sign in to comment.