Merge pull request #8173 from NomicFoundation/refactor-sentry-reporter

Lazy-load Sentry reporter

Author: alcuadrado

.changeset/cool-planets-reflect.md

@@ -0,0 +1,5 @@
+---
+"hardhat": patch
+---
+
+Improve error reporter

packages/hardhat/src/internal/builtin-plugins/network-manager/edr/edr-provider.ts

@@ -36,7 +36,7 @@ import { numberToHexString } from "@nomicfoundation/hardhat-utils/hex";
 import { deepEqual } from "@nomicfoundation/hardhat-utils/lang";
 import debug from "debug";
 
-import { sendErrorTelemetry } from "../../../cli/telemetry/sentry/reporter.js";
+import { sendErrorTelemetry } from "../../../cli/telemetry/error-reporter/reporter.js";
 import { EDR_NETWORK_REVERT_SNAPSHOT_EVENT } from "../../../constants.js";
 import { hardhatChainTypeToEdrChainType } from "../../../edr/chain-type.js";
 import { getGlobalEdrContext } from "../../../edr/context.js";

packages/hardhat/src/internal/builtin-plugins/node/helpers.ts

@@ -16,7 +16,7 @@ import chalk from "chalk";
 // micro-eth-signer is known to be slow to load, so we lazy load it
 let microEthSigner: typeof MicroEthSignerT | undefined;
 
-import { sendErrorTelemetry } from "../../cli/telemetry/sentry/reporter.js";
+import { sendErrorTelemetry } from "../../cli/telemetry/error-reporter/reporter.js";
 import { isDefaultEdrNetworkHDAccountsConfig } from "../network-manager/edr/edr-provider.js";
 import { normalizeEdrNetworkAccountsConfig } from "../network-manager/edr/utils/convert-to-edr.js";
 

packages/hardhat/src/internal/builtin-plugins/solidity-test/reporter.ts

@@ -9,7 +9,7 @@ import type { TestResult } from "@nomicfoundation/edr";
 import { bytesToHexString } from "@nomicfoundation/hardhat-utils/hex";
 import chalk from "chalk";
 
-import { sendErrorTelemetry } from "../../cli/telemetry/sentry/reporter.js";
+import { sendErrorTelemetry } from "../../cli/telemetry/error-reporter/reporter.js";
 import { SolidityTestStackTraceGenerationError } from "../network-manager/edr/stack-traces/stack-trace-generation-errors.js";
 import { encodeStackTraceEntry } from "../network-manager/edr/stack-traces/stack-trace-solidity-errors.js";
 import { formatTraces } from "../network-manager/edr/utils/trace-formatters.js";

packages/hardhat/src/internal/cli/init/init.ts

@@ -31,7 +31,7 @@ import {
 } from "../../utils/package.js";
 import { BannerManager } from "../banner-manager.js";
 import { sendProjectTypeAnalytics } from "../telemetry/analytics/analytics.js";
