Skip to content

Commit

Permalink
Merge pull request #874 from Netflix/UIPLATFORM-459
Browse files Browse the repository at this point in the history
Include missing paths in MaxRetryExceededError instances
  • Loading branch information
asyncanup authored Sep 11, 2017
2 parents 596ca02 + d86f2ee commit 9d12c0f
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 28 deletions.
3 changes: 2 additions & 1 deletion lib/errors/MaxRetryExceededError.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ var NAME = "MaxRetryExceededError";
*
* @private
*/
function MaxRetryExceededError() {
function MaxRetryExceededError(missingOptimizedPaths) {
this.message = "The allowed number of retries have been exceeded.";
this.missingOptimizedPaths = missingOptimizedPaths || [];
this.stack = (new Error()).stack;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/response/get/getRequestCycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module.exports = function getRequestCycle(getResponse, model, results, observer,
errors, count) {
// we have exceeded the maximum retry limit.
if (count === model._maxRetries) {
observer.onError(new MaxRetryExceededError());
observer.onError(new MaxRetryExceededError(results.optimizedMissingPaths));
return {
dispose: function() {}
};
Expand Down
9 changes: 5 additions & 4 deletions lib/response/set/setRequestCycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@ var MaxRetryExceededError = require("./../../errors/MaxRetryExceededError");
*/
module.exports = function setRequestCycle(model, observer, groups,
isJSONGraph, isProgressive, count) {
var requestedAndOptimizedPaths = setGroupsIntoCache(model, groups);
var optimizedPaths = requestedAndOptimizedPaths.optimizedPaths;
var requestedPaths = requestedAndOptimizedPaths.requestedPaths;

// we have exceeded the maximum retry limit.
if (count === model._maxRetries) {
observer.onError(new MaxRetryExceededError());
observer.onError(new MaxRetryExceededError(optimizedPaths));
return {
dispose: function() {}
};
}

var requestedAndOptimizedPaths = setGroupsIntoCache(model, groups);
var optimizedPaths = requestedAndOptimizedPaths.optimizedPaths;
var requestedPaths = requestedAndOptimizedPaths.requestedPaths;
var isMaster = model._source === undefined;

// Local set only. We perform a follow up get. If performance is ever
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"license": "Apache-2.0",
"scripts": {
"test": "gulp test-coverage",
"test:only": "mocha test/index.js",
"dist": "gulp all",
"doc": "gulp doc",
"perf": "gulp perf-run",
Expand Down
76 changes: 54 additions & 22 deletions test/falcor/get/get.dataSource-only.spec.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
var falcor = require("./../../../lib/");
var falcor = require('./../../../lib/');
var Model = falcor.Model;
var Rx = require('rx');
var noOp = function() {};
var LocalDataSource = require('../../data/LocalDataSource');
var ErrorDataSource = require('../../data/ErrorDataSource');
var asyncifyDataSource = require('../../data/asyncifyDataSource')
var isPathValue = require("./../../../lib/support/isPathValue");
var expect = require("chai").expect;
var isPathValue = require('./../../../lib/support/isPathValue');
var expect = require('chai').expect;
var sinon = require('sinon');
var cacheGenerator = require('./../../CacheGenerator');
var atom = require('falcor-json-graph').atom;
var MaxRetryExceededError = require('./../../../lib/errors/MaxRetryExceededError');
var strip = require('./../../cleanData').stripDerefAndVersionKeys;
var isAssertionError = require('./../../isAssertionError');


describe('DataSource Only', function() {
Expand Down Expand Up @@ -90,13 +91,13 @@ describe('DataSource Only', function() {
it('should get a directly referenced value from falcor.', function(done) {
var cache = {
reference: {
$type: "ref",
value: ["foo", "bar"]
$type: 'ref',
value: ['foo', 'bar']
},
foo: {
bar: {
$type: "atom",
value: "value"
$type: 'atom',
value: 'value'
}
}
};
Expand All @@ -106,7 +107,7 @@ describe('DataSource Only', function() {
get(['reference', null])).
doAction(onNext, noOp, function() {
expect(strip(onNext.getCall(0).args[0])).to.deep.equals({
json: {reference: "value"}
json: {reference: 'value'}
});
}).
subscribe(noOp, done, done);
Expand Down Expand Up @@ -171,7 +172,7 @@ describe('DataSource Only', function() {
doAction(noOp, function(err) {
outputError = err;
expect(err).to.deep.equals({
$type: "error",
$type: 'error',
value: {
message: 'Oops!',
status: 500
Expand All @@ -190,31 +191,31 @@ describe('DataSource Only', function() {
}
});
});
it("should get all missing paths in a single request", function(done) {
it('should get all missing paths in a single request', function(done) {
var serviceCalls = 0;
var cacheModel = new Model({
cache: {
lolomo: {
summary: {
$type: "atom",
value: "hello"
$type: 'atom',
value: 'hello'
},
0: {
summary: {
$type: "atom",
value: "hello-0"
$type: 'atom',
value: 'hello-0'
}
},
1: {
summary: {
$type: "atom",
value: "hello-1"
$type: 'atom',
value: 'hello-1'
}
},
2: {
summary: {
$type: "atom",
value: "hello-2"
$type: 'atom',
value: 'hello-2'
}
}
}
Expand All @@ -230,7 +231,7 @@ describe('DataSource Only', function() {

var onNext = sinon.spy();
toObservable(model.
get("lolomo.summary", "lolomo[0..2].summary")).
get('lolomo.summary', 'lolomo[0..2].summary')).
doAction(onNext, noOp, function() {
var data = onNext.getCall(0).args[0];
var json = data.json;
Expand Down Expand Up @@ -341,8 +342,39 @@ describe('DataSource Only', function() {
return done();
}, done.bind('should not complete'));
});

it('should return missing optimized paths with MaxRetryExceededError', function(done) {
var model = new Model({
source: asyncifyDataSource(new LocalDataSource({})),
cache: {
lolomo: {
0: {
$type: 'ref',
value: ['videos', 1]
}
},
videos: {
0: {
title: 'Revolutionary Road'
}
}
}
});
toObservable(model.
get(['lolomo', 0, 'title'], 'videos[0].title', 'hall[0].ween')).
doAction(noOp, function(e) {
expect(MaxRetryExceededError.is(e), 'MaxRetryExceededError expected.').to.be.ok;
expect(e.missingOptimizedPaths).to.deep.equals([
['videos', 1, 'title'],
['hall', 0, 'ween']
]);
}).
subscribe(noOp, function(e) {
if (isAssertionError(e)) {
return done(e);
}
return done();
}, done.bind(null, new Error('should not complete')));
});
});

function isAssertionError(e) {
return e.hasOwnProperty('expected') && e.hasOwnProperty('actual');
}
44 changes: 44 additions & 0 deletions test/falcor/set/set.dataSource-only.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ var sinon = require('sinon');
var LocalDataSource = require('./../../data/LocalDataSource');
var Cache = require('./../../data/Cache');
var strip = require('./../../cleanData').stripDerefAndVersionKeys;
var MaxRetryExceededError = require('./../../../lib/errors/MaxRetryExceededError');
var isAssertionError = require('./../../isAssertionError');

describe('DataSource.', function() {
it('should validate args are sent to the dataSource collapsed.', function(done) {
Expand Down Expand Up @@ -194,4 +196,46 @@ describe('DataSource.', function() {
}).
subscribe(noOp, done, done);
});

it('should return missing optimized paths with a MaxRetryExceededError.', function(done) {
var onSet = function(source, tmpGraph, jsonGraphFromSet) {
model.invalidate('videos[1234].title');
return {
jsonGraph: {
videos: {
1234: {}
}
},
paths: []
};
};
var dataSource = new LocalDataSource(Cache(), {
onSet: onSet
});
var model = new Model({
source: dataSource
});

toObservable(model.
set({
json: {
videos: {
1234: {
title: 'Nowhere to be found'
}
}
}
})).
doAction(noOp, function(e) {
expect(MaxRetryExceededError.is(err), 'MaxRetryExceededError expected').to.be.ok;
expect(err.missingOptimizedPaths).to.deep.equal([['videos', '1234', 'title']]);
}).
subscribe(noOp, function(e) {
if (isAssertionError(e)) {
return done(e);
}
return done();
}, done.bind(null, new Error('should not complete')));
});
});

0 comments on commit 9d12c0f

Please sign in to comment.