From 57b93f0639ae20c881d34d4d56abf8c8608e7a46 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 12 Feb 2024 11:52:11 +0100 Subject: [PATCH 1/3] fix(sveltekit): Properly await sourcemaps flattening (#10602) We didn't properly await sourcemaps flattening via sorcery before proceeding to upload them. The reason is that the async callbacks in `forEach` weren't awaited. A `for` loop is the better approach here. Wondering if we should lint against async `forEach` callbacks. This behaviour could have caused various inconsistencies. My suspicion is that the timing worked _well enough_ in most cases but we definitely want to properly await this step. Thanks to @MSDev201 for bringing this up! Unfortunately this likely won't fix #10589 as a whole :( --- packages/sveltekit/src/vite/sourceMaps.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index eea862e9d901..2f5fd28ac1f8 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -166,7 +166,7 @@ export async function makeCustomSentryVitePlugin(options?: CustomSentryVitePlugi // eslint-disable-next-line no-console debug && console.log('[Source Maps Plugin] Flattening source maps'); - jsFiles.forEach(async file => { + for (const file of jsFiles) { try { await (sorcery as Sorcery).load(file).then(async chain => { if (!chain) { @@ -202,7 +202,7 @@ export async function makeCustomSentryVitePlugin(options?: CustomSentryVitePlugi ); await fs.promises.writeFile(mapFile, cleanedMapContent); } - }); + } try { // @ts-expect-error - this hook exists on the plugin! From 9eab11928d90bf2a129f72097cd9bff072acbbbc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Feb 2024 16:25:35 +0100 Subject: [PATCH 2/3] fix(sveltekit): Avoid capturing Http 4xx errors on the client (#10571) Add the Http 400 avoidance logic from our server-side `load` function instrumentation to the client-side wrapper. Didn't know that these errors were a thing on the client side but now that we know, we definitely don't want to capture them. Co-authored-by: Francesco Novy --- packages/sveltekit/src/client/load.ts | 8 +++++-- packages/sveltekit/test/client/load.test.ts | 24 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index 14528959d34e..3e454bdb364a 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -4,7 +4,7 @@ import { addNonEnumerableProperty, objectify } from '@sentry/utils'; import type { LoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { isRedirect } from '../common/utils'; +import { isHttpError, isRedirect } from '../common/utils'; type PatchedLoadEvent = LoadEvent & Partial; @@ -14,7 +14,11 @@ function sendErrorToSentry(e: unknown): unknown { const objectifiedErr = objectify(e); // We don't want to capture thrown `Redirect`s as these are not errors but expected behaviour - if (isRedirect(objectifiedErr)) { + // Neither 4xx errors, given that they are not valuable. + if ( + isRedirect(objectifiedErr) || + (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) + ) { return objectifiedErr; } diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts index e839b5a9cba5..ca7c1d625224 100644 --- a/packages/sveltekit/test/client/load.test.ts +++ b/packages/sveltekit/test/client/load.test.ts @@ -69,6 +69,30 @@ describe('wrapLoadWithSentry', () => { expect(mockCaptureException).not.toHaveBeenCalled(); }); + it.each([400, 404, 499])("doesn't call captureException for thrown `HttpError`s with status %s", async status => { + async function load(_: Parameters[0]): Promise> { + throw { status, body: 'error' }; + } + + const wrappedLoad = wrapLoadWithSentry(load); + const res = wrappedLoad(MOCK_LOAD_ARGS); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it.each([500, 501, 599])('calls captureException for thrown `HttpError`s with status %s', async status => { + async function load(_: Parameters[0]): Promise> { + throw { status, body: 'error' }; + } + + const wrappedLoad = wrapLoadWithSentry(load); + const res = wrappedLoad(MOCK_LOAD_ARGS); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + }); + describe('calls trace function', async () => { it('creates a load span', async () => { async function load({ params }: Parameters[0]): Promise> { From 3545e8e7602e4bd5574994abe7166b49edb19caa Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Feb 2024 16:26:09 +0100 Subject: [PATCH 3/3] fix(angular-ivy): Add `exports` field to `package.json` (#10569) Add an `exports` field to the `package.json` for `@sentry/angular-ivy`. While it seems like regular Angular apps didn't need it, tools like `vitest` expect the field as soon as `type: "module"` is specified. --- Co-authored-by: Andrei Alecu --- packages/angular-ivy/scripts/prepack.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/angular-ivy/scripts/prepack.ts b/packages/angular-ivy/scripts/prepack.ts index b9ec3f0d787f..bc9159954b39 100644 --- a/packages/angular-ivy/scripts/prepack.ts +++ b/packages/angular-ivy/scripts/prepack.ts @@ -6,6 +6,7 @@ type PackageJson = { type?: string; nx?: string; volta?: any; + exports?: Record>; }; const buildDir = path.join(process.cwd(), 'build'); @@ -18,6 +19,18 @@ const pkgJson: PackageJson = JSON.parse(fs.readFileSync(pkjJsonPath).toString()) delete pkgJson.main; pkgJson.type = 'module'; +pkgJson.exports = { + '.': { + es2015: './fesm2015/sentry-angular-ivy.js', + esm2015: './esm2015/sentry-angular-ivy.js', + fesm2015: './fesm2015/sentry-angular-ivy.js', + import: './fesm2015/sentry-angular-ivy.js', + require: './bundles/sentry-angular-ivy.umd.js', + types: './sentry-angular-ivy.d.ts', + }, + './*': './*', +}; + // no need to keep around other properties that are only relevant for our reop: delete pkgJson.nx; delete pkgJson.volta;