Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sveltekit): Introduce client-side handleError wrapper #7406

Merged
merged 6 commits into from
Mar 10, 2023

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Mar 10, 2023

ref #7402

Adds a wrapper for the client-side handleError hook.

Usage is like the following:

src/hooks.client.js

import { wrapHandleError } from '@sentry/sveltekit';

/** @type {import('@sveltejs/kit').HandleClientError} */
function myCustomHandleError() {
 ...
}

export const handleError = wrapHandleError(myCustomHandleError);

@AbhiPrasad AbhiPrasad self-assigned this Mar 10, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.24 KB (+0.12% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 62.95 KB (+0.11% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.87 KB (+0.1% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 55.87 KB (+0.12% 🔺)
@sentry/browser - Webpack (gzipped + minified) 20.62 KB (+0.12% 🔺)
@sentry/browser - Webpack (minified) 67.39 KB (+0.1% 🔺)
@sentry/react - Webpack (gzipped + minified) 20.65 KB (+0.12% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified) 48.54 KB (+0.18% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 27.51 KB (+0.23% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.76 KB (+0.21% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 43.17 KB (+0.01% 🔺)
@sentry/replay - Webpack (gzipped + minified) 37.19 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 61.18 KB (+0.09% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 54.35 KB (+0.04% 🔺)

@AbhiPrasad AbhiPrasad marked this pull request as ready for review March 10, 2023 11:04
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks great! Let's just make sure that the conditional export paths are fine in the tarball, then this is good to 🚀

packages/sveltekit/package.json Show resolved Hide resolved
packages/sveltekit/package.json Show resolved Hide resolved
packages/sveltekit/src/client/handleError.ts Show resolved Hide resolved
@AbhiPrasad AbhiPrasad force-pushed the abhi-sveltekit-client-handle-error branch from 5e04be9 to 2a9e94b Compare March 10, 2023 12:19
@AbhiPrasad AbhiPrasad enabled auto-merge (squash) March 10, 2023 13:14
@AbhiPrasad AbhiPrasad merged commit bd45dfc into develop Mar 10, 2023
@AbhiPrasad AbhiPrasad deleted the abhi-sveltekit-client-handle-error branch March 10, 2023 13:33
@psytrx
Copy link

psytrx commented Mar 10, 2023

Currently, sequence does not support HandleClientError/HandleServerError. See source.

This would allow us to filter out, transform and enrich errors in an idiomatic way.

export const handleError = sequence(
  filterUselessErrors,
  addContextOrMetadata,
  sentryHandleError,
  myCustomHandleError);

Would it make sense for you to suggest such a change to the SvelteKit team? I'm open to starting a discussion. I'm not quite sure about the consequences, but it feels like it should be a non-breaking change.

@Lms24
Copy link
Member

Lms24 commented Mar 10, 2023

@fjdumont thanks for the suggestion. We're definitely going to consider sequence for the server-side handle hook. If SvelteKit would support sequence for handleError, we could also make consider it in the SDK.

Feel free to go ahead and open a discussion on the SvelteKit repo 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants