add throwByDefault flag and require it on every handleRecoverableError call

Author: Lms24

packages/bundler-plugin-core/src/debug-id-upload.ts

@@ -12,6 +12,7 @@ import { stripQueryAndHashFromPath } from "./utils";
 import { setMeasurement, spanToTraceHeader, startSpan } from "@sentry/core";
 import { getDynamicSamplingContextFromSpan, Scope } from "@sentry/core";
 import { Client } from "@sentry/types";
+import { HandleRecoverableErrorFn } from "./types";
 
 interface RewriteSourcesHook {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -25,7 +26,7 @@ interface DebugIdUploadPluginOptions {
   releaseName?: string;
   dist?: string;
   rewriteSourcesHook?: RewriteSourcesHook;
-  handleRecoverableError: (error: unknown) => void;
+  handleRecoverableError: HandleRecoverableErrorFn;
   sentryScope: Scope;
   sentryClient: Client;
   sentryCliOptions: {
@@ -187,7 +188,7 @@ export function createDebugIdUploadFunction({
           }
         } catch (e) {
           sentryScope.captureException('Error in "debugIdUploadPlugin" writeBundle hook');
-          handleRecoverableError(e);
+          handleRecoverableError(e, false);
         } finally {
           if (folderToCleanUp) {
             void startSpan({ name: "cleanup", scope: sentryScope }, async () => {

packages/bundler-plugin-core/src/index.ts

@@ -155,7 +155,16 @@ export function sentryUnpluginFactory({
       "SENTRY_PIPELINE"
     ] = `${unpluginMetaContext.framework}-plugin/${__PACKAGE_VERSION__}`;
 
-    function handleRecoverableError(unknownError: unknown) {
+    /**
+     * Handles errors caught and emitted in various areas of the plugin.
+     *
+     * Also sets the sentry session status according to the error handling.
+     *
+     * If users specify their custom `errorHandler` we'll leave the decision to throw
+     * or continue up to them. By default, @param throwByDefault controls if the plugin
+     * should throw an error (which causes a build fail in most bundlers) or continue.
+     */
+    function handleRecoverableError(unknownError: unknown, throwByDefault: boolean) {
       sentrySession.status = "abnormal";
       try {
         if (options.errorHandler) {
@@ -173,6 +182,9 @@ export function sentryUnpluginFactory({
           // setting the session to "crashed" b/c from a plugin perspective this run failed.
           // However, we're intentionally not rethrowing the error to avoid breaking the user build.
           sentrySession.status = "crashed";
+          if (throwByDefault) {
+            throw unknownError;
+          }
           logger.error("An error occurred. Couldn't finish all operations:", unknownError);
         }
       } finally {
@@ -181,8 +193,10 @@ export function sentryUnpluginFactory({
     }
 
     if (!validateOptions(options, logger)) {
+      // Throwing by default to avoid a misconfigured plugin going unnoticed.
       handleRecoverableError(
-        new Error("Options were not set correctly. See output above for more details.")
+        new Error("Options were not set correctly. See output above for more details."),
+        true
       );
     }
 

packages/bundler-plugin-core/src/plugins/release-management.ts

@@ -3,7 +3,7 @@ import { Scope } from "@sentry/core";
 import { UnpluginOptions } from "unplugin";
 import { Logger } from "../sentry/logger";
 import { safeFlushTelemetry } from "../sentry/telemetry";
-import { IncludeEntry } from "../types";
+import { HandleRecoverableErrorFn, IncludeEntry } from "../types";
 import { arrayify } from "../utils";
 import { Client } from "@sentry/types";
 
@@ -16,7 +16,7 @@ interface ReleaseManagementPluginOptions {
   setCommitsOption?: SentryCliCommitsOptions;
   deployOptions?: SentryCliNewDeployOptions;
   dist?: string;
-  handleRecoverableError: (error: unknown) => void;
+  handleRecoverableError: HandleRecoverableErrorFn;
   sentryScope: Scope;
   sentryClient: Client;
   sentryCliOptions: {
@@ -100,7 +100,7 @@ export function releaseManagementPlugin({
       } catch (e) {
         sentryScope.captureException('Error in "releaseManagementPlugin" writeBundle hook');
         await safeFlushTelemetry(sentryClient);
-        handleRecoverableError(e);
+        handleRecoverableError(e, false);
       } finally {
         freeGlobalDependencyOnSourcemapFiles();
         freeWriteBundleInvocationDependencyOnSourcemapFiles();

packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts

@@ -5,9 +5,10 @@ import { safeFlushTelemetry } from "../sentry/telemetry";
 import fs from "fs";
 import { Scope } from "@sentry/core";
 import { Client } from "@sentry/types";
+import { HandleRecoverableErrorFn } from "../types";
 
 interface FileDeletionPlugin {
-  handleRecoverableError: (error: unknown) => void;
+  handleRecoverableError: HandleRecoverableErrorFn;
   waitUntilSourcemapFileDependenciesAreFreed: () => Promise<void>;
   sentryScope: Scope;
   sentryClient: Client;
@@ -59,7 +60,9 @@ export function fileDeletionPlugin({
       } catch (e) {
         sentryScope.captureException('Error in "sentry-file-deletion-plugin" buildEnd hook');
         await safeFlushTelemetry(sentryClient);
-        handleRecoverableError(e);
+        // We throw by default if we get here b/c not being able to delete
+        // source maps could leak them to production
+        handleRecoverableError(e, true);
       }
     },
   };

packages/bundler-plugin-core/src/types.ts

@@ -552,3 +552,5 @@ type DeployOptions = {
    */
   url?: string;
 };
+
+export type HandleRecoverableErrorFn = (error: unknown, throwByDefault: boolean) => void;