diff --git a/.gitignore b/.gitignore index 54a3c448d..31c9362c4 100644 --- a/.gitignore +++ b/.gitignore @@ -78,6 +78,7 @@ web_modules/ .env.test.local .env.production.local .env.local +.envrc # parcel-bundler cache (https://parceljs.org/) .cache diff --git a/config/index.js b/config/index.js index 7afbc119e..efc3303ea 100644 --- a/config/index.js +++ b/config/index.js @@ -6,12 +6,16 @@ const readConfig = (config) => { return yaml.load(fs.readFileSync(`./config/${config}.yaml`, 'utf8')) } +/** + * + * @returns {{defaultConfig, environment}} + */ const getConfig = () => { const defaultConfig = readConfig('default') const environment = process.env.NODE_ENV || process.env.ENVIRONMENT || 'production' - console.log('USING CONFIG: ' + environment) + if (environment !== 'test') console.info('USING CONFIG: ' + environment) const customConfig = readConfig(environment) diff --git a/src/assets/js/statusPage.js b/src/assets/js/statusPage.js index 2ae644cb4..cee246997 100644 --- a/src/assets/js/statusPage.js +++ b/src/assets/js/statusPage.js @@ -1,6 +1,7 @@ // poll the server for the status of the job import { finishedProcessingStatuses } from '../../utils/utils.js' +import logger from '../../utils/logger.js' export default class StatusPage { constructor (pollingInterval, maxPollAttempts) { @@ -36,7 +37,7 @@ export default class StatusPage { fetch(statusEndpoint) .then(res => res.json()) .then(data => { - console.log('polled request and got a status of: ' + data.status) + logger.info('StatusPage: polled request and got a status of: ' + data.status) // ToDo: handle other status' here if (finishedProcessingStatuses.includes(data.status)) { this.updatePageToComplete() @@ -46,7 +47,7 @@ export default class StatusPage { this.pollAttempts++ if (this.pollAttempts > this.maxPollAttempts) { - console.log('polling timed out') + logger.info('StatusPage: polling timed out') this.updatePageForPollingTimeout() clearInterval(interval) } diff --git a/src/controllers/submitUrlController.js b/src/controllers/submitUrlController.js index 3a06e2a43..a7b2f055e 100644 --- a/src/controllers/submitUrlController.js +++ b/src/controllers/submitUrlController.js @@ -17,7 +17,7 @@ class SubmitUrlController extends UploadController { const errors = { url: new SubmitUrlController.Error(error.key, error, req, res) } - logger.error('local validation failed during url submission', error) + logger.warn('SubmitUrlController: local validation failed during url submission', error) return next(errors) } @@ -91,7 +91,7 @@ class SubmitUrlController extends UploadController { return sizeInMB <= 10 } catch (err) { - console.error(err) + console.warn(err) return true // for now we will allow this file as we can't be sure } } @@ -103,7 +103,7 @@ class SubmitUrlController extends UploadController { try { return (response.status >= 200 && response.status < 300) || response.status === 400 // need to add 400 as some servers return 400 for head requests } catch (err) { - console.error(err) + logger.warn(err) return true } } @@ -114,7 +114,7 @@ class SubmitUrlController extends UploadController { const acceptedTypes = Object.values(allowedFileTypes).flat() return acceptedTypes.includes(contentType) } catch (err) { - console.error(err) + logger.warn(err) return false } } diff --git a/src/controllers/uploadFileController.js b/src/controllers/uploadFileController.js index 8b464e46e..c22ed39d6 100644 --- a/src/controllers/uploadFileController.js +++ b/src/controllers/uploadFileController.js @@ -46,7 +46,7 @@ class UploadFileController extends UploadController { const errors = { datafile: new UploadFileController.Error(error.key, error, req, res) } - logger.error('local validation failed during file upload', error) + logger.warn('UploadFileController: local validation failed during file upload', error) return next(errors) } @@ -58,8 +58,7 @@ class UploadFileController extends UploadController { const id = await postFileRequest({ ...this.getBaseFormData(req), originalFilename: req.file.originalname, uploadedFilename }) req.body.request_id = id - // log the file name, type and size as an object - logger.info('file submitted for processing:', { type: 'fileUploaded', name: req.file.originalname, mimetype: req.file.mimetype, size: req.file.size }) + logger.info('UploadFileController: file submitted for processing:', { type: 'fileUploaded', name: req.file.originalname, mimetype: req.file.mimetype, size: req.file.size }) super.post(req, res, next) } catch (error) { @@ -100,7 +99,7 @@ class UploadFileController extends UploadController { await s3.upload(params).promise() return uuid } catch (error) { - logger.error('Error uploading file to S3: ' + error.message) + logger.warn('Error uploading file to S3: ' + error.message) throw error } } diff --git a/src/filters/validationMessageLookup.js b/src/filters/validationMessageLookup.js index dfee37103..f57a76d4f 100644 --- a/src/filters/validationMessageLookup.js +++ b/src/filters/validationMessageLookup.js @@ -62,7 +62,7 @@ const validationMessages = { function validationMessageLookup (field, type) { if (!validationMessages[field] || !validationMessages[field][type]) { - logger.error('No validation message found for field ' + field + ' and type ' + type) + logger.warn('No validation message found for field ' + field + ' and type ' + type) return `An error occurred of type ${type} for field ${field}` } return validationMessages[field][type] diff --git a/src/models/requestData.js b/src/models/requestData.js index dcbdd5ad8..3ad503c69 100644 --- a/src/models/requestData.js +++ b/src/models/requestData.js @@ -29,7 +29,7 @@ export default class RequestData { getErrorSummary () { if (!this.response || !this.response.data || !this.response.data['error-summary']) { - logger.error('trying to get error summary when there is none: request id: ' + this.id) + logger.warn('trying to get error summary when there is none: request id: ' + this.id) return [] } return this.response.data['error-summary'] @@ -45,7 +45,7 @@ export default class RequestData { getError () { if (!this.response) { - logger.error('trying to get error when there are none: request id: ' + this.id) + logger.warn('trying to get error when there are none: request id: ' + this.id) return { message: 'An unknown error occurred.' } } @@ -54,11 +54,11 @@ export default class RequestData { hasErrors () { if (!this.response || !this.response.data) { - logger.error('trying to check for errors when there are none: request id: ' + this.id) + logger.warn('trying to check for errors when there are none: request id: ' + this.id) return true } if (this.response.data['error-summary'] == null) { - logger.error('trying to check for errors but there is no error-summary: request id: ' + this.id) + logger.warn('trying to check for errors but there is no error-summary: request id: ' + this.id) return true } return this.response.data['error-summary'].length > 0 @@ -71,7 +71,7 @@ export default class RequestData { getColumnFieldLog () { if (!this.response || !this.response.data || !this.response.data['column-field-log']) { - logger.error('trying to get column field log when there is none: request id: ' + this.id) + logger.warn('trying to get column field log when there is none: request id: ' + this.id) return [] } return this.response.data['column-field-log'] diff --git a/src/models/responseDetails.js b/src/models/responseDetails.js index d29b569f3..028b037a4 100644 --- a/src/models/responseDetails.js +++ b/src/models/responseDetails.js @@ -11,7 +11,7 @@ export default class ResponseDetails { getRows () { if (!this.response) { - logger.error('trying to get response details when there are none: request id: ' + this.id) + logger.warn('trying to get response details when there are none: request id: ' + this.id) return [] } return this.response @@ -19,7 +19,7 @@ export default class ResponseDetails { getColumnFieldLog () { if (!this.columnFieldLog) { - logger.error('trying to get column field log when there is none: request id: ' + this.id) + logger.warn('trying to get column field log when there is none: request id: ' + this.id) return [] } return this.columnFieldLog @@ -69,7 +69,7 @@ export default class ResponseDetails { // This function returns an array of rows with verbose columns getRowsWithVerboseColumns (filterNonErrors = false) { if (!this.response) { - logger.error('trying to get response details when there are none: request id: ' + this.id) + logger.warn('trying to get response details when there are none: request id: ' + this.id) return [] } @@ -105,7 +105,7 @@ export default class ResponseDetails { getGeometries () { if (!this.response) { - logger.error('trying to get response details when there are none: request id: ' + this.id) + logger.warn('trying to get response details when there are none: request id: ' + this.id) return undefined } diff --git a/src/routes/health.js b/src/routes/health.js index 3d1ccb76b..99bd94325 100644 --- a/src/routes/health.js +++ b/src/routes/health.js @@ -72,7 +72,7 @@ const checkRedis = async () => { return false } }).catch((err) => { - logger.error(`checkRedis/connect: ${err.message}`) + logger.warn(`checkRedis/connect: ${err.message}`) if (config.environment !== 'test') { console.error(err) } diff --git a/src/utils/fetchLocalAuthorities.js b/src/utils/fetchLocalAuthorities.js index 3891ce0f5..758b692a9 100644 --- a/src/utils/fetchLocalAuthorities.js +++ b/src/utils/fetchLocalAuthorities.js @@ -1,6 +1,5 @@ import axios from 'axios' import logger from '../../src/utils/logger.js' -import config from '../../config/index.js' /** * Fetches a list of local authority names from a specified dataset. @@ -30,7 +29,7 @@ export const fetchLocalAuthorities = async () => { const response = await axios.get(url) const names = response.data.rows.map(row => { if (row[1] === null) { - console.log('Null value found in response:', row) + logger.debug('Null value found in response:', row) return null } else { return row[1] @@ -38,10 +37,7 @@ export const fetchLocalAuthorities = async () => { }).filter(name => name !== null) // Filter out null values return names } catch (error) { - logger.error(`fetchLocalAuthorities: Error fetching local authorities data: ${error.message}`) - if (config.environment !== 'test') { - console.error(error) - } + logger.warn(`fetchLocalAuthorities: Error fetching local authorities data: ${error.message}`, error) throw error } } diff --git a/src/utils/logger.js b/src/utils/logger.js index 371d7df3a..61bb4fcae 100644 --- a/src/utils/logger.js +++ b/src/utils/logger.js @@ -1,4 +1,14 @@ import { createLogger, format, transports } from 'winston' +import config from '../../config/index.js' + +const appTransports = () => [ + new transports.Console(), + new transports.File({ filename: 'combined.log' }) +] + +const testTransports = () => [ + new transports.Console({ level: 'error' }) +] const logger = createLogger({ format: format.combine( @@ -6,10 +16,7 @@ const logger = createLogger({ format.json() ), defaultMeta: { service: 'lpa-data-validation-frontend' }, - transports: [ - new transports.Console(), - new transports.File({ filename: 'combined.log' }) - ] + transports: config.environment === 'test' ? testTransports() : appTransports() }) export default logger diff --git a/test/unit/requestData.test.js b/test/unit/requestData.test.js index 3e2b714c2..b8c26d2a6 100644 --- a/test/unit/requestData.test.js +++ b/test/unit/requestData.test.js @@ -89,15 +89,13 @@ describe('RequestData', () => { expect(errorSummary).toStrictEqual(['error1', 'error2']) }) - it('should return an empty array if there is no error summary and log an error', () => { + it('should return an empty array if there is no error summary', () => { const response = {} const requestData = new RequestData(response) const errorSummary = requestData.getErrorSummary() expect(errorSummary).toStrictEqual([]) - - expect(logger.error).toHaveBeenCalledWith('trying to get error summary when there is none: request id: undefined') }) }) @@ -150,14 +148,12 @@ describe('RequestData', () => { expect(error).toStrictEqual({ message: 'error message' }) }) - it('should return an unknown error if there is no error and log an error', () => { + it('should return an unknown error if there is no error', () => { const requestData = new RequestData({}) const error = requestData.getError() expect(error).toStrictEqual({ message: 'An unknown error occurred.' }) - - expect(logger.error).toHaveBeenCalledWith('trying to get error when there are none: request id: undefined') }) }) @@ -175,17 +171,15 @@ describe('RequestData', () => { expect(hasErrors).toBe(true) }) - it('should return true if there are no errors and log an error', () => { + it('should return true if there are no errors', () => { const requestData = new RequestData({}) const hasErrors = requestData.hasErrors() expect(hasErrors).toBe(true) - - expect(logger.error).toHaveBeenCalledWith('trying to check for errors when there are none: request id: undefined') }) - it('should return true if there is no error summary and log an error', () => { + it('should return true if there is no error summary', () => { const response = { data: {} } @@ -194,8 +188,6 @@ describe('RequestData', () => { const hasErrors = requestData.hasErrors() expect(hasErrors).toBe(true) - - expect(logger.error).toHaveBeenCalledWith('trying to check for errors but there is no error-summary: request id: undefined') }) it('should return false if the error summary is empty', () => { @@ -261,14 +253,12 @@ describe('RequestData', () => { expect(columnFieldLog).toStrictEqual(['column1', 'column2']) }) - it('should return an empty array if there is no column field log and log an error', () => { + it('should return an empty array if there is no column field log', () => { const requestData = new RequestData({}) const columnFieldLog = requestData.getColumnFieldLog() expect(columnFieldLog).toStrictEqual([]) - - expect(logger.error).toHaveBeenCalledWith('trying to get column field log when there is none: request id: undefined') }) }) diff --git a/test/unit/responseDetails.test.js b/test/unit/responseDetails.test.js index 791a6a47d..711506ead 100644 --- a/test/unit/responseDetails.test.js +++ b/test/unit/responseDetails.test.js @@ -1,6 +1,5 @@ import ResponseDetails, { pagination } from '../../src/models/responseDetails' -import { describe, it, expect, vi, beforeEach } from 'vitest' -import logger from '../../src/utils/logger.js' +import { describe, it, expect, vi } from 'vitest' vi.mock('../../src/utils/getVerboseColumns.js', () => { return { @@ -70,12 +69,6 @@ describe('ResponseDetails', () => { } }) - const loggerErrorSpy = vi.spyOn(logger, 'error') - - beforeEach(() => { - loggerErrorSpy.mockClear() - }) - describe('getRows', () => { it('returns the rows', () => { const responseDetails = new ResponseDetails(undefined, mockResponse, mockPagination, mockColumnFieldLog) @@ -83,11 +76,10 @@ describe('ResponseDetails', () => { expect(result).toBe(mockResponse) }) - it('returns an empty array if there are no rows and logs an error', () => { + it('returns an empty array if there are no rows', () => { const responseDetails = new ResponseDetails(undefined, undefined, undefined, undefined) const result = responseDetails.getRows() expect(result).toStrictEqual([]) - expect(loggerErrorSpy).toHaveBeenCalled() }) }) @@ -98,11 +90,10 @@ describe('ResponseDetails', () => { expect(result).toStrictEqual(mockColumnFieldLog) }) - it('returns an empty array if there is no column field log and logs an error', () => { + it('returns an empty array if there is no column field log', () => { const responseDetails = new ResponseDetails(undefined, undefined, undefined, undefined) const result = responseDetails.getColumnFieldLog() expect(result).toStrictEqual([]) - expect(loggerErrorSpy).toHaveBeenCalled() }) }) @@ -203,11 +194,10 @@ describe('ResponseDetails', () => { expect(result).toStrictEqual(expected) }) - it('returns an empty array if there are no rows and logs an error', () => { + it('returns an empty array if there are no rows', () => { const responseDetails = new ResponseDetails(undefined, undefined, undefined, undefined) const result = responseDetails.getRowsWithVerboseColumns() expect(result).toStrictEqual([]) - expect(loggerErrorSpy).toHaveBeenCalled() }) }) @@ -256,7 +246,6 @@ describe('ResponseDetails', () => { const responseDetails = new ResponseDetails(undefined, undefined, undefined, undefined) const result = responseDetails.getGeometries() expect(result).toBeUndefined() - expect(loggerErrorSpy).toHaveBeenCalled() }) it('returns null if there are no geometries', () => { diff --git a/test/unit/validationLookup.test.js b/test/unit/validationLookup.test.js index 63510b040..6a94defe9 100644 --- a/test/unit/validationLookup.test.js +++ b/test/unit/validationLookup.test.js @@ -2,7 +2,6 @@ import { describe, it, expect, vi } from 'vitest' import validationMessageLookup from '../../src/filters/validationMessageLookup.js' -import logger from '../../src/utils/logger.js' describe('validationMessageLookup', () => { it('returns the correct message for a given field and type', () => { @@ -19,12 +18,6 @@ describe('validationMessageLookup', () => { error: vi.fn() } }) - - const loggerErrorSpy = vi.spyOn(logger, 'error') - validationMessageLookup('email-address', 'missing') - - expect(loggerErrorSpy).toHaveBeenCalledOnce() - expect(loggerErrorSpy.mock.calls[0][0]).toBe('No validation message found for field email-address and type missing') }) })