NEWRELIC-5279 refactored Error Collector to reduce complexity #1431
Conversation
Also fixed/added JSDocs
Codecov Report
@@ Coverage Diff @@
## main #1431 +/- ##
=======================================
Coverage 96.23% 96.23%
=======================================
Files 194 194
Lines 37635 37704 +69
Branches 23 23
=======================================
+ Hits 36217 36286 +69
Misses 1418 1418
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| * @param {Array} errorTrace list of error information | ||
| * @param {Transaction} transaction the transaction associated with the trace | ||
| */ | ||
| _maybeRecordErrorMetrics(errorTrace, transaction) { |
There was a problem hiding this comment.
line 384 and 385 can be combined right? I thought we had a rule eslint rule to catch that
There was a problem hiding this comment.
So we do have a rule, but it only kicks in if the logic doesn't change from the combining, in this case, the logic would change in that we would start logging "other" error metrics if the transaction doesn't exist. All that being said, I think it would be safe to combine regardless. Thoughts?
There was a problem hiding this comment.
never mind. this should stay. other has a special meaning within a transaction
|
really nice refactor overall. that collect method was doing so much before |
| expectedErrors | ||
| ) | ||
| } else if (isErroredTransaction) { | ||
| ;[collectedErrors, expectedErrors] = this._processTransactionErrors( |
There was a problem hiding this comment.
What are these semicolons doing here? Did eslint put them there?
There was a problem hiding this comment.
since we do not use semi-colons, this is necessary for destructuring of let variables. The AST parser does not know how to handle it without it
There was a problem hiding this comment.
ok, I don't think this is necessary. It's only needed when object destructuring
There was a problem hiding this comment.
I don't think this is an AST parser issue but how ESLint parses code without semi colons
Proposed Release Notes
Links
Closes NEWRELIC-5279
Details