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

fix(core): unref timer to not block node exit #11430

Merged
merged 3 commits into from
Apr 4, 2024
Merged

Conversation

AbhiPrasad
Copy link
Member

Add unref to make sure that the session flusher or metrics aggregator does not block nodejs exit. Especially important for serverless scenarios.

@AbhiPrasad AbhiPrasad requested review from timfish and a team April 4, 2024 14:54
@AbhiPrasad AbhiPrasad self-assigned this Apr 4, 2024
@AbhiPrasad AbhiPrasad requested review from mydea and Lms24 and removed request for a team April 4, 2024 14:54
@lforst
Copy link
Member

lforst commented Apr 4, 2024

Can you think of an easy way to test this? 🤔

Copy link
Contributor

github-actions bot commented Apr 4, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) 76.26 KB (0%)
@sentry/browser (incl. Tracing, Replay) 67.53 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 71.35 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 61.08 KB (0%)
@sentry/browser (incl. Tracing) 32.19 KB (0%)
@sentry/browser (incl. browserTracingIntegration) 32.19 KB (0%)
@sentry/browser (incl. feedbackIntegration) 31.3 KB (0%)
@sentry/browser (incl. feedbackModalIntegration) 31.43 KB (0%)
@sentry/browser (incl. feedbackScreenshotIntegration) 31.43 KB (0%)
@sentry/browser (incl. sendFeedback) 27.37 KB (0%)
@sentry/browser 22.52 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 71.27 KB (0%)
CDN Bundle (incl. Tracing, Replay) 65.99 KB (0%)
CDN Bundle (incl. Tracing) 32.3 KB (0%)
CDN Bundle 23.74 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 205.57 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 96.28 KB (0%)
CDN Bundle - uncompressed 70.05 KB (0%)
@sentry/react (incl. Tracing, Replay) 67.52 KB (0%)
@sentry/react 22.56 KB (0%)

@timfish
Copy link
Collaborator

timfish commented Apr 4, 2024

Can you think of an easy way to test this?

We have e2e tests for this for ANR as there were some similar bugs there:

@AbhiPrasad
Copy link
Member Author

Added a test in 4abea77 (and some guards because these packages technically live in core 😬)

@AbhiPrasad AbhiPrasad merged commit 30e7dcc into develop Apr 4, 2024
95 checks passed
@AbhiPrasad AbhiPrasad deleted the abhi-unref-timer branch April 4, 2024 19:05
AbhiPrasad added a commit that referenced this pull request Apr 8, 2024
Add `unref` to make sure that the session flusher or metrics aggregator
does not block nodejs exit. Especially important for serverless
scenarios.
AbhiPrasad added a commit that referenced this pull request Apr 8, 2024
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this pull request Apr 19, 2024
Add `unref` to make sure that the session flusher or metrics aggregator
does not block nodejs exit. Especially important for serverless
scenarios.
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.

3 participants