`TypeError` exception when `handleExceptions: true`

Terrabits commented 7 years ago

When I use winston-console-formatter with winston.transports.Console and handleExceptions: true, I get the following error:

.../winston-console-formatter/dist/utils.js:14
  return clc.magenta('\n  ' + stackTrace.replace(/(\r\n|\n|\r)/gm, '$1  '));
                                         ^

TypeError: stackTrace.replace is not a function
    at Object.getStackTrace (.../winston-console-formatter/dist/utils.js:14:42)
    at Object.formatter (.../winston-console-formatter/dist/index.js:32:31)
    at Object.exports.log (.../winston/lib/winston/common.js:218:27)
    at Console.log (.../winston/lib/winston/transports/console.js:99:19)
    at Transport.logException (.../winston/lib/winston/transports/transport.js:134:8)
    at logAndWait (.../winston/lib/winston/logger.js:649:15)
    at .../async/lib/async.js:157:13
    at _each (.../async/lib/async.js:57:9)
    at Object.async.each (.../async/lib/async.js:156:9)
    at Logger._uncaughtException (.../winston/lib/winston/logger.js:672:9)

To reproduce:

const winston = require('winston');
const toYAML  = require('winston-console-formatter');

const {timestamp, formatter} = toYAML.config();

const logger = new winston.Logger({
  level: 'silly'
});

logger.add(winston.transports.Console, {
  timestamp,
  formatter,
  handleExceptions: true,
});

throw 'My exception';

This is using Node 6.11.0 and winston-console-formatter 0.27.5.

I was able to fix this by making the following change:

in src/utils.js at line 12

return clc.magenta('\n  ' + String(stackTrace).replace(/(\r\n|\n|\r)/gm, '$1  '));

I can pull-request this if it helps.

Terrabits commented 7 years ago

In creating tests for the proposed fix above, I did more research into what values the options.meta.stack and options.meta.trace variables have when passed into formatter(options). In doing so, I realized that simply converting stackTrace to a string, while it avoids the error, is not the best solution to this problem.

It appears that stack is an array of strings, each corresponding to one line of the human-readable stack normally displayed in console. Trace, on the other hand, is an array of objects, each of which contains information that can be used to recreate the stack display (calling function, source file, line and column). When stack is missing (for whatever reason), a recreation of the stack display is what (I assume) we ultimately want.

So I think a better solution might be to modify index.js line 39: utils.getStackTrace(stack || trace) and utils.getStackTrace to utils.getStackTrace(stack, trace) and the following for utils.getStackTrace:

/**
 * @param {[strings...]|undefined} stack
 * @param {[callbacks...]|undefined}
 * @returns {string}
 */
function getStackTrace (stack, trace) {
  let msg;
  if (stack && stack.length) {
    msg = '\n  ' + stack.join('\n  ');
  }
  else if (trace && trace.length) {
    const lines = trace.map((i) => {
      const fnString = i.function ? `${i.function} ` : '';
      return `at ${fnString}(${i.file}:${i.line}:${i.column})`;
    });
    msg = '\n  ' + lines.join('\n    ');
  }
  return msg ? clc.magenta(msg) : '';
}

This function basically either prints the stack trace (restoring line breaks) as it is if it exists, or recreates as much of this message as it can from the trace if it does not exist.

If this solution seems better than the original one, I will kill the original pull request and create a new one. I also have tests for the above case to submit as well.

See the master branch of my fork.

eugeny-dementev commented 6 years ago

Fixed in 1.0.0-beta.1

eugeny-dementev / winston-console-formatter

`TypeError` exception when `handleExceptions: true` #4