Skip to content

ref(core): Switch all internal uses of captureException to use @sentry/core #6488

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

Closed
wants to merge 13 commits into from

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Dec 9, 2022

As part of the work to add correct handled data to events (albeit under a different name for the moment), this switches all internal uses of captureException to use the version that lives in @sentry/core rather than a version imported from @sentry/browser, @sentry/node, or any of the SDKs which wrap them. This is necessary so that we can differentiate between users' captureException calls (which will mostly use the SDK-level captureException, and if not, either hub.captureException or client.captureException) and our own internal ones.

Key changes:

  • A new eslint rule which enforces two structures:

    • imports of the form import { captureException } from '@sentry/xxx'; are only allowed if xxx is core
    • uses of the form someModule.captureException(...) where someModule comes from an import of the form import * as someModule from '@sentry/xxx' are only allowed if xxx is core
    • uses of getCurrentHub().captureException(...), getCurrentHub().getClient().captureException(...), hub.captureException(...), client.captureException(...), and the like are disallowed.

    It doesn't handle unpacking (import * as someModule from '@sentry/xxx'; const { captureException } = someModule; captureException(...)) because it seemed unlikely we'd do that and the rule seemed like it'd be more complicated than it was worth putting a bunch of effort into given that fact, but I can add it if we think it's necessary.

  • A new function, captureExceptionWithHint() has been added to @sentry/core (because it seemed easier, less disruptive, and better for treeshaking than making the current captureException method accept an EventHint as its second parameter).

  • A new protected method, _captureException has been added to the BaseClient class. For the moment, it's identical to the old captureException method, and the old method now just calls the new method.

  • The mocks in the serverless tests have a slightly strange setup, and currently manually clear all of the mocks, using a function in the mocked @sentry/node module. Because this change required splitting the mocked functions between a mocked @sentry/node and a mocked @sentry/core, using that single function was no longer possible. Fortunately, it wasn't actually necessary to do it that way in the first place - clearAllMocks works just fine.

Ref: #6073

@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2022

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.7 KB (+0.08% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 61 KB (+0.07% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.49 KB (+0.09% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 54.53 KB (+0.04% 🔺)
@sentry/browser - Webpack (gzipped + minified) 20.29 KB (+0.1% 🔺)
@sentry/browser - Webpack (minified) 66.32 KB (+0.1% 🔺)
@sentry/react - Webpack (gzipped + minified) 20.31 KB (+0.07% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified) 47.29 KB (+0.02% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.68 KB (+0.05% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.13 KB (+0.07% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 41.8 KB (-0.01% 🔽)
@sentry/replay - Webpack (gzipped + minified) 37.88 KB (0%)

@lobsterkatie lobsterkatie force-pushed the kmclb-use-captureException-from-core branch 2 times, most recently from bc8a7a8 to c95132a Compare December 12, 2022 07:07
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2023

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@github-actions github-actions bot closed this Jan 12, 2023
@AbhiPrasad AbhiPrasad deleted the kmclb-use-captureException-from-core branch June 24, 2024 22:26
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.

2 participants