Skip to content

Commit

Permalink
NEWRELIC-5279 refactored Error Collector to reduce complexity
Browse files Browse the repository at this point in the history 
Also fixed/added JSDocs
  • Loading branch information
@jmartin4563
jmartin4563 committed Nov 21, 2022
1 parent 1083c7e commit 8425344
Showing 1 changed file with 124 additions and 55 deletions.
179 changes: 124 additions & 55 deletions lib/errors/error-collector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@

'use strict'

/* eslint sonarjs/cognitive-complexity: ["error", 25] -- TODO: https://issues.newrelic.com/browse/NEWRELIC-5252 */

const errorsModule = require('./index')

const logger = require('../logger').child({ component: 'error_tracer' })
const urltils = require('../util/urltils')
const Exception = require('../errors').Exception
const Transaction = require('../transaction')
const errorHelper = require('./helper')
const createError = errorsModule.createError
const createEvent = errorsModule.createEvent
Expand Down Expand Up @@ -75,6 +74,7 @@ class ErrorCollector {
*
* @param {?Transaction} transaction -
* @param {Error} exception - The error to be checked.
* @returns {boolean} whether or not the exception has already been tracked
*/
_haveSeen(transaction, exception) {
const txId = transaction ? transaction.id : 'Unknown'
Expand Down Expand Up @@ -109,12 +109,78 @@ class ErrorCollector {
return false
}

/**
* Helper method for processing errors that are created with .noticeError()
*
* @param {Transaction} transaction the collected exception's transaction
* @param {number} collectedErrors the number of errors we've successfully .collect()-ed
* @returns {number} the new number of errors successfully .collect()-ed
*/
_processUserErrors(transaction, collectedErrors) {
if (transaction.userErrors.length) {
for (let i = 0; i < transaction.userErrors.length; i++) {
const exception = transaction.userErrors[i]
if (this.collect(transaction, exception)) {
++collectedErrors
}
}
}

return collectedErrors
}

/**
* Helper method for processing exceptions on the transaction (transaction.exceptions array)
*
* @param {Transaction} transaction the transaction being processed
* @param {number} collectedErrors the number of errors successfully .collect()-ed
* @param {number} expectedErrors the number of collected errors that were expected
* @returns {Array.<number>} the updated [collectedErrors, expectedErrors] numbers post-processing
*/
_processTransactionExceptions(transaction, collectedErrors, expectedErrors) {
for (let i = 0; i < transaction.exceptions.length; i++) {
const exception = transaction.exceptions[i]
if (this.collect(transaction, exception)) {
++collectedErrors
// if we could collect it, then check if expected
if (
urltils.isExpectedError(this.config, transaction.statusCode) ||
errorHelper.isExpectedException(transaction, exception.error, this.config, urltils)
) {
++expectedErrors
}
}
}

return [collectedErrors, expectedErrors]
}

/**
* Helper method for processing an inferred error based on Transaction metadata
*
* @param {Transaction} transaction the transaction being processed
* @param {number} collectedErrors the number of errors successfully .collect()-ed
* @param {number} expectedErrors the number of collected errors that were expected
* @returns {Array.<number>} the updated [collectedErrors, expectedErrors] numbers post-processing
*/
_processTransactionErrors(transaction, collectedErrors, expectedErrors) {
if (this.collect(transaction)) {
++collectedErrors
if (urltils.isExpectedError(this.config, transaction.statusCode)) {
++expectedErrors
}
}

return [collectedErrors, expectedErrors]
}

/**
* Every finished transaction goes through this handler, so do as little as
* possible.
*
* @param {Transaction} transaction
* @returns {number} The number of unexpected errors
* TODO: Prob shouldn't do any work if errors fully disabled.
*
* @param {Transaction} transaction the completed transaction
*/
onTransactionFinished(transaction) {
if (!transaction) {
Expand All @@ -124,48 +190,30 @@ class ErrorCollector {
return
}

// TODO: Prob shouldn't do any work if errors fully disabled.

// collect user errors even if status code is ignored
let collectedErrors = 0
let expectedErrors = 0

// errors from noticeError are currently exempt from
// ignore and exclude rules
if (transaction.userErrors.length) {
for (let i = 0; i < transaction.userErrors.length; i++) {
const exception = transaction.userErrors[i]
if (this.collect(transaction, exception)) {
++collectedErrors
}
}
}
collectedErrors = this._processUserErrors(transaction, collectedErrors)

const isErroredTransaction = urltils.isError(this.config, transaction.statusCode)
const isIgnoredErrorStatusCode = urltils.isIgnoredError(this.config, transaction.statusCode)

const isExpectedErrorStatusCode = urltils.isExpectedError(this.config, transaction.statusCode)

// collect other exceptions only if status code is not ignored
if (transaction.exceptions.length && !isIgnoredErrorStatusCode) {
for (let i = 0; i < transaction.exceptions.length; i++) {
const exception = transaction.exceptions[i]
if (this.collect(transaction, exception)) {
++collectedErrors
// if we could collect it, then check if expected
if (
isExpectedErrorStatusCode ||
errorHelper.isExpectedException(transaction, exception.error, this.config, urltils)
) {
++expectedErrors
}
}
}
} else if (isErroredTransaction && this.collect(transaction)) {
++collectedErrors
if (isExpectedErrorStatusCode) {
++expectedErrors
}
;[collectedErrors, expectedErrors] = this._processTransactionExceptions(
transaction,
collectedErrors,
expectedErrors
)
} else if (isErroredTransaction) {
;[collectedErrors, expectedErrors] = this._processTransactionErrors(
transaction,
collectedErrors,
expectedErrors
)
}

const unexpectedErrors = collectedErrors - expectedErrors
Expand Down Expand Up @@ -226,9 +274,8 @@ class ErrorCollector {
* NOTE: this interface is unofficial and may change in future.
*
* @param {?Transaction} transaction Transaction associated with the error.
* @param {Exception} exception The Exception to be traced.
* @param error
* @param customAttributes
* @param {*} error The error passed into `API#noticeError()`
* @param {object} customAttributes custom attributes to add to the error
*/
addUserError(transaction, error, customAttributes) {
if (!error) {
Expand Down Expand Up @@ -263,13 +310,38 @@ class ErrorCollector {
*
* @param {?Transaction} transaction Transaction associated with the error.
* @param {?Exception} exception The Exception object to be traced.
* @returns {bool} True if the error was collected.
* @returns {boolean} True if the error was collected.
*/
collect(transaction, exception) {
if (!exception) {
exception = new Exception({})
collect(transaction, exception = new Exception({})) {
if (!this._isValidException(exception, transaction)) {
return false
}

const errorTrace = createError(transaction, exception, this.config)
this._maybeRecordErrorMetrics(errorTrace, transaction)

// defaults true in config/index. can be modified server-side
if (this.config.collect_errors) {
this.traceAggregator.add(errorTrace)
}

if (this.config.error_collector.capture_events === true) {
const priority = (transaction && transaction.priority) || Math.random()
const event = createEvent(transaction, errorTrace, exception.timestamp, this.config)
this.eventAggregator.add(event, priority)
}

return true
}

/**
* Helper method for ensuring that a collected exception/transaction combination can be collected
*
* @param {object} exception the exception to validate
* @param {Transaction} transaction the Transaction to validate, if exception is malformed we'll try to fallback to transaction data
* @returns {boolean} whether or not the exception/transaction combo has everything needed for processing
*/
_isValidException(exception, transaction) {
if (exception.error) {
if (this._haveSeen(transaction, exception.error)) {
return false
Expand All @@ -288,10 +360,20 @@ class ErrorCollector {

if (exception.error) {
logger.trace(exception.error, 'Got exception to trace:')
} else {
logger.trace(transaction, 'Got transaction error to trace:')
}

const errorTrace = createError(transaction, exception, this.config)
return true
}

/**
* Helper method for recording metrics about errors depending on the type of error that happened
*
* @param {Array} errorTrace list of error information
* @param {Transaction} transaction the transaction associated with the trace
*/
_maybeRecordErrorMetrics(errorTrace, transaction) {
const isExpectedError = true === errorTrace[4].intrinsics['error.expected']

if (isExpectedError) {
Expand All @@ -307,19 +389,6 @@ class ErrorCollector {
}
}
}

// defaults true in config/index. can be modified server-side
if (this.config.collect_errors) {
this.traceAggregator.add(errorTrace)
}

if (this.config.error_collector.capture_events === true) {
const priority = (transaction && transaction.priority) || Math.random()
const event = createEvent(transaction, errorTrace, exception.timestamp, this.config)
this.eventAggregator.add(event, priority)
}

return true
}

// TODO: ideally, this becomes unnecessary
Expand Down

0 comments on commit 8425344

Please sign in to comment.