-
Notifications
You must be signed in to change notification settings - Fork 131
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: lazily load exception autocapture #856
Conversation
Size Change: -30.7 kB (-4%) Total Size: 709 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this works, but one thing wasn't clear to me
import { PostHog } from '../../posthog-core' | ||
import { DecideResponse, Properties } from '../../types' | ||
import { ErrorEventArgs, ErrorProperties, errorToProperties, unhandledRejectionToProperties } from './error-conversion' | ||
import { isPrimitive } from './type-checking' | ||
|
||
const EXCEPTION_INGESTION_ENDPOINT = '/e/' | ||
|
||
export const extendPostHog = (instance: PostHog, response: DecideResponse) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The place I could see this being called, the decide response isn't passed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eyes!
It's a while until we're going to be able to work on exception autocapture as events but we ship the code to everyone. No bueno.
Move it to lazy loading so things are there when we're ready to use them. Reduces bundle by c7%
Nit-pickingly this is a breaking change by removing
posthog.captureException
but we don't see any events logged from that in the last 90 days