diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index d5c7358d..d201b3a0 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/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: { @@ -167,7 +168,7 @@ export function createDebugIdUploadFunction({ }); await cliInstance.releases.uploadSourceMaps( - releaseName ?? "undefined", // unfortunetly this needs a value for now but it will not matter since debug IDs overpower releases anyhow + releaseName ?? "undefined", // unfortunately this needs a value for now but it will not matter since debug IDs overpower releases anyhow { include: [ { @@ -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 () => { diff --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts index 159444bc..61aeb52d 100644 --- a/packages/bundler-plugin-core/src/index.ts +++ b/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) { @@ -170,8 +179,13 @@ export function sentryUnpluginFactory({ throw e; } } else { + // 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"; - throw unknownError; + if (throwByDefault) { + throw unknownError; + } + logger.error("An error occurred. Couldn't finish all operations:", unknownError); } } finally { endSession(); @@ -179,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 ); } diff --git a/packages/bundler-plugin-core/src/plugins/release-management.ts b/packages/bundler-plugin-core/src/plugins/release-management.ts index e3f2cabf..8abac956 100644 --- a/packages/bundler-plugin-core/src/plugins/release-management.ts +++ b/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(); diff --git a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts index 334282ea..e6efd08d 100644 --- a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts +++ b/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; 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); } }, }; diff --git a/packages/bundler-plugin-core/src/types.ts b/packages/bundler-plugin-core/src/types.ts index e6153fff..61db9049 100644 --- a/packages/bundler-plugin-core/src/types.ts +++ b/packages/bundler-plugin-core/src/types.ts @@ -552,3 +552,5 @@ type DeployOptions = { */ url?: string; }; + +export type HandleRecoverableErrorFn = (error: unknown, throwByDefault: boolean) => void; diff --git a/packages/integration-tests/fixtures/errorhandling/build-esbuild.ts b/packages/integration-tests/fixtures/errorhandling/build-esbuild.ts new file mode 100644 index 00000000..080d9d12 --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/build-esbuild.ts @@ -0,0 +1,18 @@ +import { sentryEsbuildPlugin } from "@sentry/esbuild-plugin"; +import { build } from "esbuild"; +import pluginOptions from "./plugin-options"; +import path from "path"; + +build({ + entryPoints: [path.join(__dirname, "input", "bundle.js")], + outdir: path.join(__dirname, "out", "esbuild"), + plugins: [sentryEsbuildPlugin(pluginOptions)], + minify: true, + bundle: true, + format: "cjs", + sourcemap: true, +}).catch((e) => { + // eslint-disable-next-line no-console + console.error(e); + process.exit(1); +}); diff --git a/packages/integration-tests/fixtures/errorhandling/build-rollup.ts b/packages/integration-tests/fixtures/errorhandling/build-rollup.ts new file mode 100644 index 00000000..4fed29ea --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/build-rollup.ts @@ -0,0 +1,24 @@ +import { sentryRollupPlugin } from "@sentry/rollup-plugin"; +import * as path from "path"; +import * as rollup from "rollup"; +import pluginOptions from "./plugin-options"; + +rollup + .rollup({ + input: { + index: path.join(__dirname, "input", "bundle.js"), + }, + plugins: [sentryRollupPlugin(pluginOptions)], + }) + .then((bundle) => + bundle.write({ + dir: path.join(__dirname, "out", "rollup"), + format: "cjs", + exports: "named", + }) + ) + .catch((e) => { + // eslint-disable-next-line no-console + console.error(e); + process.exit(1); + }); diff --git a/packages/integration-tests/fixtures/errorhandling/build-vite.ts b/packages/integration-tests/fixtures/errorhandling/build-vite.ts new file mode 100644 index 00000000..a5489beb --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/build-vite.ts @@ -0,0 +1,27 @@ +import { sentryVitePlugin } from "@sentry/vite-plugin"; +import * as path from "path"; +import * as vite from "vite"; +import pluginOptions from "./plugin-options"; + +vite + .build({ + clearScreen: false, + build: { + outDir: path.join(__dirname, "out", "vite"), + rollupOptions: { + input: { + index: path.join(__dirname, "input", "bundle.js"), + }, + output: { + format: "cjs", + entryFileNames: "[name].js", + }, + }, + }, + plugins: [sentryVitePlugin(pluginOptions)], + }) + .catch((e) => { + // eslint-disable-next-line no-console + console.error(e); + process.exit(1); + }); diff --git a/packages/integration-tests/fixtures/errorhandling/build-webpack4.ts b/packages/integration-tests/fixtures/errorhandling/build-webpack4.ts new file mode 100644 index 00000000..46db8686 --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/build-webpack4.ts @@ -0,0 +1,26 @@ +import { sentryWebpackPlugin } from "@sentry/webpack-plugin"; +import * as path from "path"; +import { default as webpack4 } from "webpack4"; +import pluginOptions from "./plugin-options"; + +webpack4( + { + devtool: "source-map", + cache: false, + entry: { + index: path.join(__dirname, "input", "bundle.js"), + }, + output: { + path: path.join(__dirname, "out", "webpack4"), + libraryTarget: "commonjs", + }, + mode: "production", + target: "node", // needed for webpack 4 so we can access node api + plugins: [sentryWebpackPlugin(pluginOptions)], + }, + (err) => { + if (err) { + process.exit(1); + } + } +); diff --git a/packages/integration-tests/fixtures/errorhandling/build-webpack5.ts b/packages/integration-tests/fixtures/errorhandling/build-webpack5.ts new file mode 100644 index 00000000..c2c31a70 --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/build-webpack5.ts @@ -0,0 +1,27 @@ +import { sentryWebpackPlugin } from "@sentry/webpack-plugin"; +import * as path from "path"; +import { webpack as webpack5 } from "webpack5"; +import pluginOptions from "./plugin-options"; + +webpack5( + { + devtool: "source-map", + cache: false, + entry: { + index: path.join(__dirname, "input", "bundle.js"), + }, + output: { + path: path.join(__dirname, "out", "webpack5"), + library: { + type: "commonjs", + }, + }, + mode: "production", + plugins: [sentryWebpackPlugin(pluginOptions)], + }, + (err) => { + if (err) { + process.exit(1); + } + } +); diff --git a/packages/integration-tests/fixtures/errorhandling/error-no-handler.test.ts b/packages/integration-tests/fixtures/errorhandling/error-no-handler.test.ts new file mode 100644 index 00000000..fd93a8cb --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/error-no-handler.test.ts @@ -0,0 +1,60 @@ +/* eslint-disable jest/no-standalone-expect */ +/* eslint-disable jest/expect-expect */ +import path from "path"; +import { spawn } from "child_process"; + +jest.setTimeout(10_000); + +describe("Error throwing by default (no errorHandler)", () => { + const FAKE_SENTRY_PORT = "9876"; + + const sentryServer = spawn("node", [path.join(__dirname, "fakeSentry.js")], { + cwd: __dirname, + stdio: "ignore", // <-- set to "inherit" to get server logs. Deactivated to avoid test logs. + env: { ...process.env, FAKE_SENTRY_PORT }, + shell: true, + }); + + beforeAll(async () => { + await new Promise((resolve) => + sentryServer.on("spawn", () => { + resolve(); + }) + ); + }); + + afterAll(() => { + sentryServer.kill(); + }); + + const bundlersToTest = ["vite", "rollup", "webpack5", "esbuild"]; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + if (parseInt(process.version.split(".")[0]!.slice(1)) < 18) { + bundlersToTest.push("webpack4"); + } + + test.each(bundlersToTest)( + "doesn't throw when Sentry server responds with error code for %s", + async (bundler) => { + const buildProcess = spawn("yarn", ["ts-node", path.join(__dirname, `build-${bundler}.ts`)], { + env: { ...process.env, FAKE_SENTRY_PORT }, + stdio: "ignore", // <-- set to "inherit" to get build output. Deactivated to avoid spamming test logs. + shell: true, + }); + + const exitCode = await new Promise((resolve, reject) => { + buildProcess.on("exit", (code) => { + resolve(code ?? 99); + }); + + buildProcess.on("error", (err) => { + reject(err); + }); + }); + + expect(exitCode).toBe(0); + + buildProcess.kill(); + } + ); +}); diff --git a/packages/integration-tests/fixtures/errorhandling/fakeSentry.js b/packages/integration-tests/fixtures/errorhandling/fakeSentry.js new file mode 100644 index 00000000..bacc9540 --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/fakeSentry.js @@ -0,0 +1,16 @@ +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { createServer } = require("http"); + +const port = process.env["FAKE_SENTRY_PORT"] || 3000; + +const server = createServer((req, res) => { + // eslint-disable-next-line no-console + console.log("[SANTRY] incoming request", req.url); + res.statusCode = 503; + res.end("Error: Santry unreachable"); +}); + +server.listen(port, () => { + // eslint-disable-next-line no-console + console.log(`[SANTRY] running on http://localhost:${port}/`); +}); diff --git a/packages/integration-tests/fixtures/errorhandling/input/bundle.js b/packages/integration-tests/fixtures/errorhandling/input/bundle.js new file mode 100644 index 00000000..aa70f660 --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/input/bundle.js @@ -0,0 +1,2 @@ +// eslint-disable-next-line no-console +console.log("whatever"); diff --git a/packages/integration-tests/fixtures/errorhandling/plugin-options.ts b/packages/integration-tests/fixtures/errorhandling/plugin-options.ts new file mode 100644 index 00000000..fb5960b1 --- /dev/null +++ b/packages/integration-tests/fixtures/errorhandling/plugin-options.ts @@ -0,0 +1,10 @@ +export default { + url: `http://localhost:${process.env["FAKE_SENTRY_PORT"] || 3000}`, + authToken: "fake-auth", + org: "fake-org", + project: "fake-project", + release: { + name: "1.0.0", + }, + debug: true, +};