From c78364e8e13f69030297a2904a329d99f8b15748 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Fri, 19 Jan 2018 01:02:27 +0100 Subject: [PATCH 1/8] Adding addtional option stack to control the stack --- README.md | 4 ++++ src/WorkerPool.js | 7 ++++++- src/workerPools.js | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 7888c69..8d04e83 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,10 @@ use: [ // name of the pool // can be used to create different pools with elsewise identical options name: "my-pool" + + // add additional stack trace to the error + // defaults to true + stack: true } }, "expensive-loader" diff --git a/src/WorkerPool.js b/src/WorkerPool.js index 17bb761..0f6c4bb 100644 --- a/src/WorkerPool.js +++ b/src/WorkerPool.js @@ -11,6 +11,7 @@ let workerId = 0; class PoolWorker { constructor(options, onJobDone) { + this.options = options; this.nextJobId = 0; this.jobs = Object.create(null); this.activeJobs = 0; @@ -186,7 +187,11 @@ class PoolWorker { const err = new Error(obj.message); err.message = obj.message; if (obj.stack) { - err.stack = `${obj.stack}\n\tfrom thread-loader (worker ${this.id})\n${err.stack}`; + let additionalStack = ''; + if (this.options.stack) { + additionalStack = `\n\tfrom thread-loader (worker ${this.id})\n${err.stack}`; + } + err.stack = `${obj.stack}${additionalStack}`; } err.hideStack = obj.hideStack; err.details = obj.details; diff --git a/src/workerPools.js b/src/workerPools.js index 3a81b7a..5700ac1 100644 --- a/src/workerPools.js +++ b/src/workerPools.js @@ -11,6 +11,7 @@ function getPool(options) { workerParallelJobs: options.workerParallelJobs || 20, poolTimeout: options.poolTimeout || 500, poolParallelJobs: options.poolParallelJobs || 200, + stack: options.stack || true, }; const tpKey = JSON.stringify(workerPoolOptions); workerPools[tpKey] = workerPools[tpKey] || new WorkerPool(workerPoolOptions); From 23c884e17cee228a3092fca5e698ea2c8cc97471 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Fri, 19 Jan 2018 15:45:39 +0100 Subject: [PATCH 2/8] revamping the stack trace with more better options --- src/WorkerPool.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/WorkerPool.js b/src/WorkerPool.js index 0f6c4bb..804a64f 100644 --- a/src/WorkerPool.js +++ b/src/WorkerPool.js @@ -187,11 +187,19 @@ class PoolWorker { const err = new Error(obj.message); err.message = obj.message; if (obj.stack) { - let additionalStack = ''; + err.stack = `${obj.stack}\n\tThread Loader (Worker ${this.id})`; if (this.options.stack) { - additionalStack = `\n\tfrom thread-loader (worker ${this.id})\n${err.stack}`; + // Limit the err.stack manually in case stackTraces are to verbose + if (typeof this.options.stack === 'number') { + Error.stackTraceLimit = (this.options.stack); + } + + err.stack += `\n\n${err.stack}`; + } else { + // Only the err.message + const message = stack => stack.split('\n')[0]; + err.stack += `\n\n${message(err.stack)}`; } - err.stack = `${obj.stack}${additionalStack}`; } err.hideStack = obj.hideStack; err.details = obj.details; @@ -210,6 +218,7 @@ class PoolWorker { export default class WorkerPool { constructor(options) { this.options = options || {}; + this.stack = options.stack || false; this.numberOfWorkers = options.numberOfWorkers; this.poolTimeout = options.poolTimeout; this.workerNodeArgs = options.workerNodeArgs; @@ -248,6 +257,7 @@ export default class WorkerPool { createWorker() { // spin up a new worker const newWorker = new PoolWorker({ + stack: this.stack, nodeArgs: this.workerNodeArgs, parallelJobs: this.workerParallelJobs, }, () => this.onJobDone()); @@ -277,4 +287,3 @@ export default class WorkerPool { } } } - From 0589078e28ad7702b280958bf467c21298508160 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Sat, 20 Jan 2018 23:54:13 +0100 Subject: [PATCH 3/8] making the stack to false by default --- README.md | 4 ++-- src/WorkerPool.js | 14 +++----------- src/workerPools.js | 2 +- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 8d04e83..63d02f7 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,8 @@ use: [ name: "my-pool" // add additional stack trace to the error - // defaults to true - stack: true + // defaults to false + stack: false } }, "expensive-loader" diff --git a/src/WorkerPool.js b/src/WorkerPool.js index 804a64f..ce7dac5 100644 --- a/src/WorkerPool.js +++ b/src/WorkerPool.js @@ -185,20 +185,12 @@ class PoolWorker { obj = arg; } const err = new Error(obj.message); - err.message = obj.message; if (obj.stack) { - err.stack = `${obj.stack}\n\tThread Loader (Worker ${this.id})`; + const objStack = `${obj.stack}\n\tThread Loader (Worker ${this.id})`; if (this.options.stack) { - // Limit the err.stack manually in case stackTraces are to verbose - if (typeof this.options.stack === 'number') { - Error.stackTraceLimit = (this.options.stack); - } - - err.stack += `\n\n${err.stack}`; + err.stack += `${objStack}\n\n${err.stack}`; } else { - // Only the err.message - const message = stack => stack.split('\n')[0]; - err.stack += `\n\n${message(err.stack)}`; + err.stack = `${objStack}`; } } err.hideStack = obj.hideStack; diff --git a/src/workerPools.js b/src/workerPools.js index 5700ac1..eeb96f3 100644 --- a/src/workerPools.js +++ b/src/workerPools.js @@ -11,7 +11,7 @@ function getPool(options) { workerParallelJobs: options.workerParallelJobs || 20, poolTimeout: options.poolTimeout || 500, poolParallelJobs: options.poolParallelJobs || 200, - stack: options.stack || true, + stack: options.stack || false, }; const tpKey = JSON.stringify(workerPoolOptions); workerPools[tpKey] = workerPools[tpKey] || new WorkerPool(workerPoolOptions); From d0692fdbbf3a0fbf68c55d938c66fe40f4ee2d0b Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Sun, 21 Jan 2018 15:13:20 +0100 Subject: [PATCH 4/8] Removing flag and just trimming the duplicate message --- README.md | 4 ---- src/WorkerPool.js | 15 +++++++-------- src/workerPools.js | 1 - 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 63d02f7..7888c69 100644 --- a/README.md +++ b/README.md @@ -84,10 +84,6 @@ use: [ // name of the pool // can be used to create different pools with elsewise identical options name: "my-pool" - - // add additional stack trace to the error - // defaults to false - stack: false } }, "expensive-loader" diff --git a/src/WorkerPool.js b/src/WorkerPool.js index ce7dac5..5298143 100644 --- a/src/WorkerPool.js +++ b/src/WorkerPool.js @@ -185,13 +185,14 @@ class PoolWorker { obj = arg; } const err = new Error(obj.message); + err.message = obj.message; if (obj.stack) { - const objStack = `${obj.stack}\n\tThread Loader (Worker ${this.id})`; - if (this.options.stack) { - err.stack += `${objStack}\n\n${err.stack}`; - } else { - err.stack = `${objStack}`; - } + const trim = (errorObj) => { + const { stack, message } = errorObj; + const traceIndex = stack.indexOf(message) + message.length; + return stack.substr(traceIndex); + }; + err.stack = `${obj.stack}\n\tfrom thread-loader (worker: ${this.id})\n${trim(err)}`; } err.hideStack = obj.hideStack; err.details = obj.details; @@ -210,7 +211,6 @@ class PoolWorker { export default class WorkerPool { constructor(options) { this.options = options || {}; - this.stack = options.stack || false; this.numberOfWorkers = options.numberOfWorkers; this.poolTimeout = options.poolTimeout; this.workerNodeArgs = options.workerNodeArgs; @@ -249,7 +249,6 @@ export default class WorkerPool { createWorker() { // spin up a new worker const newWorker = new PoolWorker({ - stack: this.stack, nodeArgs: this.workerNodeArgs, parallelJobs: this.workerParallelJobs, }, () => this.onJobDone()); diff --git a/src/workerPools.js b/src/workerPools.js index eeb96f3..3a81b7a 100644 --- a/src/workerPools.js +++ b/src/workerPools.js @@ -11,7 +11,6 @@ function getPool(options) { workerParallelJobs: options.workerParallelJobs || 20, poolTimeout: options.poolTimeout || 500, poolParallelJobs: options.poolParallelJobs || 200, - stack: options.stack || false, }; const tpKey = JSON.stringify(workerPoolOptions); workerPools[tpKey] = workerPools[tpKey] || new WorkerPool(workerPoolOptions); From 111ed758b846a28ca40d520301a76deed1812163 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Mon, 22 Jan 2018 23:32:17 +0100 Subject: [PATCH 5/8] fixing the review comments --- src/WorkerPool.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/WorkerPool.js b/src/WorkerPool.js index 5298143..7fe521e 100644 --- a/src/WorkerPool.js +++ b/src/WorkerPool.js @@ -11,7 +11,6 @@ let workerId = 0; class PoolWorker { constructor(options, onJobDone) { - this.options = options; this.nextJobId = 0; this.jobs = Object.create(null); this.activeJobs = 0; @@ -187,12 +186,13 @@ class PoolWorker { const err = new Error(obj.message); err.message = obj.message; if (obj.stack) { - const trim = (errorObj) => { - const { stack, message } = errorObj; - const traceIndex = stack.indexOf(message) + message.length; - return stack.substr(traceIndex); + const trim = (error) => { + const { stack, message } = error; + const trace = stack.indexOf(message) + message.length; + return stack.substr(trace); }; - err.stack = `${obj.stack}\n\tfrom thread-loader (worker: ${this.id})\n${trim(err)}`; + + err.stack = `${obj.stack}\nThread Loader (Worker ${this.id})\n\n${trim(err)}`; } err.hideStack = obj.hideStack; err.details = obj.details; From 62f487145aff000b4872bce2c92f9b5f9c9a8fea Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Wed, 7 Feb 2018 14:57:46 +0100 Subject: [PATCH 6/8] moving the error into workererror --- package-lock.json | 56 +++++++++++++++++++++++----------------------- src/WorkerError.js | 31 +++++++++++++++++++++++++ src/WorkerPool.js | 16 ++----------- 3 files changed, 61 insertions(+), 42 deletions(-) create mode 100644 src/WorkerError.js diff --git a/package-lock.json b/package-lock.json index 2d6b975..abd22ff 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4,6 +4,16 @@ "lockfileVersion": 1, "requires": true, "dependencies": { + "JSONStream": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.1.tgz", + "integrity": "sha1-cH92HgHa6eFvG8+TcDt4xwlmV5o=", + "dev": true, + "requires": { + "jsonparse": "1.3.1", + "through": "2.3.8" + } + }, "abab": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/abab/-/abab-1.0.4.tgz", @@ -1933,8 +1943,8 @@ "integrity": "sha512-8od6g684Fhi5Vpp4ABRv/RBsW1AY6wSHbJHEK6FGTv+8jvAAnlABniZu/FVmX9TcirkHepaEsa1QGkRvbg0CKw==", "dev": true, "requires": { - "is-text-path": "1.0.1", "JSONStream": "1.3.1", + "is-text-path": "1.0.1", "lodash": "4.17.4", "meow": "3.7.0", "split2": "2.2.0", @@ -3933,14 +3943,6 @@ } } }, - "string_decoder": { - "version": "1.0.1", - "bundled": true, - "dev": true, - "requires": { - "safe-buffer": "5.0.1" - } - }, "string-width": { "version": "1.0.2", "bundled": true, @@ -3951,6 +3953,14 @@ "strip-ansi": "3.0.1" } }, + "string_decoder": { + "version": "1.0.1", + "bundled": true, + "dev": true, + "requires": { + "safe-buffer": "5.0.1" + } + }, "stringstream": { "version": "0.0.5", "bundled": true, @@ -5479,16 +5489,6 @@ "integrity": "sha1-T9kss04OnbPInIYi7PUfm5eMbLk=", "dev": true }, - "JSONStream": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.1.tgz", - "integrity": "sha1-cH92HgHa6eFvG8+TcDt4xwlmV5o=", - "dev": true, - "requires": { - "jsonparse": "1.3.1", - "through": "2.3.8" - } - }, "jsprim": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/jsprim/-/jsprim-1.4.1.tgz", @@ -9207,15 +9207,6 @@ "integrity": "sha1-J5siXfHVgrH1TmWt3UNS4Y+qBxM=", "dev": true }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", - "dev": true, - "requires": { - "safe-buffer": "5.1.1" - } - }, "string-length": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/string-length/-/string-length-1.0.1.tgz", @@ -9236,6 +9227,15 @@ "strip-ansi": "3.0.1" } }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", + "dev": true, + "requires": { + "safe-buffer": "5.1.1" + } + }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", diff --git a/src/WorkerError.js b/src/WorkerError.js new file mode 100644 index 0000000..d670b9d --- /dev/null +++ b/src/WorkerError.js @@ -0,0 +1,31 @@ +const stack = (origin, worker, workerId) => { + const originError = origin + .split('\n') + .filter(line => line.includes('at')); + + const workerError = worker + .split('\n') + .filter(line => line.includes('at')) + .map(line => `${line} [Thread Loader (Worker ${workerId})]`); + + const diff = workerError.slice(0, workerError.length - originError.length).join('\n'); + + originError.unshift(diff); + + return originError.join('\n'); +}; + +class WorkerError extends Error { + constructor(err, workerId) { + super(err); + this.name = err.name; + this.message = err.message; + + Error.captureStackTrace(this, this.constructor); + + this.stack = stack(err.stack, this.stack, workerId); + // this.stack = [`Thread Loader (Worker ${workerId})`, err.message, this.stack].join('\n'); + } +} + +export default WorkerError; diff --git a/src/WorkerPool.js b/src/WorkerPool.js index 7fe521e..0fd4406 100644 --- a/src/WorkerPool.js +++ b/src/WorkerPool.js @@ -4,6 +4,7 @@ import childProcess from 'child_process'; import asyncQueue from 'async/queue'; import asyncMapSeries from 'async/mapSeries'; import readBuffer from './readBuffer'; +import WorkerError from './WorkerError'; const workerPath = require.resolve('./worker'); @@ -183,20 +184,7 @@ class PoolWorker { } else { obj = arg; } - const err = new Error(obj.message); - err.message = obj.message; - if (obj.stack) { - const trim = (error) => { - const { stack, message } = error; - const trace = stack.indexOf(message) + message.length; - return stack.substr(trace); - }; - - err.stack = `${obj.stack}\nThread Loader (Worker ${this.id})\n\n${trim(err)}`; - } - err.hideStack = obj.hideStack; - err.details = obj.details; - return err; + return new WorkerError(obj, this.id); } readBuffer(length, callback) { From a2f3c199ec96f53159ca63aaedd7f999e3497e00 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Wed, 7 Feb 2018 15:59:39 +0100 Subject: [PATCH 7/8] handling trim --- src/WorkerError.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/WorkerError.js b/src/WorkerError.js index d670b9d..e2b9607 100644 --- a/src/WorkerError.js +++ b/src/WorkerError.js @@ -1,16 +1,17 @@ -const stack = (origin, worker, workerId) => { - const originError = origin +const stack = (err, worker, workerId) => { + const originError = err.stack .split('\n') - .filter(line => line.includes('at')); + .filter(line => line.trim().startsWith('at')); const workerError = worker .split('\n') - .filter(line => line.includes('at')) - .map(line => `${line} [Thread Loader (Worker ${workerId})]`); + .filter(line => line.trim().startsWith('at')); const diff = workerError.slice(0, workerError.length - originError.length).join('\n'); originError.unshift(diff); + originError.unshift(err.message); + originError.unshift(`Thread Loader (Worker ${workerId})`); return originError.join('\n'); }; @@ -23,8 +24,7 @@ class WorkerError extends Error { Error.captureStackTrace(this, this.constructor); - this.stack = stack(err.stack, this.stack, workerId); - // this.stack = [`Thread Loader (Worker ${workerId})`, err.message, this.stack].join('\n'); + this.stack = stack(err, this.stack, workerId); } } From ce61e12a381dc9b2b7672d89e7aefe1fda57c816 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Wed, 7 Feb 2018 16:28:27 +0100 Subject: [PATCH 8/8] revert package-lock file --- package-lock.json | 56 +++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/package-lock.json b/package-lock.json index abd22ff..2d6b975 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4,16 +4,6 @@ "lockfileVersion": 1, "requires": true, "dependencies": { - "JSONStream": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.1.tgz", - "integrity": "sha1-cH92HgHa6eFvG8+TcDt4xwlmV5o=", - "dev": true, - "requires": { - "jsonparse": "1.3.1", - "through": "2.3.8" - } - }, "abab": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/abab/-/abab-1.0.4.tgz", @@ -1943,8 +1933,8 @@ "integrity": "sha512-8od6g684Fhi5Vpp4ABRv/RBsW1AY6wSHbJHEK6FGTv+8jvAAnlABniZu/FVmX9TcirkHepaEsa1QGkRvbg0CKw==", "dev": true, "requires": { - "JSONStream": "1.3.1", "is-text-path": "1.0.1", + "JSONStream": "1.3.1", "lodash": "4.17.4", "meow": "3.7.0", "split2": "2.2.0", @@ -3943,22 +3933,22 @@ } } }, - "string-width": { - "version": "1.0.2", + "string_decoder": { + "version": "1.0.1", "bundled": true, "dev": true, "requires": { - "code-point-at": "1.1.0", - "is-fullwidth-code-point": "1.0.0", - "strip-ansi": "3.0.1" + "safe-buffer": "5.0.1" } }, - "string_decoder": { - "version": "1.0.1", + "string-width": { + "version": "1.0.2", "bundled": true, "dev": true, "requires": { - "safe-buffer": "5.0.1" + "code-point-at": "1.1.0", + "is-fullwidth-code-point": "1.0.0", + "strip-ansi": "3.0.1" } }, "stringstream": { @@ -5489,6 +5479,16 @@ "integrity": "sha1-T9kss04OnbPInIYi7PUfm5eMbLk=", "dev": true }, + "JSONStream": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.1.tgz", + "integrity": "sha1-cH92HgHa6eFvG8+TcDt4xwlmV5o=", + "dev": true, + "requires": { + "jsonparse": "1.3.1", + "through": "2.3.8" + } + }, "jsprim": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/jsprim/-/jsprim-1.4.1.tgz", @@ -9207,6 +9207,15 @@ "integrity": "sha1-J5siXfHVgrH1TmWt3UNS4Y+qBxM=", "dev": true }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", + "dev": true, + "requires": { + "safe-buffer": "5.1.1" + } + }, "string-length": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/string-length/-/string-length-1.0.1.tgz", @@ -9227,15 +9236,6 @@ "strip-ansi": "3.0.1" } }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", - "dev": true, - "requires": { - "safe-buffer": "5.1.1" - } - }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz",