Skip to content

fx(node): Fix anr worker check #10719

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

Merged
merged 1 commit into from
Feb 19, 2024
Merged

fx(node): Fix anr worker check #10719

merged 1 commit into from
Feb 19, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 19, 2024

I've removed the hub global on the carrier in a recent PR, and noticed this leftover. Guess this is not used/covered by tests...? It should work with using acs as that should now always be set too (also for the default case), unless sentry is not initialized. But for type safety, the acs may be undefined.

@mydea mydea requested a review from timfish February 19, 2024 13:20
@mydea mydea self-assigned this Feb 19, 2024
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.81 KB (-0.11% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 69.07 KB (-0.14% 🔽)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 73 KB (-0.14% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.64 KB (-0.17% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.25 KB (-0.32% 🔽)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.17 KB (-0.29% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.15 KB (-0.27% 🔽)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.14 KB (-0.3% 🔽)
@sentry/browser - Webpack (gzipped) 22.4 KB (-0.39% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 76.17 KB (-0.14% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.68 KB (-0.15% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.59 KB (-0.37% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.71 KB (-0.43% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 213 KB (-0.12% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 101.62 KB (-0.25% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 74.11 KB (-0.33% 🔽)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.86 KB (-0.26% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.41 KB (-0.13% 🔽)
@sentry/react - Webpack (gzipped) 22.43 KB (-0.41% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 86.88 KB (-0.13% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 50.09 KB (-0.2% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 17.14 KB (-0.48% 🔽)

@timfish
Copy link
Collaborator

timfish commented Feb 19, 2024

These values are checked in the integration tests but I guess they're ending up as empty strings which still pass:

trace: {
span_id: expect.any(String),
trace_id: expect.any(String),
},

I'll look into how these tests weren't failing!

@timfish
Copy link
Collaborator

timfish commented Feb 19, 2024

Ah, these integration tests continued to pass because the trace context was still successfully read from __SENTRY__.hub. This is due to the following backwards compatibility code:

}
// eslint-disable-next-line deprecation/deprecation
sentry.hub = new Hub(undefined, getDefaultCurrentScope(), getDefaultIsolationScope());
return sentry.hub;
}

@mydea mydea merged commit e4dc5a7 into develop Feb 19, 2024
@mydea mydea deleted the fn-remove-hub-lookup branch February 19, 2024 16:13
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.

2 participants