Long stack improvements

When longStackSupport is enabled count each stack trace that is created. While following the promise chain keep appending stacks as long as they are older than the source.
patrixgdd · Sep 5, 2016 · ff07d8b · ff07d8b
1 parent 01252cb
commit ff07d8b
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 3 deletions.
diff --git a/q.js b/q.js
@@ -327,6 +327,11 @@ var object_create = Object.create || function (prototype) {
     return new Type();
 };
 
+var object_defineProperty = Object.defineProperty || function (obj, prop, descriptor) {
+    obj[prop] = descriptor.value;
+    return obj;
+};
+
 var object_hasOwnProperty = uncurryThis(Object.prototype.hasOwnProperty);
 
 var object_keys = Object.keys || function (object) {
@@ -377,12 +382,12 @@ function makeStackTraceLong(error, promise) {
         promise.stack &&
         typeof error === "object" &&
         error !== null &&
-        error.stack &&
-        error.stack.indexOf(STACK_JUMP_SEPARATOR) === -1
+        error.stack
     ) {
         var stacks = [];
         for (var p = promise; !!p; p = p.source) {
-            if (p.stack) {
+            if (p.stack && (!error.__minimumStackCounter__ || error.__minimumStackCounter__ > p.stackCounter)) {
+                object_defineProperty(error, "__minimumStackCounter__", {value: p.stackCounter, configurable: true});
                 stacks.unshift(p.stack);
             }
         }
@@ -516,6 +521,8 @@ Q.nextTick = nextTick;
  */
 Q.longStackSupport = false;
 
+var longStackCounter = 1;
+
 // enable long stacks if Q_DEBUG is set
 if (typeof process === "object" && process && process.env && process.env.Q_DEBUG) {
     Q.longStackSupport = true;
@@ -588,6 +595,7 @@ function defer() {
             // At the same time, cut off the first line; it's always just
             // "[object Promise]\n", as per the `toString`.
             promise.stack = e.stack.substring(e.stack.indexOf("\n") + 1);
+            promise.stackCounter = longStackCounter++;
         }
     }
 

diff --git a/spec/q-spec.js b/spec/q-spec.js
@@ -2758,6 +2758,77 @@ describe("stack trace formatting", function () {
     });
 });
 
+describe("long stack traces", function () {
+    beforeEach(function () {
+        Q.longStackSupport = true;
+    });
+
+    afterEach(function () {
+        Q.longStackSupport = false;
+    });
+
+    it("include all the calling functions", function () {
+        function func1() {
+            return Q().then(function () { return func2(); })
+                .catch(function rethrow (err) {throw err;});
+        }
+        function func2() {
+            return new Q.Promise(function (resolve, reject) {
+                func3().then(resolve, reject);
+            });
+        }
+        function func3() {
+            return new Q.Promise(function (resolve, reject) {
+                setTimeout(function () {
+                    reject(new Error(REASON));
+                }, 0);
+            });
+        }
+
+        return func1()
+        .catch(function (err) {
+            expect(err.stack).toMatch(/^Error: this is not an error/);
+            expect(err.stack).toMatch(/func3(.|\n)*func2(.|\n)*func1/);
+        });
+    });
+
+    it("does not duplicate lines when rethrowing an error", function () {
+        function func1() {
+            return func2()
+                .catch(function rethrow (err) {throw err;})
+                .catch(function rethrow (err) {throw err;});
+        }
+        function func2() {
+            return Q().then(function () {
+                return func3();
+            });
+        }
+        function func3() {
+            return Q.reject(new Error(REASON));
+        }
+
+        return func1()
+        .catch(function (err) {
+            expect(err.stack).toMatch(/^Error: this is not an error/);
+            expect(err.stack).toMatch(/func3(.|\n)*func2(.|\n)*func1/);
+            expect(err.stack.match(/func1/g).length).toBe(1);
+            expect(err.stack.match(/func2/g).length).toBe(1);
+            expect(err.stack.match(/func3/g).length).toBe(1);
+        });
+    });
+
+    it("does not add visible properties to thrown errors", function () {
+        return Q().then(function () { throw new Error(REASON); })
+        .catch(function (err) {
+            var keys = [];
+            for (var key in err) {
+                keys.push(key);
+            }
+            expect(keys.length).toBe(0);
+        });
+    });
+});
+
 describe("possible regressions", function () {
 
     describe("gh-9", function () {