Skip to content

Conversation

@isaacs
Copy link

@isaacs isaacs commented Nov 18, 2025

This is a fix commit for the test fragility, on top of JealousGx:fix/local-variables-12588, from PR #17545 which seems to have gone stale. Also, rebased onto develop branch, squashed, and updated to comply with commit message guidelines.

Close: #17545
Fix: #12588
CC: @JealousGx

@isaacs isaacs requested a review from AbhiPrasad November 18, 2025 23:20
@isaacs isaacs force-pushed the isaacs/JealousGx-fix-local-variables-12588 branch from 44f1258 to a5170ec Compare November 18, 2025 23:25
@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.62 kB - -
@sentry/browser - with treeshaking flags 23.13 kB - -
@sentry/browser (incl. Tracing) 41.37 kB - -
@sentry/browser (incl. Tracing, Profiling) 45.69 kB - -
@sentry/browser (incl. Tracing, Replay) 79.82 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 69.52 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 84.5 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 96.73 kB - -
@sentry/browser (incl. Feedback) 41.29 kB - -
@sentry/browser (incl. sendFeedback) 29.29 kB - -
@sentry/browser (incl. FeedbackAsync) 34.21 kB - -
@sentry/react 26.32 kB - -
@sentry/react (incl. Tracing) 43.32 kB - -
@sentry/vue 29.11 kB - -
@sentry/vue (incl. Tracing) 43.17 kB - -
@sentry/svelte 24.64 kB - -
CDN Bundle 26.95 kB - -
CDN Bundle (incl. Tracing) 41.95 kB - -
CDN Bundle (incl. Tracing, Replay) 78.5 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 83.96 kB - -
CDN Bundle - uncompressed 78.95 kB - -
CDN Bundle (incl. Tracing) - uncompressed 124.33 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 240.36 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 253.13 kB - -
@sentry/nextjs (client) 45.74 kB - -
@sentry/sveltekit (client) 41.76 kB - -
⛔️ @sentry/node-core (max: 51 kB) 51.01 kB +0.11% +52 B 🔺
@sentry/node 159.3 kB +0.03% +41 B 🔺
@sentry/node - without tracing 92.87 kB +0.05% +41 B 🔺
@sentry/aws-serverless 106.64 kB +0.06% +61 B 🔺

View base workflow run

@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2025

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 9,245 - 9,040 +2%
GET With Sentry 1,394 15% 1,334 +4%
GET With Sentry (error only) 6,037 65% 5,951 +1%
POST Baseline 1,192 - 1,193 -0%
POST With Sentry 528 44% 501 +5%
POST With Sentry (error only) 1,054 88% 1,044 +1%
MYSQL Baseline 3,301 - 3,259 +1%
MYSQL With Sentry 465 14% 450 +3%
MYSQL With Sentry (error only) 2,708 82% 2,668 +1%

View base workflow run

@isaacs isaacs force-pushed the isaacs/JealousGx-fix-local-variables-12588 branch from a5170ec to 4f88959 Compare November 19, 2025 01:33
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Looks good! I wonder if the e2e test failures are related?

Let's also make sure we're following the commit guidelines here: https://github.com/getsentry/sentry-javascript/blob/develop/docs/commit-issue-pr-guidelines.md#commits

@isaacs isaacs self-assigned this Nov 19, 2025
isaacs pushed a commit that referenced this pull request Nov 19, 2025
Address an issue where local variables were not being captured for
out-of-app frames, even when the `includeOutOfAppFrames` option was
enabled.

The `localVariablesSyncIntegration` had a race condition where it would
process events before the debugger session was fully initialized. Fix
this by awaiting the session creation in `setupOnce`.

The tests for this integration were failing because they were not
setting up a Sentry client, which is required for the integration to be
enabled. Correct by adding a client to the test setup.

Additionally, add tests for the `localVariablesAsyncIntegration` to
ensure it correctly handles the `includeOutOfAppFrames` option.

The `LocalVariables` integrations `setupOnce` method was `async`, which
violates the `Integration` interface. This caused a race condition where
events could be processed before the integration was fully initialized,
leading to missed local variables.

Fix the race condition by:

- Make `setupOnce` synchronous to adhere to the interface contract
- Move the asynchronous initialization logic to a separate `setup`
  function
- Make `processEvent` asynchronous and await the result of the `setup`
  function, so the integration is fully initialized before processing
  any events
- Update tests to correctly `await` the `processEvent` method

Fixes GH-12588
Fixes GH-17545
@isaacs isaacs force-pushed the isaacs/JealousGx-fix-local-variables-12588 branch from 4f88959 to fb3ba43 Compare November 19, 2025 21:49
@isaacs isaacs requested a review from AbhiPrasad November 19, 2025 21:50
Address an issue where local variables were not being captured for
out-of-app frames, even when the `includeOutOfAppFrames` option was
enabled.

The `localVariablesSyncIntegration` had a race condition where it would
process events before the debugger session was fully initialized. Fix
this by awaiting the session creation in `setupOnce`.

The tests for this integration were failing because they were not
setting up a Sentry client, which is required for the integration to be
enabled. Correct by adding a client to the test setup.

Additionally, add tests for the `localVariablesAsyncIntegration` to
ensure it correctly handles the `includeOutOfAppFrames` option.

The `LocalVariables` integrations `setupOnce` method was `async`, which
violates the `Integration` interface. This caused a race condition where
events could be processed before the integration was fully initialized,
leading to missed local variables.

Fix the race condition by:

- Make `setupOnce` synchronous to adhere to the interface contract
- Move the asynchronous initialization logic to a separate `setup`
  function
- Make `processEvent` asynchronous and await the result of the `setup`
  function, so the integration is fully initialized before processing
  any events
- Update tests to correctly `await` the `processEvent` method

Fixes GH-12588
Fixes GH-17545
@isaacs isaacs force-pushed the isaacs/JealousGx-fix-local-variables-12588 branch from fb3ba43 to a7767a3 Compare November 19, 2025 21:54
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.

Local variables missing when exception originates in node_modules dependency

4 participants