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(node): Disable LocalVariables integration on Node < v18 #7748

Merged
merged 7 commits into from Apr 5, 2023

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Apr 5, 2023

It looks like the v8 inspector API has some incompatibilities with domains and async which cause them to not be GC'd in a reasonable timeframe for it to be used in production applications.

This PR causes the LocalVariables integration not to be enabled if the Node version is less than v18.

@AbhiPrasad
Copy link
Member

I refactored the node version constant usage in the SDK - #7734, so we'll prob have to adjust this based on that.

@timfish
Copy link
Collaborator Author

timfish commented Apr 5, 2023

Not sure what's going on here. The tests pass locally under Node v16!

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Apr 5, 2023

Should we only run the local variables unit tests in Node 18 and above?

@timfish
Copy link
Collaborator Author

timfish commented Apr 5, 2023

Oh yes, thanks!

@AbhiPrasad AbhiPrasad marked this pull request as ready for review April 5, 2023 12:34
@AbhiPrasad
Copy link
Member

Marking this ready to review because we want to do a release today.

@AbhiPrasad AbhiPrasad enabled auto-merge (squash) April 5, 2023 12:56
@AbhiPrasad AbhiPrasad merged commit 41ba98a into getsentry:develop Apr 5, 2023
43 checks passed
@timfish timfish deleted the fix/local-variables branch April 5, 2023 13:05
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

2 participants