Fix stacktrace comment losing context

index.js

@@ -55,7 +55,7 @@ exports.wrapSequelize = (sequelize) => {
         // mysqldumpslow (which aggregates slow query logs) by default replaces
         // all strings with quotes with 'S'. We don't want that, since we want
         // to see stacktraces in our slow query logs.
-        const commentStr = `stacktrace=\n${makeMinimalUsefulStacktrace().replace(/[^\w.:/\\\-\s\n]/g, '')}`;
+        const commentStr = `stacktrace=\n${makeMinimalUsefulStacktrace(new Error().stack ?? 'stacktrace not defined').replace(/[^\w.:/\\\-\s\n]/g, '')}`;
 
         if (commentStr && commentStr.length > 0)
             sql = `${sql} /* ${commentStr} */`;

test/util.test.js

@@ -52,12 +52,17 @@ describe("Unit", () => {
         });
     });
 
+    describe("makeMinimalUsefulStacktrace", () => {
     // This test doesn't work well since when we run the test with mocha, the
     // test functions in node_modules are at the bottom, resulting in us
     // getting the wrong stacktrace
-    // describe("makeMinimalUsefulStacktrace", () => {
     //     it("should only contain about 4 lines of context", () => {
     //         expect(makeMinimalUsefulStacktrace().match(/\n/g) || []).to.have.length(3);
     //     });
-    // });
+
+        it("should include the whole stacktrace if it is not in V8 format", () => {
+            const stacktrace = "this is a non-V8 stacktrace";
+            expect(makeMinimalUsefulStacktrace(stacktrace)).to.equal(stacktrace);
+        });
+    });
 });

util.js

@@ -44,25 +44,36 @@ exports.hasComment = (sql) => {
  * Create a stacktrace summary context useful for figuring out where
  * Sequelize queries originate.
  */
-exports.makeMinimalUsefulStacktrace = () => {
-    const stacktrace = new Error().stack;
-
+exports.makeMinimalUsefulStacktrace = (stacktrace) => {
     // Most Sequelize queries contain something of the form "at Function.{query}",
     // e.g. "at Function.findAll". This is a hint to help us find useful
     // context.
     const indexOfUsefulInfoForQuery = stacktrace.lastIndexOf('node_modules/');
-    let minimalUsefulStacktrace = stacktrace.slice(
-        indexOfUsefulInfoForQuery >= 0
-            ? indexOfUsefulInfoForQuery
-            : stacktrace.indexOf('at'),
-    );
+    // Find the index of the 'at' keyword. This is typically used 
+    const indexOfAt = stacktrace.indexOf('at');
+    let minimalUsefulStacktrace = stacktrace;
+
+    if (indexOfUsefulInfoForQuery >= 0) {
+        minimalUsefulStacktrace = stacktrace.slice(indexOfUsefulInfoForQuery);
+    } else if (indexOfAt >= 0) {
+        minimalUsefulStacktrace = stacktrace.slice(indexOfAt);
+    }
+
+    // If the minimal useful stacktrace is shorter than the original stacktrace,
+    // it means the trace is likely in V8 format. We can safely truncate without
+    // losing too much context.
 
-    // Only get about 4 lines of context
-    minimalUsefulStacktrace = minimalUsefulStacktrace
-        .split('\n')
-        .slice(1, 5)
-        .map((stackLine) => stackLine.trim())
-        .join('\n');
+    // If we couldn't find a useful truncation point, we should just keep the 
+    // whole stacktrace to avoid losing too much context and iterate on this 
+    // utility how to better handle exceptions.
+    if (minimalUsefulStacktrace.length < stacktrace.length) {
+        // Only get about 4 lines of context
+        minimalUsefulStacktrace = minimalUsefulStacktrace
+            .split('\n')
+            .slice(1, 5)
+            .map((stackLine) => stackLine.trim())
+            .join('\n');
+    }
 
     return minimalUsefulStacktrace;
 }