Error stacktrace overhaul #1944
Conversation
Codecov Report
@@ Coverage Diff @@
## 2-dev #1944 +/- ##
==========================================
+ Coverage 93.76% 93.77% +0.01%
==========================================
Files 120 121 +1
Lines 7572 7604 +32
==========================================
+ Hits 7100 7131 +31
- Misses 472 473 +1
Continue to review full report at Codecov.
|
Co-authored-by: Michele Leo <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
| || (line.indexOf('at /') === -1 && line.charAt(line.indexOf('(') + 1) !== '/') // ignore node internal | ||
| || line.includes('node_modules') // ignore dependencies | ||
| ) { | ||
| return ' ' + line; |
There was a problem hiding this comment.
Why do we include the line if the comments say it is ignored?
There was a problem hiding this comment.
Because it's an internal line from Node.js or from Kuzzle so we keep it either way because it's a bad pratice to hide stack calls (for us it's more difficult to debug Kuzzle bug, for users it can lead to confusion)
| * > at init (/home/aschen/projets/app/test.ts:8:3) | ||
| * at Module._compile (internal/modules/cjs/loader.js:1133:30) | ||
| */ | ||
| function hilightUserCode (line) { |
There was a problem hiding this comment.
Correct me if I'm wrong but, rather than highlighting user code, this function filters out system code. #renaming #nitpicking
There was a problem hiding this comment.
Nop, it does not remove any line from the stacktrace
| else if (data && data.content && data.content.error) { | ||
| delete data.content.error.stack; | ||
| else { | ||
| data.content.error.stack = data.content.error.stack.split('\n').map(hilightUserCode).join('\n'); |
There was a problem hiding this comment.
data.content.error.stack = data.content.error.stack.split('\n').map(hilightUserCode).join('\n');Maybe having a function to do this would be better instead of repeating this bit of code everywhere ?
There was a problem hiding this comment.
I can't because I use this method in a custom way in the kerror/index.js file
# [2.9.2](https://github.com/kuzzleio/kuzzle/releases/tag/2.9.2) (2021-01-29) #### Bug fixes - [ [#1938](#1938) ] Export BackendCluster class ([Aschen](https://github.com/Aschen)) #### Enhancements - [ [#1944](#1944) ] Error stacktrace overhaul ([Aschen](https://github.com/Aschen)) - [ [#1946](#1946) ] Allows to not send realtime notification on document controller ([Aschen](https://github.com/Aschen)) ---
## What does this PR do ? Work had been done on stacktrace a while ago to enhance them so developers can distinguish there code from Kuzzle one. (See #1944) Few problems remains: - `hilightUserCode` was called many times on the same stack - errors thrown by the SDK in a pipe did not include the line with the code that triggered the error This fix cover the following additional use cases: ``` app.pipe.register('server:afterNow', async req => { await app.sdk.collection.list('do-not-exist'); }); app.pipe.register('server:afterInfo', async req => { throw new BadRequestError('afterInfo'); }); app.pipe.register('server:afterGetStats', async function afterGetStats (req) { await app.sdk.collection.list('do-not-exist'); }); app.pipe.register('server:afterMetrics', async function afterMetrics (req) { throw new BadRequestError('afterMetrics'); }); ``` ### Other changes - remove useless await in FunnelProtocol + convert to Typescript - extract `removeErrorStack` into `util` directory
What does this PR do ?
Highlight user code in internal stacktrace.
How should this be manually tested?
The following code will generate the following stacktrace:
stack:apiErrorstack:jsErrorstack:sdkErrorOther Changes
process.envat runtime because it does an expensive syscall (CF: Cache accesses to process.env? nodejs/node#3104 (comment))Boyscout
Globalin a TS file to avoid the necessity of building the app to run it