Skip to content

Commit

Permalink
feat: env vars to configure user error logging (#557)
Browse files Browse the repository at this point in the history
  • Loading branch information
banders authored Jun 27, 2024
1 parent d56189a commit 279ceae
Show file tree
Hide file tree
Showing 13 changed files with 253 additions and 41 deletions.
9 changes: 9 additions & 0 deletions backend/src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ config.defaults({
reportUnlockDurationInDays: parseInt(
process.env.REPORT_UNLOCK_DURATION_IN_DAYS || '2',
),
userErrorLogging: {
isEnabled:
process.env.IS_USER_ERROR_LOGGING_ENABLED?.toUpperCase() == 'TRUE' ||
false,
deleteScheduleCronTime: process.env.DELETE_USER_ERRORS_CRON_CRONTIME,
numMonthsOfUserErrorsToKeep:
process.env.NUM_MONTHS_OF_USER_ERRORS_TO_KEEP || 6,
},
rateLimit: {
enabled: process.env.IS_RATE_LIMIT_ENABLED || false, // Disable if rate limiting is not required
windowMs: process.env.RATE_LIMIT_WINDOW_MS || 60000, // 1 minute
Expand Down Expand Up @@ -108,4 +116,5 @@ config.defaults({
emailRecipients: process.env.CHES_EMAIL_RECIPIENTS?.split(','), // comma separated email addresses
},
});

export { config };
83 changes: 83 additions & 0 deletions backend/src/schedulers/delete-user-errors-scheduler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import waitFor from 'wait-for-expect';
import deleteUserErrorsJob from './delete-user-errors-scheduler';

jest.mock('../v1/services/utils-service', () => ({
utils: {
delay: jest.fn(),
},
}));

const mockDeleteErrorsOlderThan = jest.fn();
jest.mock('../v1/services/error-service', () => ({
errorService: {
deleteErrorsOlderThan: (...args) => mockDeleteErrorsOlderThan(...args),
},
}));

const mock_asyncRetry = jest.fn((fn) => fn());
jest.mock('async-retry', () => ({
__esModule: true,
default: async (fn) => mock_asyncRetry(fn),
}));

const mock_generateHtmlEmail = jest.fn();
const mock_sendEmailWithRetry = jest.fn();

jest.mock('../external/services/ches', () => ({
__esModule: true,
default: {
generateHtmlEmail: (...args) => mock_generateHtmlEmail(...args),
sendEmailWithRetry: (...args) => mock_sendEmailWithRetry(...args),
},
}));

jest.mock('cron', () => ({
CronJob: class MockCron {
constructor(
public expression: string,
public cb: Function,
) {}
async start() {
return this.cb();
}
},
}));

jest.mock('../config', () => ({
config: {
get: (key: string) => {
const settings = {
'server:userErrorLogging:deleteScheduleCronTime': '121212121',
'server:userErrorLogging:numMonthsOfUserErrorsToKeep': 6,
'server:schedulerTimeZone': 'Canada/Vancouver',
'ches:enabled': true,
'ches:emailRecipients': '[email protected]',
};

return settings[key];
},
},
}));

const mock_tryLock = jest.fn();
const mock_unlock = jest.fn();
jest.mock('advisory-lock', () => ({
...jest.requireActual('advisory-lock'),
default: () => {
return () => ({
tryLock: () => mock_tryLock(),
});
},
}));

describe('delete-draft-service-scheduler', () => {
it('should delete draft reports', async () => {
mock_tryLock.mockReturnValue(mock_unlock);
deleteUserErrorsJob.start();
await waitFor(async () => {
expect(mock_tryLock).toHaveBeenCalledTimes(1);
expect(mockDeleteErrorsOlderThan).toHaveBeenCalled();
expect(mock_unlock).toHaveBeenCalledTimes(1);
});
});
});
33 changes: 33 additions & 0 deletions backend/src/schedulers/delete-user-errors-scheduler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { DateTimeFormatter, LocalDate, ZoneId } from '@js-joda/core';
import advisoryLock from 'advisory-lock';
import { config } from '../config';
import { logger as log } from '../logger';
import { errorService } from '../v1/services/error-service';
import { createJob } from './create-job';

const SCHEDULER_NAME = 'DeleteUserErrors';
const mutex = advisoryLock(config.get('server:databaseUrl'))(
`${SCHEDULER_NAME}-lock`,
);
const crontime = config.get('server:userErrorLogging:deleteScheduleCronTime');
const numMonthsOfErrorsToKeep = config.get(
'server:userErrorLogging:numMonthsOfUserErrorsToKeep',
);

export default createJob(
crontime,
async () => {
log.info(`Starting scheduled job '${SCHEDULER_NAME}'.`);
const thresholdDate = LocalDate.now(ZoneId.UTC)
.atStartOfDay(ZoneId.UTC)
.minusMonths(numMonthsOfErrorsToKeep)
.format(DateTimeFormatter.ISO_DATE_TIME);
await errorService.deleteErrorsOlderThan(thresholdDate);
log.info(`Completed scheduled job '${SCHEDULER_NAME}'.`);
},
mutex,
{
title: `Error in ${SCHEDULER_NAME}`,
message: `Error in scheduled job: ${SCHEDULER_NAME}`,
},
);
10 changes: 10 additions & 0 deletions backend/src/schedulers/run.all.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ jest.mock('./lock-reports-scheduler', () => ({
},
}));

const mockDeleteUserErrorsLock = jest.fn();
jest.mock('./delete-user-errors-scheduler', () => ({
__esModule: true,

default: {
start: () => mockDeleteUserErrorsLock(),
},
}));

const mockDeleteDraftReportLock = jest.fn();
jest.mock('./delete-draft-service-scheduler', () => ({
__esModule: true,
Expand All @@ -22,6 +31,7 @@ describe('run.all', () => {
it('should start all jobs', async () => {
run();
expect(mockDeleteDraftReportLock).toHaveBeenCalled();
expect(mockDeleteUserErrorsLock).toHaveBeenCalled();
expect(mockStartReportLock).toHaveBeenCalled();
});
});
6 changes: 4 additions & 2 deletions backend/src/schedulers/run.all.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { logger } from '../logger';
import deleteDraftReportsJob from './delete-draft-service-scheduler';
import deleteUserErrorsJob from './delete-user-errors-scheduler';
import lockReportsJob from './lock-reports-scheduler';

export const run = () => {
try {
deleteDraftReportsJob.start();
lockReportsJob.start();
deleteDraftReportsJob?.start();
deleteUserErrorsJob?.start();
lockReportsJob?.start();
} catch (error) {
/* istanbul ignore next */
logger.error(error);
Expand Down
103 changes: 67 additions & 36 deletions backend/src/v1/services/error-service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { DateTimeFormatter, ZoneId, ZonedDateTime } from '@js-joda/core';
import { config } from '../../config';
import prisma from '../prisma/prisma-client';
import prismaReadOnlyReplica from '../prisma/prisma-client-readonly-replica';
import { errorService } from './error-service';
import { SubmissionError } from './file-upload-service';
import { ValidationError, RowError } from './validate-service';
import { RowError, ValidationError } from './validate-service';

jest.mock('../prisma/prisma-client', () => {
return {
Expand Down Expand Up @@ -75,49 +76,79 @@ afterEach(() => {
jest.clearAllMocks();
});

const mockConfigLoggingDisbled = (key) => {
if (key == 'server:userErrorLogging:isEnabled') {
return false;
}
return undefined;
};
const mockConfigLoggingEnabled = (key) => {
if (key == 'server:userErrorLogging:isEnabled') {
return true;
}
return undefined;
};

describe('storeError', () => {
(
prismaReadOnlyReplica.pay_transparency_company.findFirst as jest.Mock
).mockResolvedValue(mockCompanyInDB);
(
prismaReadOnlyReplica.pay_transparency_user.findFirst as jest.Mock
).mockResolvedValue(mockUserInDB);

it('stores SubmissionError with ValidationError', async () => {
await errorService.storeError(
mockUserInfo,
new SubmissionError(mockValidationError),
);
const createMany = (prisma.user_error.createMany as jest.Mock).mock
.calls[0][0];
expect(createMany.data.map((x) => x.error)).toStrictEqual(mockArray);
});

it('stores SubmissionError with string', async () => {
const error = 'Something went wrong';
const subError = new SubmissionError(error);
await errorService.storeError(mockUserInfo, subError);
const createMany = (prisma.user_error.createMany as jest.Mock).mock
.calls[0][0];
expect(createMany.data.map((x) => x.error)).toStrictEqual([error]);
});

it('stores Error', async () => {
const error = 'test error';
const errorObj = new Error(error);
await errorService.storeError(mockUserInfo, errorObj);
const createMany = (prisma.user_error.createMany as jest.Mock).mock
.calls[0][0];
expect(createMany.data.map((x) => x.error)).toStrictEqual([error]);
describe('if error logging is disabled', () => {
it('', async () => {
jest.spyOn(config, 'get').mockImplementation(mockConfigLoggingDisbled);
await errorService.storeError(
mockUserInfo,
new SubmissionError(mockValidationError),
);
expect(prisma.user_error.createMany as jest.Mock).toHaveBeenCalledTimes(
0,
);
});
});

it('does not store anything if there are no errors', async () => {
await errorService.storeError(
mockUserInfo,
new SubmissionError(new ValidationError([], [], [])),
);

expect(prisma.user_error.createMany).toHaveBeenCalledTimes(0);
describe('if error logging is enabled', () => {
it('stores SubmissionError with ValidationError', async () => {
jest.spyOn(config, 'get').mockImplementation(mockConfigLoggingEnabled);
await errorService.storeError(
mockUserInfo,
new SubmissionError(mockValidationError),
);
const createMany = (prisma.user_error.createMany as jest.Mock).mock
.calls[0][0];
expect(createMany.data.map((x) => x.error)).toStrictEqual(mockArray);
});

it('stores SubmissionError with string', async () => {
jest.spyOn(config, 'get').mockImplementation(mockConfigLoggingEnabled);
const error = 'Something went wrong';
const subError = new SubmissionError(error);
await errorService.storeError(mockUserInfo, subError);
const createMany = (prisma.user_error.createMany as jest.Mock).mock
.calls[0][0];
expect(createMany.data.map((x) => x.error)).toStrictEqual([error]);
});

it('stores Error', async () => {
jest.spyOn(config, 'get').mockImplementation(mockConfigLoggingEnabled);
const error = 'test error';
const errorObj = new Error(error);
await errorService.storeError(mockUserInfo, errorObj);
const createMany = (prisma.user_error.createMany as jest.Mock).mock
.calls[0][0];
expect(createMany.data.map((x) => x.error)).toStrictEqual([error]);
});

it('does not store anything if there are no errors', async () => {
jest.spyOn(config, 'get').mockImplementation(mockConfigLoggingEnabled);
await errorService.storeError(
mockUserInfo,
new SubmissionError(new ValidationError([], [], [])),
);

expect(prisma.user_error.createMany).toHaveBeenCalledTimes(0);
});
});
});

Expand Down
32 changes: 30 additions & 2 deletions backend/src/v1/services/error-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ZonedDateTime,
convert,
} from '@js-joda/core';
import { config } from '../../config';
import prisma from '../prisma/prisma-client';
import prismaReadOnlyReplica from '../prisma/prisma-client-readonly-replica';
import { SubmissionError } from '../services/file-upload-service';
Expand All @@ -32,11 +33,17 @@ function ConvertStringToZonedISO(str: string): ZonedDateTime {

const errorService = {
/**
* Stores the errors in the database
* Stores the errors in the database. If the config setting
* 'server:userErrorLogging:isEnabled' is false then this method silently
* returns without storing any error.
* @param userInfo
* @param errors - supports classes Error, SubmissionError, ValidationError
*/
async storeError(userInfo, errors: Error) {
async storeError(userInfo, errors: Error): Promise<void> {
if (!config.get('server:userErrorLogging:isEnabled')) {
return;
}

let errorArr = [];
if (
errors instanceof SubmissionError &&
Expand Down Expand Up @@ -159,6 +166,27 @@ const errorService = {
records,
};
},

/**
Deletes errors that were logged before a given date
@param thresholdDate: an ISO 8601 datetime string identifying the date in
which errors older than this should be deleted. (default: 6 months ago)
Example usage:
const sixMonthsAgo = LocalDate.now(ZoneId.UTC)
.atStartOfDay(ZoneId.UTC)
.minusMonths(6)
.format(DateTimeFormatter.ISO_DATE_TIME;
errorService.deleteErrorsOlderThan(sixMonthsAgo);
*/
async deleteErrorsOlderThan(thresholdDate: string) {
await prisma.user_error.deleteMany({
where: {
create_date: {
lte: thresholdDate,
},
},
});
},
};

export { errorService };
3 changes: 3 additions & 0 deletions charts/fin-pay-transparency/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ data:
REPORT_EDIT_DURATION_IN_DAYS: {{ .Values.global.config.report_edit_duration_in_days | quote }}
REPORT_UNLOCK_DURATION_IN_DAYS: {{ .Values.global.config.report_unlock_duration_in_days | quote }}
DELETE_DRAFT_REPORT_CRON_CRONTIME: {{ .Values.global.config.delete_draft_report_cron_crontime | quote }}
DELETE_USER_ERRORS_CRON_CRONTIME: {{ .Values.global.config.delete_user_errors_cron_crontime | quote }}
LOCK_REPORT_CRON_CRONTIME: {{ .Values.global.config.lock_report_cron_crontime | quote }}
REPORTS_SCHEDULER_CRON_TIMEZONE: {{ .Values.global.config.reports_scheduler_cron_timezone | quote }}
FIRST_YEAR_WITH_PREV_REPORTING_YEAR_OPTION: {{ .Values.global.config.first_year_with_prev_reporting_year_option | quote }}
DB_CONNECTION_POOL_SIZE: {{ .Values.global.config.db_connection_pool_size | quote }}
IS_USER_ERROR_LOGGING_ENABLED: {{ .Values.global.config.is_user_error_logging_enabled | quote }}
NUM_MONTHS_OF_USER_ERRORS_TO_KEEP: {{ .Values.global.config.num_months_of_user_errors_to_keep | quote }}
4 changes: 3 additions & 1 deletion charts/fin-pay-transparency/values-dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ global:
report_unlock_duration_in_days: "2"
template_path: "./dist/templates"
delete_draft_report_cron_crontime: "0 0 0 * * *" # 12:00 AM PST/PDT
delete_user_errors_cron_crontime: "30 0 1 */6 *" #At 12:30 AM on the 1st day of every 6th month
lock_report_cron_crontime: "0 15 0 * * *" # 12:15 AM PST/PDT
reports_scheduler_cron_timezone: "America/Vancouver"
first_year_with_prev_reporting_year_option: "2025"
db_connection_pool_size: "5"

num_months_of_user_errors_to_keep: "6"
is_user_error_logging_enabled: "true"
crunchyEnabled: true
backend:
enabled: true
Expand Down
2 changes: 2 additions & 0 deletions charts/fin-pay-transparency/values-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ global:
domain: "apps.silver.devops.gov.bc.ca" # it is required, apps.silver.devops.gov.bc.ca for silver cluster
autoscaling:
enabled: false
config:
is_user_error_logging_enabled: "true"
crunchyEnabled: false
seedData: true
backend:
Expand Down
Loading

0 comments on commit 279ceae

Please sign in to comment.