Merge pull request #1334 from log4js-node/feat/invalid-shutdown-callback

feat(log4js): if cb is passed to `shutdown()`, it must be a function or it will throw error immediately

Author: lamweili

.github/workflows/node.js.yml

@@ -63,8 +63,6 @@ jobs:
     - name: Disable prettier on older Node.js (8.x, 10.x, 12.x)
       run: |
         sed -i '/"prettier": "prettier/d' package.json
-        echo Removed
-        sed -n '/"prettier": "prettier/p' package.json
       if: contains(fromJson('["8.x", "10.x", "12.x"]'), matrix.node-version)
 
     - name: Install downgraded modules ${{ matrix.npm-i }}

docs/api.md

@@ -35,7 +35,7 @@ This function takes a single optional string argument to denote the category to
 - `addContext(<key>,<value>)` - where `<key>` is a string, `<value>` can be anything. This stores a key-value pair that is added to all log events generated by the logger. Uses would be to add ids for tracking a user through your application. Currently only the `logFaces` appenders make use of the context values.
 - `removeContext(<key>)` - removes a previously defined key-value pair from the context.
 - `clearContext()` - removes all context pairs from the logger.
-- `setParseCallStackFunction(function | undefined)` - Allow to override the default way to parse the callstack data for the layout pattern, a generic javascript Error object is passed to the function. Must return an object with properties : `functionName` / `fileName` / `lineNumber` / `columnNumber` / `callStack`. Can for example be used if all of your log call are made from one "debug" class and you would to "erase" this class from the callstack to only show the function which called your "debug" class. If you pass `undefined` as the argument, it will be reset to the default parser.
+- `setParseCallStackFunction(function | undefined)` - Allow to override the default way to parse the callstack data for the layout pattern, a generic javascript Error object is passed to the function. Must return an object with properties : `fileName` / `lineNumber` / `columnNumber` / `callStack` / `className` / `functionName` / `functionAlias` / `callerName`. Can, for example, be used if all of your log call are made from one "debug" class and you would to "erase" this class from the callstack to only show the function which called your "debug" class. If you pass `undefined` as the argument, it will be reset to the default parser.
 
 The `Logger` object has the following properties:
 

lib/log4js.js

@@ -98,7 +98,10 @@ function recording() {
  *  shutdown. If an error occurs, the callback will be given the error object
  *  as the first argument.
  */
-function shutdown(callback) {
+function shutdown(callback = () => {}) {
+  if (typeof callback !== 'function') {
+    throw new TypeError('Invalid callback passed to shutdown');
+  }
   debug('Shutdown called. Disabling all log writing.');
   // First, disable all writing to appenders. This prevents appenders from
   // not being able to be drained because of run-away log writes.
@@ -112,15 +115,13 @@ function shutdown(callback) {
   categories.init();
 
   // Count the number of shutdown functions
-  const shutdownFunctions = appendersToCheck.reduceRight(
+  const shutdownFunctions = appendersToCheck.reduce(
     (accum, next) => (next.shutdown ? accum + 1 : accum),
     0
   );
   if (shutdownFunctions === 0) {
     debug('No appenders with shutdown functions found.');
-    if (callback) {
-      callback();
-    }
+    callback();
   }
 
   let completed = 0;
@@ -132,9 +133,7 @@ function shutdown(callback) {
     debug(`Appender shutdowns complete: ${completed} / ${shutdownFunctions}`);
     if (completed >= shutdownFunctions) {
       debug('All shutdown functions completed.');
-      if (callback) {
-        callback(error);
-      }
+      callback(error);
     }
   }
 

test/tap/logging-test.js

@@ -4,6 +4,12 @@ const util = require('util');
 const recording = require('../../lib/appenders/recording');
 
 test('log4js', (batch) => {
+  batch.test('should throw error for invalid callback to shutdown', (t) => {
+    const log4js = require('../../lib/log4js');
+    t.throws(() => log4js.shutdown([]));
+    t.end();
+  });
+
   batch.test(
     'shutdown should return appenders and categories back to initial state',
     (t) => {