Merge pull request #3004 from github/koesie10/redactable-error-stack

Show stack for redactable error in log

Author: koesie10

extensions/ql-vscode/src/common/errors.ts

@@ -22,6 +22,14 @@ export class RedactableError extends Error {
       .join("");
   }
 
+  public get fullMessageWithStack(): string {
+    if (!this.stack) {
+      return this.fullMessage;
+    }
+
+    return `${this.fullMessage}\n${this.stack}`;
+  }
+
   public get redactedMessage(): string {
     return this.strings
       .map((s, i) => s + (this.hasValue(i) ? this.getRedactedValue(i) : ""))

extensions/ql-vscode/src/common/logging/notifications.ts

@@ -112,5 +112,5 @@ export async function showAndLogExceptionWithTelemetry(
   options: ShowAndLogExceptionOptions = {},
 ): Promise<void> {
   telemetry?.sendError(error, options.extraTelemetryProperties);
-  return showAndLogErrorMessage(logger, error.fullMessage, options);
+  return showAndLogErrorMessage(logger, error.fullMessageWithStack, options);
 }

extensions/ql-vscode/src/common/vscode/commands.ts

@@ -6,11 +6,7 @@ import {
   showAndLogExceptionWithTelemetry,
 } from "../logging";
 import { extLogger } from "../logging/vscode";
-import {
-  asError,
-  getErrorMessage,
-  getErrorStack,
-} from "../../common/helpers-pure";
+import { asError, getErrorMessage } from "../../common/helpers-pure";
 import { redactableError } from "../../common/errors";
 import { UserCancellationException } from "./progress";
 import { telemetryListener } from "./telemetry";
@@ -66,10 +62,7 @@ export function registerCommandWithErrorHandling(
         }
       } else {
         // Include the full stack in the error log only.
-        const errorStack = getErrorStack(e);
-        const fullMessage = errorStack
-          ? `${errorMessage.fullMessage}\n${errorStack}`
-          : errorMessage.fullMessage;
+        const fullMessage = errorMessage.fullMessageWithStack;
         void showAndLogExceptionWithTelemetry(logger, telemetry, errorMessage, {
           fullMessage,
           extraTelemetryProperties: {

extensions/ql-vscode/src/extension.ts

@@ -1170,10 +1170,7 @@ function addUnhandledRejectionListener() {
       const message = redactableError(
         asError(error),
       )`Unhandled error: ${getErrorMessage(error)}`;
-      const stack = getErrorStack(error);
-      const fullMessage = stack
-        ? `Unhandled error: ${stack}`
-        : message.fullMessage;
+      const fullMessage = message.fullMessageWithStack;
 
       // Add a catch so that showAndLogExceptionWithTelemetry fails, we avoid
       // triggering "unhandledRejection" and avoid an infinite loop

extensions/ql-vscode/src/query-server/legacy/run-queries.ts

@@ -441,7 +441,6 @@ export async function compileAndRunQueryAgainstDatabaseCore(
         const error = result.message
           ? redactableError`${result.message}`
           : redactableError`Failed to run query`;
-        void extLogger.log(error.fullMessage);
         void showAndLogExceptionWithTelemetry(
           extLogger,
           telemetryListener,

extensions/ql-vscode/test/unit-tests/common/errors.test.ts

@@ -19,6 +19,40 @@ describe("errorMessage", () => {
     ).toEqual("Failed to create database foo");
   });
 
+  it("fullMessageWithStack includes the stack", () => {
+    expect(
+      redactableError`Failed to create database ${"foo"}`.fullMessageWithStack,
+    ).toMatch(
+      /^Failed to create database foo\nError: Failed to create database foo\n +at redactableError \(/,
+    );
+  });
+
+  it("fullMessageWithStack includes the cause stack for given error", () => {
+    function myRealFunction() {
+      throw new Error("Internal error");
+    }
+
+    let error: Error;
+    try {
+      myRealFunction();
+
+      fail("Expected an error to be thrown");
+    } catch (e: unknown) {
+      if (!(e instanceof Error)) {
+        throw new Error("Expected an Error to be thrown");
+      }
+
+      error = e;
+    }
+
+    expect(
+      redactableError(error)`Failed to create database ${"foo"}`
+        .fullMessageWithStack,
+    ).toMatch(
+      /^Failed to create database foo\nError: Internal error\n +at myRealFunction \(/,
+    );
+  });
+
   it("redactedMessage redacts the given message", () => {
     expect(
       redactableError`Failed to create database ${"foo"}`.redactedMessage,