Commit 13eab232 authored by Robert Knight's avatar Robert Knight Committed by Nick Stenning

Stop fetching result pages if current page is empty (#119)

Yesterday a brief issue occurred where the server reported an incorrect
(too high) total number of results for a URL due to the search index
being out of date. Once the client reached the end of the actually
available results, it made queries to the search endpoint which returned
an empty result set but had a `total` figure implying that there should
be more pages. The client then ended up polling the server indefinitely
for more results.

This commit defensively makes the client stop fetching more result pages
if the current page is empty.
parent 42a61c6f
...@@ -49,8 +49,14 @@ SearchClient.prototype._getBatch = function (query, offset) { ...@@ -49,8 +49,14 @@ SearchClient.prototype._getBatch = function (query, offset) {
self._results = self._results.concat(chunk); self._results = self._results.concat(chunk);
} }
// Check if there are additional pages of results to fetch. In addition to
// checking the `total` figure from the server, we also require that at
// least one result was returned in the current page, otherwise we would
// end up repeating the same query for the next page. If the server's
// `total` count is incorrect for any reason, that will lead to the client
// polling the server indefinitely.
var nextOffset = offset + results.rows.length; var nextOffset = offset + results.rows.length;
if (results.total > nextOffset) { if (results.total > nextOffset && chunk.length > 0) {
self._getBatch(query, nextOffset); self._getBatch(query, nextOffset);
} else { } else {
if (!self._incremental) { if (!self._incremental) {
......
...@@ -49,6 +49,30 @@ describe('SearchClient', function () { ...@@ -49,6 +49,30 @@ describe('SearchClient', function () {
}); });
}); });
it('stops fetching chunks if the results array is empty', function () {
// Simulate a situation where the `total` count for the server is incorrect
// and we appear to have reached the end of the result list even though
// `total` implies that there should be more results available.
//
// In that case the client should stop trying to fetch additional pages.
fakeSearchFn = sinon.spy(function () {
return Promise.resolve({
rows: [],
total: 1000,
});
});
var client = new SearchClient(fakeSearchFn, {chunkSize: 2});
var onResults = sinon.stub();
client.on('results', onResults);
client.get({uri: 'http://example.com'});
return await(client, 'end').then(function () {
assert.calledWith(onResults, []);
assert.calledOnce(fakeSearchFn);
});
});
it('emits "results" once in non-incremental mode', function () { it('emits "results" once in non-incremental mode', function () {
var client = new SearchClient(fakeSearchFn, var client = new SearchClient(fakeSearchFn,
{chunkSize: 2, incremental: false}); {chunkSize: 2, incremental: false});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment