Skip to content

Commit

Permalink
chore: removed uncaught exception config
Browse files Browse the repository at this point in the history
  • Loading branch information
kirrg001 authored and kirrg001 committed Dec 7, 2021
1 parent 722bbff commit e9e27c2
Show file tree
Hide file tree
Showing 15 changed files with 11 additions and 759 deletions.
4 changes: 1 addition & 3 deletions dockerfile-examples/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@

'use strict';

require('@instana/collector')({
reportUncaughtException: true
});
require('@instana/collector')();

const http = require('http');

Expand Down
71 changes: 0 additions & 71 deletions packages/collector/src/agentConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const fs = require('fs');
const { http } = require('@instana/core').uninstrumentedHttp;
const pathUtil = require('path');
const { propertySizes } = require('@instana/core').util;
const childProcess = require('child_process');

/** @typedef {import('@instana/core/src/tracing/cls').InstanaBaseSpan} InstanaBaseSpan */

Expand Down Expand Up @@ -414,76 +413,6 @@ function sendData(path, data, cb, ignore404 = false) {
req.end();
}

/**
* Sends the given event data and trace data synchronously to the agent via HTTP. This function is synchronous, that is,
* it blocks the event loop!
*
* YOU MUST NOT USE THIS FUNCTION, except for the one use case where it is actually required to block the event loop
* (reporting an uncaught exception tot the agent in the process.on('uncaughtException') handler).
*
* @param {*} eventData
* @param {Array.<InstanaBaseSpan>} spans
*/
exports.reportUncaughtExceptionToAgentSync = function reportUncaughtExceptionToAgentSync(eventData, spans) {
sendRequestsSync(
'/com.instana.plugin.generic.event',
eventData,
`/com.instana.plugin.nodejs/traces.${pidStore.pid}`,
spans
);
};

/**
* Sends two HTTP POST requests to the agent. This function is synchronous, that is, it blocks the event loop!
*
* YOU MUST NOT USE THIS FUNCTION, except for the one use case where it is actually required to block the event loop
* (reporting an uncaught exception tot the agent in the process.on('uncaughtException') handler).
*
* @param {string} path1
* @param {Object} data1
* @param {string} path2
* @param {Object} data2
*/
function sendRequestsSync(path1, data1, path2, data2) {
let port = agentOpts.port;
if (typeof port !== 'number') {
try {
port = parseInt(port, 10);
} catch (nonNumericPortError) {
logger.warn('Detected non-numeric port configuration value %s, uncaught exception will not be reported.', port);
return;
}
}

let payload1;
let payload2;
try {
payload1 = JSON.stringify(data1);
payload2 = JSON.stringify(data2);
} catch (payloadSerializationError) {
logger.warn('Could not serialize payload, uncaught exception will not be reported.', {
error: payloadSerializationError
});
return;
}

try {
childProcess.execFileSync(process.execPath, [pathUtil.join(__dirname, 'uncaught', 'syncHttp.js')], {
env: {
INSTANA_AGENT_HOST: agentOpts.host,
INSTANA_AGENT_PORT: String(agentOpts.port),
PATH1: path1,
PAYLOAD1: payload1,
PATH2: path2,
PAYLOAD2: payload2
},
timeout: 400
});
} catch (error) {
logger.warn('Failed to report uncaught exception due to network error.', { error });
}
}

exports.isConnected = function () {
return isConnected;
};
Expand Down
114 changes: 1 addition & 113 deletions packages/collector/src/uncaught/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,13 @@ logger = require('../logger').getLogger('util/uncaughtExceptionHandler', newLogg
logger = newLogger;
});

const { tracing, util } = require('@instana/core');
const spanBuffer = tracing.spanBuffer;
const stackTraceUtil = util.stackTrace;

/** @type {import('../agentConnection')} */
let downstreamConnection = null;
/** @type {import('../pidStore')} */
let processIdentityProvider = null;
const uncaughtExceptionEventName = 'uncaughtException';
const unhandledRejectionEventName = 'unhandledRejection';
let unhandledRejectionDeprecationWarningHasBeenEmitted = false;
const stackTraceLength = 10;

/** @type {import('../util/normalizeConfig').CollectorConfig} */
let config;

Expand Down Expand Up @@ -61,123 +56,16 @@ exports.init = function (_config, _downstreamConnection, _processIdentityProvide
config = _config;
downstreamConnection = _downstreamConnection;
processIdentityProvider = _processIdentityProvider;
if (config.reportUncaughtException) {
if (!processIdentityProvider) {
logger.warn('Reporting uncaught exceptions is enabled, but no process identity provider is available.');
} else if (typeof processIdentityProvider.getEntityId !== 'function') {
logger.warn(
'Reporting uncaught exceptions is enabled, but the process identity provider does not support ' +
'retrieving an entity ID.'
);
}
}
};

exports.activate = function () {
activateUncaughtExceptionHandling();
activateUnhandledPromiseRejectionHandling();
};

function activateUncaughtExceptionHandling() {
if (config.reportUncaughtException) {
process.once(uncaughtExceptionEventName, onUncaughtException);
logger.warn(
'Reporting uncaught exceptions is enabled. This feature is deprecated. Please consider disabling it ' +
'and rely on https://www.instana.com/docs/ecosystem/os-process/#abnormal-termination instead.'
);
if (process.version === 'v12.6.0') {
logger.warn(
'You are running Node.js v12.6.0 and have enabled reporting uncaught exceptions. To enable this feature, ' +
'@instana/collector will register an uncaughtException handler. Due to a bug in Node.js v12.6.0, the ' +
'original stack trace will get lost when this process is terminated with an uncaught exception. ' +
'Instana recommends to use a different Node.js version (<= v12.5.0 or >= v12.6.1). See ' +
'https://github.com/nodejs/node/issues/28550 for details.'
);
}
} else {
logger.info('Reporting uncaught exceptions is disabled.');
}
}

exports.deactivate = function () {
process.removeListener(uncaughtExceptionEventName, onUncaughtException);
process.removeListener(unhandledRejectionEventName, onUnhandledRejection);
};

/**
* @param {InstanaExtendedError} uncaughtError
*/
function onUncaughtException(uncaughtError) {
// because of the way Error.prepareStackTrace works and how error.stack is only created once and then cached it is
// important to create the JSON formatted stack trace first, before anything else accesses error.stack.
const jsonStackTrace =
uncaughtError != null ? stackTraceUtil.getStackTraceAsJson(stackTraceLength, uncaughtError) : null;
finishCurrentSpanAndReportEvent(uncaughtError, jsonStackTrace);
logAndRethrow(uncaughtError);
}

/**
* @param {InstanaExtendedError} uncaughtError
* @param {Array.<import('@instana/core/src/util/stackTrace').InstanaCallSite>} jsonStackTrace
*/
function finishCurrentSpanAndReportEvent(uncaughtError, jsonStackTrace) {
const spans = finishCurrentSpan(jsonStackTrace);
const eventPayload = createEventForUncaughtException(uncaughtError);
downstreamConnection.reportUncaughtExceptionToAgentSync(eventPayload, spans);
}

/**
* @param {Error} uncaughtError
* @returns {import('../agentConnection').AgentConnectionEvent}
*/
function createEventForUncaughtException(uncaughtError) {
return createEvent(uncaughtError, 'A Node.js process terminated abnormally due to an uncaught exception.', 10, false);
}

/**
* @param {Array.<import('@instana/core/src/util/stackTrace').InstanaCallSite>} jsonStackTrace
* @returns {Array.<import('@instana/core/src/tracing/cls').InstanaBaseSpan>}
*/
function finishCurrentSpan(jsonStackTrace) {
const cls = tracing.getCls();
if (!cls) {
return [];
}
const currentSpan = cls.getCurrentSpan();
if (!currentSpan) {
return [];
}

currentSpan.ec = 1;
currentSpan.d = Date.now() - currentSpan.ts;
if (jsonStackTrace) {
currentSpan.stack = jsonStackTrace;
}
currentSpan.transmit();
return spanBuffer.getAndResetSpans();
}

