Skip to content

Commit c6151d2

Browse files
committed
Implement retry for likely transient errors
1 parent 95962e5 commit c6151d2

File tree

3 files changed

+130
-11
lines changed

3 files changed

+130
-11
lines changed

src/archivist/errors.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
export class InaccessibleContentError extends Error {
2-
constructor(reasonOrReasons) {
3-
const reasons = [].concat(reasonOrReasons);
2+
constructor(errorOrErrors) {
3+
const errors = [].concat(errorOrErrors);
4+
const reasons = errors.map(error => (error instanceof Error ? error.message : error));
45

56
super(`The documents cannot be accessed or their contents can not be selected:${`\n - ${reasons.join('\n - ')}`}`);
67
this.name = 'InaccessibleContentError';
78
this.reasons = reasons;
9+
this.errors = errors;
810
}
911
}

src/archivist/index.js

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export const EVENTS = [
2929
'trackingCompleted',
3030
'inaccessibleContent',
3131
'info',
32+
'warn',
3233
'error',
3334
'pluginError',
3435
];
@@ -76,15 +77,35 @@ export default class Archivist extends events.EventEmitter {
7677

7778
initQueue() {
7879
this.trackingQueue = async.queue(this.trackTermsChanges.bind(this), MAX_PARALLEL_TRACKING);
79-
this.trackingQueue.error((error, { terms }) => {
80-
if (error instanceof InaccessibleContentError) {
81-
this.emit('inaccessibleContent', error, terms);
80+
this.trackingQueue.error(this.handleTrackingError.bind(this));
81+
}
8282

83-
return;
84-
}
83+
handleTrackingError(error, { terms, isRetry }) {
84+
if (!(error instanceof InaccessibleContentError)) {
85+
this.emit('error', {
86+
message: error.stack,
87+
serviceId: terms.service.id,
88+
termsType: terms.type,
89+
});
8590

86-
this.emit('error', error, terms);
87-
});
91+
return;
92+
}
93+
94+
const isErrorLikelyTransient = error.errors.some(err => err instanceof FetchDocumentError && err.mayBeTransient);
95+
96+
if (isErrorLikelyTransient && !isRetry) {
97+
this.emit('warn', {
98+
message: `The documents cannot be accessed due to the following likely transient errors:\n- ${error.errors.map(err => err.message).join('\n- ')}\nRetry later…`,
99+
serviceId: terms.service.id,
100+
termsType: terms.type,
101+
});
102+
103+
this.trackingQueue.push({ terms, isRetry: true });
104+
105+
return;
106+
}
107+
108+
this.emit('inaccessibleContent', error, terms);
88109
}
89110

90111
attach(listener) {
@@ -171,7 +192,7 @@ export default class Archivist extends events.EventEmitter {
171192
throw error;
172193
}
173194

174-
fetchDocumentErrors.push(error.message);
195+
fetchDocumentErrors.push(error);
175196
}
176197
}));
177198

@@ -206,7 +227,7 @@ export default class Archivist extends events.EventEmitter {
206227
throw error;
207228
}
208229

209-
extractDocumentErrors.push(error.message);
230+
extractDocumentErrors.push(error);
210231
}
211232
}));
212233

src/archivist/index.test.js

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import nock from 'nock';
88
import sinon from 'sinon';
99
import sinonChai from 'sinon-chai';
1010

11+
import { InaccessibleContentError } from './errors.js';
12+
import { FetchDocumentError } from './fetcher/index.js';
1113
import Git from './recorder/repositories/git/git.js';
1214

1315
import Archivist, { EVENTS } from './index.js';
@@ -245,6 +247,100 @@ describe('Archivist', function () {
245247
});
246248
});
247249

250+
describe('#handleTrackingError', () => {
251+
let errorSpy;
252+
let warnSpy;
253+
let inaccessibleContentSpy;
254+
let pushSpy;
255+
let terms;
256+
let app;
257+
const retryableError = new FetchDocumentError(FetchDocumentError.LIKELY_TRANSIENT_ERRORS[0]);
258+
259+
before(async () => {
260+
app = new Archivist({
261+
recorderConfig: config.get('@opentermsarchive/engine.recorder'),
262+
fetcherConfig: config.get('@opentermsarchive/engine.fetcher'),
263+
});
264+
await app.initialize();
265+
});
266+
267+
beforeEach(() => {
268+
errorSpy = sinon.spy();
269+
warnSpy = sinon.spy();
270+
inaccessibleContentSpy = sinon.spy();
271+
pushSpy = sinon.spy(app.trackingQueue, 'push');
272+
app.on('error', errorSpy);
273+
app.on('warn', warnSpy);
274+
app.on('inaccessibleContent', inaccessibleContentSpy);
275+
276+
terms = {
277+
service: { id: 'test-service' },
278+
type: 'test-type',
279+
sourceDocuments: [
280+
{ location: 'https://example.com/doc1' },
281+
{ location: 'https://example.com/doc2' },
282+
],
283+
};
284+
});
285+
286+
afterEach(() => {
287+
errorSpy.resetHistory();
288+
warnSpy.resetHistory();
289+
inaccessibleContentSpy.resetHistory();
290+
pushSpy.restore();
291+
});
292+
293+
context('with an InaccessibleContentError', () => {
294+
context('when error may be transient', () => {
295+
beforeEach(() => {
296+
const error = new InaccessibleContentError([retryableError]);
297+
298+
app.handleTrackingError(error, { terms });
299+
});
300+
301+
it('does not emit an error event', () => {
302+
expect(errorSpy).to.not.have.been.called;
303+
});
304+
305+
it('does not emit an inaccessibleContent event', () => {
306+
expect(inaccessibleContentSpy).to.not.have.been.called;
307+
});
308+
309+
it('emits a warning', () => {
310+
expect(warnSpy).to.have.been.called;
311+
});
312+
313+
it('pushes terms to tracking queue for retry', () => {
314+
expect(pushSpy).to.have.been.calledWith({ terms, isRetry: true });
315+
});
316+
});
317+
318+
context('when error comes from a retry', () => {
319+
beforeEach(() => {
320+
const error = new InaccessibleContentError([retryableError]);
321+
322+
app.handleTrackingError(error, { terms, isRetry: true });
323+
});
324+
325+
it('does not emit an error event', () => {
326+
expect(errorSpy).to.not.have.been.called;
327+
});
328+
329+
it('does not emit a warning', () => {
330+
expect(warnSpy).to.not.have.been.called;
331+
});
332+
333+
it('emits an inaccessibleContent event with error and terms', () => {
334+
expect(inaccessibleContentSpy).to.have.been.called;
335+
});
336+
337+
it('does not push terms to tracking queue for retry', () => {
338+
expect(pushSpy).to.not.have.been.called;
339+
});
340+
});
341+
});
342+
});
343+
248344
describe('Plugin system', () => {
249345
const plugin = {};
250346

0 commit comments

Comments
 (0)