Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unified Handler Tests #2020

Merged
merged 11 commits into from
Sep 28, 2024
114 changes: 11 additions & 103 deletions test/exception-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,111 +6,19 @@
*
*/

const stream = require('stream');
const assume = require('assume');
const mocha = require('mocha');
const winston = require('../lib/winston');
const baseHandlerTests = require('./helpers/handlers');
const helpers = require('./helpers');

//
// This is an awful and fragile hack that
// needs to be changed ASAP.
// https://github.com/mochajs/mocha/issues/1985
//
var _runTest = mocha.Runner.prototype.runTest;
mocha.Runner.prototype.runTest = function () {
this.allowUncaught = true;
_runTest.apply(this, arguments);
};

describe('ExceptionHandler', function () {
this.timeout(5000);

it('has expected methods', function () {
var handler = helpers.exceptionHandler();
assume(handler.handle).is.a('function');
assume(handler.unhandle).is.a('function');
assume(handler.getAllInfo).is.a('function');
assume(handler.getProcessInfo).is.a('function');
assume(handler.getOsInfo).is.a('function');
assume(handler.getTrace).is.a('function');
});

it('new ExceptionHandler()', function () {
assume(function () {
new winston.ExceptionHandler();
}).throws(/Logger is required/);
});

it('new ExceptionHandler(logger)', function () {
var logger = winston.createLogger();
var handler = new winston.ExceptionHandler(logger);
assume(handler.logger).equals(logger);
});

it('.getProcessInfo()', function () {
var handler = helpers.exceptionHandler();
helpers.assertProcessInfo(handler.getProcessInfo());
});

it('.getOsInfo()', function () {
var handler = helpers.exceptionHandler();
helpers.assertOsInfo(handler.getOsInfo());
});

it('.getTrace(new Error)', function () {
var handler = helpers.exceptionHandler();
helpers.assertTrace(handler.getTrace(new Error()));
});

it('.getTrace()', function () {
var handler = helpers.exceptionHandler();
helpers.assertTrace(handler.getTrace());
});

it('.handle()', function (done) {
var existing = helpers.clearExceptions();
var writeable = new stream.Writable({
objectMode: true,
write: function (info) {
assume(info).is.an('object');
assume(info.error).is.an('error');
assume(info.error.message).equals('wtf this error');
assume(info.message).includes('uncaughtException: wtf this error');
assume(info.stack).is.a('string');
assume(info.process).is.an('object');
assume(info.os).is.an('object');
assume(info.trace).is.an('array');

existing.restore();
done();
}
});

var transport = new winston.transports.Stream({ stream: writeable });
var handler = helpers.exceptionHandler({
exitOnError: false,
transports: [transport]
});

assume(handler.catcher).equals(undefined);

transport.handleExceptions = true;
handler.handle();

assume(handler.catcher).is.a('function');
assume(process.listeners('uncaughtException')).deep.equals([
handler.catcher
]);

helpers.throw('wtf this error');
});

after(function () {
//
// Restore normal `runTest` functionality
// so that we only affect the current suite.
//
mocha.Runner.prototype.runTest = _runTest;
this.timeout(100);

baseHandlerTests({
name: 'ExceptionHandler',
helper: 'exceptionHandler',
getAllInfo: new Error('catpants'),
setup: 'clearExceptions',
listener: 'uncaughtException',
toggleSetting: 'handleExceptions',
trigger: msg => helpers.throw(msg)
});
});
119 changes: 119 additions & 0 deletions test/helpers/handlers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
const assume = require('assume');
const mocha = require('mocha');

const helpers = require('.');
const winston = require('../../lib/winston');

module.exports = function ({ getAllInfo, helper, listener, name, setup, toggleSetting, trigger }) {
before(function () {
/*
* This is an awful and fragile hack that
* needs to be changed ASAP.
* https://github.com/mochajs/mocha/issues/1985
*/
var _runTest = this.originalRunTest = mocha.Runner.prototype.runTest;

mocha.Runner.prototype.runTest = function () {
this.allowUncaught = true;
_runTest.apply(this, arguments);
};
});

after(function () {
mocha.Runner.prototype.runTest = this.originalRunTest;
});

it('has expected methods', function () {
var handler = helpers[helper]();
assume(handler.handle).is.a('function');
assume(handler.unhandle).is.a('function');
assume(handler.getAllInfo).is.a('function');
assume(handler.getProcessInfo).is.a('function');
assume(handler.getOsInfo).is.a('function');
assume(handler.getTrace).is.a('function');
});

it(`new ${name}()`, function () {
assume(function () {
// eslint-disable-next-line no-new
new winston[name]();
}).throws(/Logger is required/);
});

it(`new ${name}(logger)`, function () {
var logger = winston.createLogger();
var handler = new winston[name](logger);
assume(handler.logger).equals(logger);
});

it('.getProcessInfo()', function () {
var handler = helpers[helper]();
helpers.assertProcessInfo(handler.getProcessInfo());
});

it('.getOsInfo()', function () {
var handler = helpers[helper]();
helpers.assertOsInfo(handler.getOsInfo());
});

it('.getTrace(new Error)', function () {
var handler = helpers[helper]();
helpers.assertTrace(handler.getTrace(new Error()));
});

it('.getTrace()', function () {
var handler = helpers[helper]();
helpers.assertTrace(handler.getTrace());
});

it('.getAllInfo(undefined)', function () {
var handler = helpers[helper]();
// eslint-disable-next-line no-undefined
handler.getAllInfo(getAllInfo);
});

describe('when error case is triggered', function () {
beforeEach(function () {
this.listeners = helpers[setup]();
});

afterEach(function () {
this.listeners.restore();
});

it('.handle()', function (done) {
var msg = new Date().toString();
var writeable = helpers.writeable(function (info) {
console.log('in writeable', info);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Called as expected for the exception handler but never for the rejection handler.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense... excellent eye @fearphage 💯 Does this allow tests to pass in mocha@9 (or whatever the latest version is)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, these changes cause the rejection handler test to fail consistently (due to timeout) accross all versions of Mocha (8 and 9).

So this appears to be a code issue and the previously bad test hid this for a while. I don't know if this is a test problem or a code problem.

The event is attached to the handler (as confirmed by the tests), but firing the appropriate event doesn't trigger the stream to be written. I don't get it. I could use another set of eyes on it for sure.

Copy link
Contributor Author

@fearphage fearphage Jan 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wbt @DABH Anything jump out as broken or misspelled here? I'm stuck.

Either the test is bad OR the code is bad... or possibly both. I'm going to take a break and circle back to it in a day or two to see if that gives me some perspective.

Edit: This test passes for the exception handler, but only fails for the rejection handler.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I have not had time to look into this, and I'm not super optimistic about being able to give much time to this over the next few days at least 😞

assume(info).is.an('object');
assume(info.error).is.an('error');
assume(info.error.message).equals(msg);
assume(info.message).includes(`${listener}: ${msg}`);
assume(info.stack).is.a('string');
assume(info.process).is.an('object');
assume(info.os).is.an('object');
assume(info.trace).is.an('array');

done();
});

var transport = new winston.transports.Stream({ stream: writeable });
var handler = helpers[helper]({
exitOnError: false,
transports: [transport]
});

assume(handler.catcher).is.a('undefined');

transport[toggleSetting] = true;
handler.handle();

assume(handler.catcher).is.a('function');
assume(process.listeners(listener)).deep.equals([
handler.catcher
]);

trigger(msg);
});
});
};
8 changes: 2 additions & 6 deletions test/helpers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@

const assume = require('assume'),
fs = require('fs'),
path = require('path'),
through = require('through2'),
spawn = require('child_process').spawn,
stream = require('stream'),
util = require('util'),
winston = require('../../lib/winston'),
mockTransport = require('./mocks/mock-transport');

Expand Down Expand Up @@ -100,7 +98,7 @@ helpers.clearExceptions = function () {
*/
helpers.clearRejections = function () {
var listeners = process.listeners('unhandledRejection');
process.removeAllListeners('unhandledRejections');
process.removeAllListeners('unhandledRejection');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the real problem with the tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no 😢


return {
restore: function () {
Expand All @@ -125,9 +123,7 @@ helpers.throw = function (msg) {
* @param {String} msg Error mesage to use
*/
helpers.reject = function (msg) {
return new Promise((resolve, reject) => {
reject(msg);
});
return Promise.reject(msg);
};

/**
Expand Down
Loading