/**
* @param {Error} err
*/
function logAndRethrow(err) {
// Remove all listeners now, so the final throw err won't trigger other registered listeners a second time.
const registeredListeners = process.listeners(uncaughtExceptionEventName);
if (registeredListeners) {
registeredListeners.forEach(listener => {
process.removeListener(uncaughtExceptionEventName, listener);
});
}
// prettier-ignore
// eslint-disable-next-line max-len
logger.error('The Instana Node.js collector caught an otherwise uncaught exception to generate a respective Instana event for you. Instana will now rethrow the error to terminate this process, otherwise the application would be left in an inconsistent state, see https://nodejs.org/api/process.html#process_warning_using_uncaughtexception_correctly. The next line on stderr will look as if Instana crashed your application, but actually the original error came from your application code, not from Instana. Since we rethrow the original error, you should see its stacktrace below (depening on how you operate your application and how logging is configured.)');

// Rethrow the original error (after notifying the agent) to trigger the process to finally terminate - Node won't
// run this handler again since it (a) has been registered with `once` and (b) we removed all handlers for
// uncaughtException anyway.
throw err;
}

function activateUnhandledPromiseRejectionHandling() {
if (config.reportUnhandledPromiseRejections) {
if (unhandledRejectionsMode === 'strict') {
Expand Down
19 changes: 5 additions & 14 deletions packages/collector/src/util/normalizeConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const defaults = {
* @property {string} [agentHost]
* @property {Object.<string, *>} [tracing]
* @property {boolean | string} [autoProfile]
* @property {boolean} [reportUncaughtException]
* @property {boolean} [reportUnhandledPromiseRejections]
* @property {import('@instana/core/src/logger').GenericLogger} [logger]
* @property {string | number} [level]
Expand All @@ -38,26 +37,18 @@ module.exports = function normalizeConfig(config = {}) {
config.agentPort = config.agentPort || parseToPositiveInteger(process.env.INSTANA_AGENT_PORT, defaults.agentPort);
config.autoProfile = config.autoProfile || process.env.INSTANA_AUTO_PROFILE || defaults.autoProfile;

normalizeConfigForUncaughtExceptions(config);

return config;
};

/**
* @param {CollectorConfig} config
*/
function normalizeConfigForUncaughtExceptions(config) {
config.tracing = config.tracing || {};

if (config.tracing.stackTraceLength == null) {
config.tracing.stackTraceLength = defaults.tracing.stackTraceLength;
}
config.reportUncaughtException = config.reportUncaughtException === true;
// Make reportUncaughtException imply reportUnhandledPromiseRejections, unless explicitly disabled.

if (config.reportUnhandledPromiseRejections == null) {
config.reportUnhandledPromiseRejections = config.reportUncaughtException;
config.reportUnhandledPromiseRejections = false;
}
}

return config;
};

/**
* @param {string | number} value
Expand Down
3 changes: 1 addition & 2 deletions packages/collector/test/nativeModuleRetry/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ instana.sharedMetrics.util.nativeModuleRetry.selfNodeModulesPath = require('path
'node_modules'
);
instana({
level: 'debug',
reportUncaughtException: true
level: 'debug'
});

const http = require('http');
Expand Down
36 changes: 0 additions & 36 deletions packages/collector/test/uncaught/apps/controls.js

This file was deleted.

31 changes: 0 additions & 31 deletions packages/collector/test/uncaught/apps/order.js

This file was deleted.

24 changes: 0 additions & 24 deletions packages/collector/test/uncaught/apps/rethrow.js

This file was deleted.

Loading

0 comments on commit e9e27c2

Please sign in to comment.