Merge pull request #81 from Jalle19/debug-logging

Refactor logging completely

.eslintrc.json

@@ -0,0 +1,13 @@
+{
+    "env": {
+        "browser": true,
+        "es2021": true
+    },
+    "parserOptions": {
+        "ecmaVersion": "latest",
+        "sourceType": "module"
+    },
+    "rules": {
+        "no-console": "error"
+    }
+}

.github/workflows/lint.yml

@@ -22,3 +22,16 @@ jobs:
         run: npm ci
       - name: Check formatting
         run: npm run check-prettier
+  eslint:
+    name: Run eslint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Use Node.js
+        uses: actions/setup-node@v2
+        with:
+          node-version: '14.x'
+      - name: Install dependencies
+        run: npm ci
+      - name: Run eslint
+        run: npm run lint

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Change log
 
+## future release
+
+* Drastically improve application logging
+* Add missing error handling to the `/summary` route, change HTTP error responses to return JSON
+* Add `-v` option which enables debug logging
+* Start using ESLint, add `no-console` rule to enforce use of logger instances
+
 ## 2.3.2
 
 * Document how to troubleshoot RS-485 connection issues

README.md

@@ -112,6 +112,7 @@ Options:
       --mqttDiscovery        Whether to enable Home Assistant MQTT discovery sup
                              port. Only effective when mqttBrokerUrl is defined.
                                                        [boolean] [default: true]
+  -v, --debug                Enable debug logging     [boolean] [default: false]
 ```
 
 ## HTTP endpoints

app/homeassistant.mjs

@@ -7,6 +7,9 @@ import {
     TOPIC_PREFIX_READINGS,
     TOPIC_PREFIX_SETTINGS,
 } from './mqtt.mjs'
+import { createLogger } from './logger.mjs'
+
+const logger = createLogger('homeassistant')
 
 export const configureMqttDiscovery = async (modbusClient, mqttClient) => {
     // Build information about the ventilation unit. The "deviceIdentifier" is used as <node_id> in discovery topic
@@ -228,7 +231,7 @@ export const configureMqttDiscovery = async (modbusClient, mqttClient) => {
             const configurationTopicName = `homeassistant/${entityType}/${deviceIdentifier}/${entityName}/config`
 
             // "retain" is used so that the entities will be available immediately after a Home Assistant restart
-            console.log(`Publishing Home Assistant auto-discovery configuration for ${entityType} "${entityName}"...`)
+            logger.info(`Publishing Home Assistant auto-discovery configuration for ${entityType} "${entityName}"...`)
             await mqttClient.publish(configurationTopicName, JSON.stringify(configuration), {
                 retain: true,
             })

app/http.mjs

@@ -9,22 +9,29 @@ import {
     getAlarmHistory,
     getDeviceState,
 } from './modbus.mjs'
+import { createLogger } from './logger.mjs'
+
+const logger = createLogger('http')
 
 export const root = async (req, res) => {
     res.send('eda-modbus-bridge')
 }
 
 export const summary = async (modbusClient, req, res) => {
-    const summary = {
-        'flags': await getFlagSummary(modbusClient),
-        'readings': await getReadings(modbusClient),
-        'settings': await getSettings(modbusClient),
-        'deviceInformation': await getDeviceInformation(modbusClient),
-        'alarmHistory': await getAlarmHistory(modbusClient),
-        'deviceState': await getDeviceState(modbusClient),
-    }
+    try {
+        const summary = {
+            'flags': await getFlagSummary(modbusClient),
+            'readings': await getReadings(modbusClient),
+            'settings': await getSettings(modbusClient),
+            'deviceInformation': await getDeviceInformation(modbusClient),
+            'alarmHistory': await getAlarmHistory(modbusClient),
+            'deviceState': await getDeviceState(modbusClient),
+        }
 
-    res.json(summary)
+        res.json(summary)
+    } catch (e) {
+        handleError(e, res)
+    }
 }
 
 export const getFlagStatus = async (modbusClient, req, res) => {
@@ -36,8 +43,7 @@ export const getFlagStatus = async (modbusClient, req, res) => {
             'active': status,
         })
     } catch (e) {
-        res.status(400)
-        res.send(e.message)
+        handleError(e, res)
     }
 }
 
@@ -46,16 +52,15 @@ export const setFlagStatus = async (modbusClient, req, res) => {
         const flag = req.params['flag']
         const status = !!req.body['active']
 
-        console.log(`Setting flag ${flag} to ${status}`)
+        logger.info(`Setting flag ${flag} to ${status}`)
 
         await setFlag(modbusClient, flag, status)
 
         res.json({
             'active': await getFlag(modbusClient, flag),
         })
     } catch (e) {
-        res.status(400)
-        res.send(e.message)
+        handleError(e, res)
     }
 }
 
@@ -64,15 +69,20 @@ export const setSetting = async (modbusClient, req, res) => {
         const setting = req.params['setting']
         const value = req.params['value']
 
-        console.log(`Setting setting ${setting} to ${value}`)
+        logger.info(`Setting setting ${setting} to ${value}`)
 
         await modbusSetSetting(modbusClient, setting, value)
 
         res.json({
             'settings': await getSettings(modbusClient),
         })
     } catch (e) {
-        res.status(400)
-        res.send(e.message)
+        handleError(e, res)
     }
 }
+
+const handleError = (e, res) => {
+    logger.error(`An exception occurred: ${e.name}: ${e.message}`, e.stack)
+    res.status(400)
+    res.json(e)
+}

app/logger.mjs

@@ -0,0 +1,27 @@
+import winston from 'winston'
+
+const DEFAULT_LOG_LEVEL = 'info'
+
+// Define log transports here so we can change the log level later
+let transports = [new winston.transports.Console()]
+
+const logFormat = winston.format.printf(({ level, message, label, timestamp }) => {
+    return `${timestamp} [${label}] ${level}: ${message}`
+})
+
+export const setLogLevel = (logger, level) => {
+    logger.info(`Setting log level to ${level}`)
+    transports[0].level = level
+}
+
+export const createLogger = (module) => {
+    return winston.createLogger({
+        'level': DEFAULT_LOG_LEVEL,
+        'format': winston.format.combine(
+            winston.format.label({ label: module }),
+            winston.format.timestamp(),
+            logFormat
+        ),
+        'transports': transports,
+    })
+}