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

ref(node): Remove unnecessary emitters option from async context abstraction #7849

Merged
merged 1 commit into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 2 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ const DEFAULT_BREADCRUMBS = 100;
export interface RunWithAsyncContextOptions {
/** Whether to reuse an existing async context if one exists. Defaults to false. */
reuseExisting?: boolean;
/** Instances that should be referenced and retained in the new context */
emitters?: unknown[];
}

/**
Expand Down
1 change: 0 additions & 1 deletion packages/nextjs/src/server/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
throw objectifiedErr;
}
},
{ emitters: [req, res] },
);

// Since API route handlers are all async, nextjs always awaits the return value (meaning it's fine for us to return
Expand Down
13 changes: 0 additions & 13 deletions packages/node/src/async/domain.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { Carrier, Hub, RunWithAsyncContextOptions } from '@sentry/core';
import { ensureHubOnCarrier, getHubFromCarrier, setAsyncContextStrategy, setHubOnCarrier } from '@sentry/core';
import * as domain from 'domain';
import { EventEmitter } from 'events';

function getActiveDomain<T>(): T | undefined {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -31,24 +30,12 @@ function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWithAsync
const activeDomain = getActiveDomain<domain.Domain & Carrier>();

if (activeDomain && options?.reuseExisting) {
for (const emitter of options.emitters || []) {
if (emitter instanceof EventEmitter) {
activeDomain.add(emitter);
}
}

// We're already in a domain, so we don't need to create a new one, just call the callback with the current hub
return callback(getHubFromCarrier(activeDomain));
}

const local = domain.create() as domain.Domain & Carrier;

for (const emitter of options.emitters || []) {
if (emitter instanceof EventEmitter) {
local.add(emitter);
}
}

const parentHub = activeDomain ? getHubFromCarrier(activeDomain) : undefined;
const newHub = createNewHub(parentHub);
setHubOnCarrier(local, newHub);
Expand Down
63 changes: 30 additions & 33 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,43 +187,40 @@ export function requestHandler(
});
};
}
runWithAsyncContext(
currentHub => {
currentHub.configureScope(scope => {
scope.setSDKProcessingMetadata({
request: req,
// TODO (v8): Stop passing this
requestDataOptionsFromExpressHandler: requestDataOptions,
});
runWithAsyncContext(currentHub => {
currentHub.configureScope(scope => {
scope.setSDKProcessingMetadata({
request: req,
// TODO (v8): Stop passing this
requestDataOptionsFromExpressHandler: requestDataOptions,
});

const client = currentHub.getClient<NodeClient>();
if (isAutoSessionTrackingEnabled(client)) {
const scope = currentHub.getScope();
if (scope) {
// Set `status` of `RequestSession` to Ok, at the beginning of the request
scope.setRequestSession({ status: 'ok' });
}
const client = currentHub.getClient<NodeClient>();
if (isAutoSessionTrackingEnabled(client)) {
const scope = currentHub.getScope();
if (scope) {
// Set `status` of `RequestSession` to Ok, at the beginning of the request
scope.setRequestSession({ status: 'ok' });
}
});
}
});

res.once('finish', () => {
const client = currentHub.getClient<NodeClient>();
if (isAutoSessionTrackingEnabled(client)) {
setImmediate(() => {
res.once('finish', () => {
const client = currentHub.getClient<NodeClient>();
if (isAutoSessionTrackingEnabled(client)) {
setImmediate(() => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (client && (client as any)._captureRequestSession) {
// Calling _captureRequestSession to capture request session at the end of the request by incrementing
// the correct SessionAggregates bucket i.e. crashed, errored or exited
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (client && (client as any)._captureRequestSession) {
// Calling _captureRequestSession to capture request session at the end of the request by incrementing
// the correct SessionAggregates bucket i.e. crashed, errored or exited
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(client as any)._captureRequestSession();
}
});
}
});
next();
},
{ emitters: [req, res] },
);
(client as any)._captureRequestSession();
}
});
}
});
next();
});
};
}

Expand Down