Skip to content

feat(node): Include global scope for eventLoopBlockIntegration #20108

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.

Sign up for GitHub

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

Open
timfish wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(node): Include global scope for eventLoopBlockIntegration #20108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dev-packages/node-integration-tests/suites/thread-blocked-native/isolated.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ const fns = [
neverResolve,
];

Sentry.getGlobalScope().setUser({ email: 'something@gmail.com' });

setTimeout(() => {
for (let id = 0; id < 10; id++) {
Sentry.withIsolationScope(async () => {
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('Thread Blocked Native', { timeout: 30_000 }, () => {
message: 'Starting task 5',
},
],
user: { id: 5 },
user: { id: 5, email: 'something@gmail.com' },
threads: {
values: [
{
Expand Down
3 changes: 2 additions & 1 deletion packages/node-native/src/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Contexts, DsnComponents, Primitive, SdkMetadata, Session } from '@sentry/core';
import type { Contexts, DsnComponents, Primitive, ScopeData, SdkMetadata, Session } from '@sentry/core';

export const POLL_RATIO = 2;

Expand Down Expand Up @@ -40,5 +40,6 @@ export interface WorkerStartData extends ThreadBlockedIntegrationOptions {

export interface ThreadState {
session: Session | undefined;
scope: ScopeData;
debugImages: Record<string, string>;
}
22 changes: 20 additions & 2 deletions packages/node-native/src/event-loop-block-integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,18 @@ import type {
EventHint,
Integration,
IntegrationFn,
ScopeData,
} from '@sentry/core';
import {
debug,
defineIntegration,
getClient,
getCurrentScope,
getFilenameToDebugIdMap,
getGlobalScope,
getIsolationScope,
mergeScopeData,
} from '@sentry/core';
import { debug, defineIntegration, getClient, getFilenameToDebugIdMap, getIsolationScope } from '@sentry/core';
import type { NodeClient } from '@sentry/node';
import { registerThread, threadPoll } from '@sentry-internal/node-native-stacktrace';
import type { ThreadBlockedIntegrationOptions, WorkerStartData } from './common';
Expand Down Expand Up @@ -37,6 +47,13 @@ async function getContexts(client: NodeClient): Promise<Contexts> {
return event?.contexts || {};
}

function getLocalScopeData(): ScopeData {
const globalScope = getGlobalScope().getScopeData();
const currentScope = getCurrentScope().getScopeData();
mergeScopeData(globalScope, currentScope);
Comment on lines +51 to +53
Copy link
Copy Markdown
Collaborator Author

@timfish timfish Apr 7, 2026

Choose a reason for hiding this comment

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

Is this required of can we just sync the global scope?

Copy link
Copy Markdown
Member

@mydea mydea Apr 8, 2026

Choose a reason for hiding this comment

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

this seems reasonable to me? ideally I guess it should be global + isolation + current scope, not sure if/how isolation scope is handled here generally :)

Copy link
Copy Markdown
Collaborator Author

@timfish timfish Apr 8, 2026

Choose a reason for hiding this comment

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

Isolation scope is captured via native code from the AsyncLocalStorage!

return globalScope;
}
Copy link
Copy Markdown

@cursor cursor bot Apr 7, 2026

Choose a reason for hiding this comment

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

ScopeData with non-serializable fields passed to native module

Medium Severity

getLocalScopeData() returns a full ScopeData object containing eventProcessors (an array of functions) and potentially span (a complex object with methods). This is passed to threadPoll which needs to share data cross-thread via the native module. The existing code already carefully strips the non-serializable toJSON method from session objects before passing them to threadPoll, but no similar care is taken for ScopeData. If event processors are registered on the global or current scope, the non-serializable functions could cause the threadPoll call to fail, with the error silently swallowed by the try/catch, losing all scope data.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0742247. Configure here.

Copy link
Copy Markdown
Collaborator Author

@timfish timfish Apr 7, 2026

Choose a reason for hiding this comment

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

ScopeData doesn't contain any of those and is safe to serialise...


type IntegrationInternal = { start: () => void; stop: () => void };

function poll(enabled: boolean, clientOptions: ClientOptions): void {
Expand All @@ -45,8 +62,9 @@ function poll(enabled: boolean, clientOptions: ClientOptions): void {
// We need to copy the session object and remove the toJSON method so it can be sent to the worker
// serialized without making it a SerializedSession
const session = currentSession ? { ...currentSession, toJSON: undefined } : undefined;
const scope = getLocalScopeData();
// message the worker to tell it the main event loop is still running
threadPoll(enabled, { session, debugImages: getFilenameToDebugIdMap(clientOptions.stackParser) });
threadPoll(enabled, { session, scope, debugImages: getFilenameToDebugIdMap(clientOptions.stackParser) });
} catch {
// we ignore all errors
}
Expand Down
15 changes: 8 additions & 7 deletions packages/node-native/src/event-loop-block-watchdog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type { ThreadState, WorkerStartData } from './common';
import { POLL_RATIO } from './common';

type CurrentScopes = {
scope: Scope;
isolationScope: Scope;
};

Expand Down Expand Up @@ -275,14 +274,16 @@ async function sendBlockEvent(crashedThreadId: string): Promise<void> {
...getExceptionAndThreads(crashedThreadId, threads),
};

const asyncState = threads[crashedThreadId]?.asyncState;
if (asyncState) {
// We need to rehydrate the scopes from the serialized objects so we can call getScopeData()
const scope = Object.assign(new Scope(), asyncState.scope).getScopeData();
const isolationScope = Object.assign(new Scope(), asyncState.isolationScope).getScopeData();
const scope = crashedThread.pollState?.scope
? new Scope().update(crashedThread.pollState.scope).getScopeData()
: new Scope().getScopeData();

if (crashedThread?.asyncState?.isolationScope) {
// We need to rehydrate the scope from the serialized object with properties beginning with _user, etc
const isolationScope = Object.assign(new Scope(), crashedThread.asyncState.isolationScope).getScopeData();
mergeScopeData(scope, isolationScope);
applyScopeToEvent(event, scope);
}
applyScopeToEvent(event, scope);

const allDebugImages: Record<string, string> = Object.values(threads).reduce((acc, threadState) => {
return { ...acc, ...threadState.pollState?.debugImages };
Expand Down
Loading