-import { sendErrorTelemetry } from "../telemetry/sentry/reporter.js";
+import { sendErrorTelemetry } from "../telemetry/error-reporter/reporter.js";
 
 import {
   getDevDependenciesInstallationCommand,

packages/hardhat/src/internal/cli/main.ts

@@ -46,11 +46,11 @@ import { printErrorMessages } from "./error-handler.js";
 import { getGlobalHelpString } from "./help/get-global-help-string.js";
 import { getHelpString } from "./help/get-help-string.js";
 import { sendTaskAnalytics } from "./telemetry/analytics/analytics.js";
+import { setupGlobalUnhandledErrorHandlers } from "./telemetry/error-reporter/global-error-handlers.js";
 import {
   sendErrorTelemetry,
   setCliHardhatConfigPath,
-  setupErrorTelemetryIfEnabled,
-} from "./telemetry/sentry/reporter.js";
+} from "./telemetry/error-reporter/reporter.js";
 import { printVersionMessage } from "./version.js";
 
 export interface MainOptions {
@@ -64,7 +64,9 @@ export async function main(
   rawArguments: string[],
   options: MainOptions = {},
 ): Promise<void> {
-  await setupErrorTelemetryIfEnabled();
+  // We set up the global unhandled errors before running any functionality
+  setupGlobalUnhandledErrorHandlers();
+
   const print = options.print ?? console.log;
 
   const log = debug("hardhat:core:cli:main");

packages/hardhat/src/internal/cli/node-version.ts

@@ -1,5 +1,8 @@
 // NOTE: We don't use the `semver` package because it's slow to load, and this
 // is always run during the initialization of the CLI.
+//
+// NOTE: This file shouldn't import any non-builtin dependency, as it's imported
+// before enabling source maps support. TODO: Change chalk to util.styleText
 
 import chalk from "chalk";
 

packages/hardhat/src/internal/cli/telemetry/error-reporter/global-error-handlers.ts

@@ -0,0 +1,64 @@
+import { isObject } from "@nomicfoundation/hardhat-utils/lang";
+import debug from "debug";
+
+import { sendErrorTelemetry } from "./reporter.js";
+
+const log = debug("hardhat:core:telemetry:global-error-handlers");
+
+function createUnhandledErrorListener(isPromiseRejection: boolean) {
+  const description = isPromiseRejection
+    ? "Unhandled promise rejection"
+    : "Uncaught exception";
+
+  const mechanismType = isPromiseRejection
+    ? "onunhandledrejection"
+    : "onuncaughtexception";
+
+  async function listener(error: Error | unknown) {
+    log(description, error);
+
+    const telemetryError =
+      error instanceof Error
+        ? error
+        : new Error(
+            isObject(error) &&
+            "message" in error &&
+            typeof error.message === "string"
+              ? error.message
+              : "Unknown error",
+            { cause: error },
+          );
+
+    try {
+      await sendErrorTelemetry(telemetryError, {
+        unhandled: true,
+        mechanismType,
+      });
+    } catch (telemetryErrorReportingError) {
+      log(
+        "Failed to send telemetry for unhandled error",
+        telemetryErrorReportingError,
+      );
+    }
+
+    console.error();
+    console.error(`${description}:`);
+    console.error();
+    console.error(error);
+
+    process.exit(1);
+  }
+
+  return listener;
+}
+
+/**
+ * Sets up global error handlers that report unhandled errors and promise
+ * rejections if authorized by the user.
+ */
+export function setupGlobalUnhandledErrorHandlers(): void {
+  log("Setting up global unhandled error handlers");
+
+  process.on("uncaughtException", createUnhandledErrorListener(false));
+  process.on("unhandledRejection", createUnhandledErrorListener(true));
+}

packages/hardhat/src/internal/cli/telemetry/error-reporter/reporter.ts

@@ -0,0 +1,49 @@
+import type * as SentryReporterT from "../sentry/reporter.js";
+
+// Sentry's reporter loads a large number of modules, so we only load it if
+// needed.
+let sentryReporterModule: typeof SentryReporterT | undefined;
+
+// We cache the `setCliHardhatConfigPath` to avoid loading the reporter just
+// for this setting. We load it and set the config path if needed.
+let cliHardhatConfigPath: string | undefined;
+
+/**
+ * Reports an error if telemetry is authorized by the user.
+ *
+ * While this function is async, it's expected to always complete quickly,
+ * delegating the actual reporting to a subprocess.
+ *
+ * @param error - The error to report.
+ * @param hint - Optional metadata describing how the error was captured, used
+ *   to tag the event in Sentry so unhandled crashes can be distinguished from
+ *   errors caught and reported by the CLI.
+ * @param hint.unhandled - Whether the error reached a global handler without
+ *   being caught (e.g. an uncaught exception or unhandled promise rejection).
+ *   Defaults to `false` (i.e. the error was handled).
+ * @param hint.mechanismType - The Sentry mechanism type, used as a tag on the
+ *   event. Common values are `"onuncaughtexception"`, `"onunhandledrejection"`,
+ *   `"instrument"`, and `"generic"`. Defaults to `"generic"`.
+ */
+export async function sendErrorTelemetry(
+  error: Error,
+  hint?: { unhandled?: boolean; mechanismType?: string },
+): Promise<void> {
+  if (sentryReporterModule === undefined) {
+    sentryReporterModule = await import("../sentry/reporter.js");
+  }
+
+  if (cliHardhatConfigPath !== undefined) {
+    sentryReporterModule.setCliHardhatConfigPath(cliHardhatConfigPath);
+  }
+
+  await sentryReporterModule.sendErrorTelemetry(error, hint);
+}
+
+/**
+ * Sets the config path used in the Hardhat CLI. This is used for better
+ * anonymization of errors.
+ */
+export function setCliHardhatConfigPath(configPath: string): void {
+  cliHardhatConfigPath = configPath;
+}

packages/hardhat/src/internal/cli/telemetry/sentry/init.ts

@@ -1,10 +1,6 @@
 /* This file is inspired by https://github.com/getsentry/sentry-javascript/blob/9.4.0/packages/node/src/sdk/index.ts */
 
-import type {
-  Client,
-  ServerRuntimeClientOptions,
-  Transport,
-} from "@sentry/core";
+import type { ServerRuntimeClientOptions, Transport } from "@sentry/core";
 
 import os from "node:os";
 import path from "node:path";
@@ -18,15 +14,12 @@ import {
   ServerRuntimeClient,
   stackParserFromStackParserOptions,
 } from "@sentry/core";
-import debug from "debug";
 
 import { GENERIC_SERVER_NAME } from "./constants.js";
 import { nodeContextIntegration } from "./vendor/integrations/context.js";
 import { contextLinesIntegration } from "./vendor/integrations/contextlines.js";
 import { createGetModuleFromFilename } from "./vendor/utils/module.js";
 
-const log = debug("hardhat:core:sentry:init");
-
 interface GlobalCustomSentryReporterOptions {
   /**
    * Sentry's DSN
@@ -49,12 +42,6 @@ interface GlobalCustomSentryReporterOptions {
    * See the transport module for the different options.
    */
   transport: Transport;
-
-  /**
-   * If `true`, the global unhandled rejection and uncaught exception handlers
-   * will be installed.
-   */
-  installGlobalHandlers?: boolean;
 }
 
 /**
@@ -72,12 +59,9 @@ interface GlobalCustomSentryReporterOptions {
  * The reason that this uses the global instance of sentry (by calling
  * initAndBind), is that using the client directly doesn't work with the linked
  * errors integration.
- *
- * Calling `init` also has an option to set global unhandled rejection and
- * uncaught exception handlers.
  */
 export function init(options: GlobalCustomSentryReporterOptions): void {
-  const client = initAndBind<ServerRuntimeClient, ServerRuntimeClientOptions>(
+  initAndBind<ServerRuntimeClient, ServerRuntimeClientOptions>(
     ServerRuntimeClient,
     {
       dsn: options.dsn,
@@ -113,48 +97,4 @@ export function init(options: GlobalCustomSentryReporterOptions): void {
       ),
     },
   );
-
-  setupGlobalUnhandledErrorHandlers(client);
-}
-
-function createUnhandledErrorListener(
-  client: Client,
-  isPromiseRejection: boolean,
-) {
-  const description = isPromiseRejection
-    ? "Unhandled promise rejection"
-    : "Uncaught exception";
-
-  async function listener(error: Error | unknown) {
-    log(description, error);
-
-    client.captureException(error, {
-      captureContext: {
-        level: "fatal",
-      },
-      mechanism: {
-        handled: false,
-        type: "onuncaughtexception",
-      },
-    });
-
-    await client.flush(100);
-    await client.close(100);
-
-    console.error();
-    console.error(`${description}:`);
-    console.error();
-    console.error(error);
-
-    process.exit(1);
-  }
-
-  return listener;
-}
-
-function setupGlobalUnhandledErrorHandlers(client: Client) {
-  log("Setting up global unhandled error handlers");
-
-  process.on("uncaughtException", createUnhandledErrorListener(client, false));
-  process.on("unhandledRejection", createUnhandledErrorListener(client, true));
 }