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): Use unique variable for ANR context transfer #11161

Merged
merged 3 commits into from Mar 18, 2024

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Mar 18, 2024

Closes #11147

Copy link
Contributor

github-actions bot commented Mar 18, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) 82.19 KB (added)
@sentry/browser (incl. Tracing, Replay) 73.52 KB (added)
@sentry/browser (incl. Tracing, Replay with Canvas) 77.33 KB (added)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 67.07 KB (added)
@sentry/browser (incl. Tracing) 38.15 KB (added)
@sentry/browser (incl. browserTracingIntegration) 38.15 KB (added)
@sentry/browser (incl. feedbackIntegration) 33.01 KB (added)
@sentry/browser (incl. feedbackModalIntegration) 33.09 KB (added)
@sentry/browser (incl. feedbackScreenshotIntegration) 33.1 KB (added)
@sentry/browser (incl. sendFeedback) 29.08 KB (added)
@sentry/browser 24.25 KB (added)
CDN Bundle (incl. Tracing, Replay, Feedback) 76.57 KB (added)
CDN Bundle (incl. Tracing, Replay) 71.43 KB (added)
CDN Bundle (incl. Tracing) 37.77 KB (added)
CDN Bundle 25.62 KB (added)
CDN Bundle (incl. Tracing, Replay) - uncompressed 223.88 KB (added)
CDN Bundle (incl. Tracing) - uncompressed 113.82 KB (added)
CDN Bundle - uncompressed 75.66 KB (added)
@sentry/react (incl. Tracing, Replay) 73.51 KB (added)
@sentry/react 24.27 KB (added)

Comment on lines 176 to 177
'const ctx = __SENTRY__.acs?.getCurrentScope().getPropagationContext() || {}; ctx.traceId + "-" + ctx.spanId + "-" + ctx.parentSpanId',
'const __sentry_ctx = __SENTRY__.acs?.getCurrentScope().getPropagationContext() || {}; __sentry_ctx.traceId + "-" + __sentry_ctx.spanId + "-" + __sentry_ctx.parentSpanId',
// Don't re-trigger the debugger if this causes an error

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever context this is evaluated in, the next time the same Runtime.evaluate is used (assuming support for multiple ANR captures is added), won't this potentially just lead to the same conflicting variable issue due to it now being defined from the last time Runtime.evaluate defined it? Here was my solution:

const t = Date.now()
// Evaluate a script in the currently paused context
session.post('Runtime.evaluate', {
	// Grab the trace context from the current scope
	expression: `const ctx_${t} = __SENTRY__.acs?.getCurrentScope().getPropagationContext() || {}; ctx_${t}.traceId + "-" + ctx_${t}.spanId + "-" + ctx_${t}.parentSpanId`,
	// Don't re-trigger the debugger if this causes an error
	silent: true,
}, (err, param) => {

Just using var instead of const with your __sentry_ctx would also work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep good point!

@AbhiPrasad AbhiPrasad merged commit 08b86e2 into develop Mar 18, 2024
63 checks passed
@AbhiPrasad AbhiPrasad deleted the timfish/fix/anr-ctx-clash branch March 18, 2024 18:06
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this pull request Apr 19, 2024
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.

Unhandled errors in ANR worker causes app to pause indefinitely
3 participants