Merge pull request #122 from digital-land/rosado/logging

Logging improvements

.gitignore

@@ -78,6 +78,7 @@ web_modules/
 .env.test.local
 .env.production.local
 .env.local
+.envrc
 
 # parcel-bundler cache (https://parceljs.org/)
 .cache

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)
 

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)
       }

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
     }
   }

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
     }
   }

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]

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']

src/models/responseDetails.js

@@ -11,15 +11,15 @@ 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
   }
 
   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
     }
 

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)
     }

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,18 +29,15 @@ 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]
       }
     }).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
   }
 }

src/utils/logger.js

@@ -1,15 +1,22 @@
 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(
     format.timestamp(),
     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

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')
     })
   })