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(v8/node): Enforce that ContextLines integration does not leave open file handles #14997

Merged
merged 3 commits into from
Jan 14, 2025

Conversation

AbhiPrasad
Copy link
Member

v8 backport of #14995

…file handles (#14995)

The ContextLines integration uses readable streams to more memory
efficiently read files that it uses to attach source context to outgoing
events. See more details here:
#12221

Unfortunately, if we don't explicitly destroy the stream after creating
and using it, it won't get closed, even when we remove the readline
interface that uses the stream (which actual does the reading of files).

To fix this, we adjust the resolve logic when getting file context to
destroy the stream, as we anyway are done with the readline interface.
@AbhiPrasad AbhiPrasad requested review from timfish and mydea January 14, 2025 03:19
@AbhiPrasad AbhiPrasad self-assigned this Jan 14, 2025
@AbhiPrasad AbhiPrasad changed the title fix(node): Enforce that ContextLines integration does not leave open … fix(v8/node): Enforce that ContextLines integration does not leave open … Jan 14, 2025
@AbhiPrasad AbhiPrasad changed the title fix(v8/node): Enforce that ContextLines integration does not leave open … fix(v8/node): Enforce that ContextLines integration does not leave open file handles Jan 14, 2025
Copy link
Contributor

github-actions bot commented Jan 14, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.29 KB - -
@sentry/browser - with treeshaking flags 21.96 KB - -
@sentry/browser (incl. Tracing) 35.85 KB - -
@sentry/browser (incl. Tracing, Replay) 73.19 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.58 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 77.5 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 89.44 KB - -
@sentry/browser (incl. Feedback) 39.5 KB - -
@sentry/browser (incl. sendFeedback) 27.89 KB - -
@sentry/browser (incl. FeedbackAsync) 32.69 KB - -
@sentry/react 25.97 KB - -
@sentry/react (incl. Tracing) 38.67 KB - -
@sentry/vue 27.57 KB - -
@sentry/vue (incl. Tracing) 37.71 KB - -
@sentry/svelte 23.45 KB - -
CDN Bundle 24.49 KB - -
CDN Bundle (incl. Tracing) 37.56 KB - -
CDN Bundle (incl. Tracing, Replay) 72.84 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 78.2 KB - -
CDN Bundle - uncompressed 71.93 KB - -
CDN Bundle (incl. Tracing) - uncompressed 111.42 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 225.68 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 238.78 KB - -
@sentry/nextjs (client) 38.92 KB - -
@sentry/sveltekit (client) 36.36 KB - -
@sentry/node 162.82 KB +0.01% +15 B 🔺
@sentry/node - without tracing 98.95 KB +0.02% +11 B 🔺
@sentry/aws-serverless 126.65 KB +0.02% +20 B 🔺

View base workflow run

@mydea
Copy link
Member

mydea commented Jan 14, 2025

tests failing because of #15004!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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