diff --git a/services/project-history/app/js/ErrorRecorder.js b/services/project-history/app/js/ErrorRecorder.js index 304b72921d..fbce06081f 100644 --- a/services/project-history/app/js/ErrorRecorder.js +++ b/services/project-history/app/js/ErrorRecorder.js @@ -10,6 +10,18 @@ import { db } from './mongodb.js' * @import { ProjectHistoryFailure } from './mongo-types' */ +/** + * @template {{error?: string}} T + * @param {T} failure + * @return {T} + */ +function normalizeFailure(failure) { + return { + ...failure, + error: failure.error?.replace('OError:', 'Error:'), + } +} + /** * @param {string} projectId * @param {number} queueSize @@ -47,7 +59,7 @@ async function record(projectId, queueSize, error) { // Since we upsert, the result should always have a value throw new OError('no value returned when recording an error', { projectId }) } - return result.value + return normalizeFailure(result.value) } async function clearError(projectId) { @@ -107,7 +119,10 @@ async function cloneFailure(sourceProjectId, targetProjectId) { * @param projectId */ async function getFailureRecord(projectId) { - return await db.projectHistoryFailures.findOne({ project_id: projectId }) + const result = await db.projectHistoryFailures.findOne({ + project_id: projectId, + }) + return result && normalizeFailure(result) } async function getLastFailure(projectId) { @@ -116,15 +131,18 @@ async function getLastFailure(projectId) { { $inc: { requestCount: 1 } }, // increment the request count every time we check the last failure { projection: { error: 1, ts: 1 } } ) - return result && result.value + return result?.value && normalizeFailure(result.value) } async function getFailedProjects() { - return await db.projectHistoryFailures.find({}).toArray() + return await db.projectHistoryFailures + .find({}) + .map(normalizeFailure) + .toArray() } async function getFailuresByType() { - const results = await db.projectHistoryFailures.find({}).toArray() + const results = await getFailedProjects() const failureCounts = {} const failureAttempts = {} const failureRequests = {} @@ -160,10 +178,6 @@ async function getFailures() { 'Error: bad response from filestore: 404': 'filestore-404', 'Error: bad response from filestore: 500': 'filestore-500', 'NotFoundError: got a 404 from web api': 'web-api-404', - 'OError: history store a non-success status code: 413': 'history-store-413', - 'OError: history store a non-success status code: 422': 'history-store-422', - 'OError: history store a non-success status code: 500': 'history-store-500', - 'OError: history store a non-success status code: 503': 'history-store-503', 'Error: history store a non-success status code: 413': 'history-store-413', 'Error: history store a non-success status code: 422': 'history-store-422', 'Error: history store a non-success status code: 500': 'history-store-500', diff --git a/services/project-history/app/js/RetryManager.js b/services/project-history/app/js/RetryManager.js index b146da29f9..c9c7ac58e4 100644 --- a/services/project-history/app/js/RetryManager.js +++ b/services/project-history/app/js/RetryManager.js @@ -20,7 +20,6 @@ const TEMPORARY_FAILURES = [ const HARD_FAILURES = [ 'Error: history store a non-success status code: 422', - 'OError: history store a non-success status code: 422', 'OpsOutOfOrderError: project structure version out of order', 'OpsOutOfOrderError: project structure version out of order on incoming updates', 'OpsOutOfOrderError: doc version out of order', diff --git a/services/project-history/test/acceptance/js/RetryTests.js b/services/project-history/test/acceptance/js/RetryTests.js index 5d788a1f19..3e5c5d66f8 100644 --- a/services/project-history/test/acceptance/js/RetryTests.js +++ b/services/project-history/test/acceptance/js/RetryTests.js @@ -1,3 +1,4 @@ +import _ from 'lodash' import nock from 'nock' import { expect } from 'chai' import { fetchStringWithResponse } from '@overleaf/fetch-utils' @@ -54,6 +55,37 @@ describe('Retrying failed projects', function () { nock.cleanAll() }) + describe('normalizeFailure', function () { + it('should normalize the error', async function () { + const baseLineFailures = await ProjectHistoryClient.getFailures() + + await ProjectHistoryClient.setFailures([ + { + project_id: new ObjectId(), + attempts: 1, + error: 'Error: ESOCKETTIMEDOUT', + }, + { + project_id: new ObjectId(), + attempts: 1, + error: 'OError: ESOCKETTIMEDOUT', + }, + ]) + + const body = await ProjectHistoryClient.getFailures() + expect(body).to.deep.equal( + _.merge(baseLineFailures, { + attempts: { + 'socket-timeout': 2, + }, + counts: { + 'socket-timeout': 2, + }, + }) + ) + }) + }) + describe('retrying project history', function () { describe('when there is a soft failure', function () { beforeEach(async function () { @@ -72,11 +104,13 @@ describe('Retrying failed projects', function () { } await ProjectHistoryClient.pushRawUpdate(this.project_id, update) - await ProjectHistoryClient.setFailure({ - project_id: this.project_id, - attempts: 1, - error: 'soft-error', - }) + await ProjectHistoryClient.setFailures([ + { + project_id: this.project_id, + attempts: 1, + error: 'soft-error', + }, + ]) }) it('flushes the project history queue', async function () { @@ -140,11 +174,13 @@ describe('Retrying failed projects', function () { }, }, }) - await ProjectHistoryClient.setFailure({ - project_id: this.project_id, - attempts: 100, - error: 'hard-error', - }) + await ProjectHistoryClient.setFailures([ + { + project_id: this.project_id, + attempts: 100, + error: 'hard-error', + }, + ]) }) it('calls web to resync the project', async function () { diff --git a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js index 6b9961f185..3fd3261a85 100644 --- a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js +++ b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js @@ -197,9 +197,9 @@ export async function deleteLabel(projectId, labelId) { expect(response.status).to.equal(204) } -export async function setFailure(failureEntry) { - await db.projectHistoryFailures.deleteOne({ project_id: { $exists: true } }) - return await db.projectHistoryFailures.insertOne(failureEntry) +export async function setFailures(failureEntries) { + await db.projectHistoryFailures.deleteMany({}) + return await db.projectHistoryFailures.insertMany(failureEntries) } export function getFailure(projectId, callback) { @@ -218,6 +218,11 @@ export async function getDump(projectId) { return await fetchJson(`http://127.0.0.1:3054/project/${projectId}/dump`) } +export async function getFailures() { + const { failures } = await fetchJson('http://127.0.0.1:3054/status/failures') + return failures +} + export async function deleteProject(projectId) { const response = await fetchNothing( `http://127.0.0.1:3054/project/${projectId}`,