ref: Avoid unnecessary hub.getScope() checks (#9008)

We've changed this some time ago so that `hub.getScope()` _always_ returns a scope, so we can actually update our code where we still check for the existence of scope.
getsentry · Sep 12, 2023 · a7f5911 · a7f5911
1 parent 868a3cd
commit a7f5911
Show file tree

Hide file tree

Showing 17 changed files with 106 additions and 144 deletions.
diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts
@@ -55,9 +55,7 @@ export function getActiveTransaction(): Transaction | undefined {
 
   if (currentHub) {
     const scope = currentHub.getScope();
-    if (scope) {
-      return scope.getTransaction();
-    }
+    return scope.getTransaction();
   }
 
   return undefined;

diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts
@@ -14,7 +14,7 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
   return async function (this: unknown, ...args) {
     const req = args[0];
     const currentScope = getCurrentHub().getScope();
-    const prevSpan = currentScope?.getSpan();
+    const prevSpan = currentScope.getSpan();
 
     let span: Span | undefined;
 

diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
@@ -89,75 +89,73 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
           const currentScope = hub.getScope();
           const options = hub.getClient()?.getOptions();
 
-          if (currentScope) {
-            currentScope.setSDKProcessingMetadata({ request: req });
-
-            if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') {
-              const sentryTrace =
-                req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
-              const baggage = req.headers?.baggage;
-              const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
-                sentryTrace,
-                baggage,
-              );
-              hub.getScope().setPropagationContext(propagationContext);
-
-              if (__DEBUG_BUILD__ && traceparentData) {
-                logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
-              }
+          currentScope.setSDKProcessingMetadata({ request: req });
+
+          if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') {
+            const sentryTrace =
+              req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
+            const baggage = req.headers?.baggage;
+            const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
+              sentryTrace,
+              baggage,
+            );
+            currentScope.setPropagationContext(propagationContext);
+
+            if (__DEBUG_BUILD__ && traceparentData) {
+              logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
+            }
 
-              // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
-              let reqPath = parameterizedRoute;
-
-              // If not, fake it by just replacing parameter values with their names, hoping that none of them match either
-              // each other or any hard-coded parts of the path
-              if (!reqPath) {
-                const url = `${req.url}`;
-                // pull off query string, if any
-                reqPath = stripUrlQueryAndFragment(url);
-                // Replace with placeholder
-                if (req.query) {
-                  for (const [key, value] of Object.entries(req.query)) {
-                    reqPath = reqPath.replace(`${value}`, `[${key}]`);
-                  }
+            // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
+            let reqPath = parameterizedRoute;
+
+            // If not, fake it by just replacing parameter values with their names, hoping that none of them match either
+            // each other or any hard-coded parts of the path
+            if (!reqPath) {
+              const url = `${req.url}`;
+              // pull off query string, if any
+              reqPath = stripUrlQueryAndFragment(url);
+              // Replace with placeholder
+              if (req.query) {
+                for (const [key, value] of Object.entries(req.query)) {
+                  reqPath = reqPath.replace(`${value}`, `[${key}]`);
                 }
               }
+            }
 
-              const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
-
-              transaction = startTransaction(
-                {
-                  name: `${reqMethod}${reqPath}`,
-                  op: 'http.server',
-                  origin: 'auto.http.nextjs',
-                  ...traceparentData,
-                  metadata: {
-                    dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
-                    source: 'route',
-                    request: req,
-                  },
+            const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
+
+            transaction = startTransaction(
+              {
+                name: `${reqMethod}${reqPath}`,
+                op: 'http.server',
+                origin: 'auto.http.nextjs',
+                ...traceparentData,
+                metadata: {
+                  dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+                  source: 'route',
+                  request: req,
                 },
-                // extra context passed to the `tracesSampler`
-                { request: req },
-              );
-              currentScope.setSpan(transaction);
-              if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
-                autoEndTransactionOnResponseEnd(transaction, res);
-              } else {
-                // If we're not on a platform that supports streaming, we're blocking res.end() until the queue is flushed.
-                // res.json() and res.send() will implicitly call res.end(), so it is enough to wrap res.end().
-
-                // eslint-disable-next-line @typescript-eslint/unbound-method
-                const origResEnd = res.end;
-                res.end = async function (this: unknown, ...args: unknown[]) {
-                  if (transaction) {
-                    await finishTransaction(transaction, res);
-                    await flushQueue();
-                  }
-
-                  origResEnd.apply(this, args);
-                };
-              }
+              },
+              // extra context passed to the `tracesSampler`
+              { request: req },
+            );
+            currentScope.setSpan(transaction);
+            if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
+              autoEndTransactionOnResponseEnd(transaction, res);
+            } else {
+              // If we're not on a platform that supports streaming, we're blocking res.end() until the queue is flushed.
+              // res.json() and res.send() will implicitly call res.end(), so it is enough to wrap res.end().
+
+              // eslint-disable-next-line @typescript-eslint/unbound-method
+              const origResEnd = res.end;
+              res.end = async function (this: unknown, ...args: unknown[]) {
+                if (transaction) {
+                  await finishTransaction(transaction, res);
+                  await flushQueue();
+                }
+
+                origResEnd.apply(this, args);
+              };
             }
           }
 
@@ -187,21 +185,19 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
             // way to prevent it from actually being reported twice.)
             const objectifiedErr = objectify(e);
 
-            if (currentScope) {
-              currentScope.addEventProcessor(event => {
-                addExceptionMechanism(event, {
-                  type: 'instrument',
-                  handled: false,
-                  data: {
-                    wrapped_handler: wrappingTarget.name,
-                    function: 'withSentry',
-                  },
-                });
-                return event;
+            currentScope.addEventProcessor(event => {
+              addExceptionMechanism(event, {
+                type: 'instrument',
+                handled: false,
+                data: {
+                  wrapped_handler: wrappingTarget.name,
+                  function: 'withSentry',
+                },
               });
+              return event;
+            });
 
-              captureException(objectifiedErr);
-            }
+            captureException(objectifiedErr);
 
             // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet
             // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that

diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts
@@ -51,9 +51,7 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
           },
         });
 
-        if (currentScope) {
-          currentScope.setSpan(transaction);
-        }
+        currentScope.setSpan(transaction);
 
         const handleErrorCase = (e: unknown): void => {
           if (isNotFoundNavigationError(e)) {

diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts
@@ -198,10 +198,8 @@ export function requestHandler(
         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' });
-          }
+          // Set `status` of `RequestSession` to Ok, at the beginning of the request
+          scope.setRequestSession({ status: 'ok' });
         }
       });
 

diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx
@@ -217,9 +217,7 @@ export { withProfiler, Profiler, useProfiler };
 export function getActiveTransaction<T extends Transaction>(hub: Hub = getCurrentHub()): T | undefined {
   if (hub) {
     const scope = hub.getScope();
-    if (scope) {
-      return scope.getTransaction() as T | undefined;
-    }
+    return scope.getTransaction() as T | undefined;
   }
 
   return undefined;

diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts
@@ -131,11 +131,6 @@ function makeWrappedDocumentRequestFunction(
     let res: Response;
 
     const activeTransaction = getActiveTransaction();
-    const currentScope = getCurrentHub().getScope();
-
-    if (!currentScope) {
-      return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context, loadContext);
-    }
 
     try {
       const span = activeTransaction?.startChild({
@@ -176,10 +171,6 @@ function makeWrappedDataFunction(origFn: DataFunction, id: string, name: 'action
     const activeTransaction = getActiveTransaction();
     const currentScope = getCurrentHub().getScope();
 
-    if (!currentScope) {
-      return origFn.call(this, args);
-    }
-
     try {
       const span = activeTransaction?.startChild({
         op: `function.remix.${name}`,
@@ -228,17 +219,15 @@ function getTraceAndBaggage(): { sentryTrace?: string; sentryBaggage?: string }
   const currentScope = getCurrentHub().getScope();
 
   if (isNodeEnv() && hasTracingEnabled()) {
-    if (currentScope) {
-      const span = currentScope.getSpan();
+    const span = currentScope.getSpan();
 
-      if (span && transaction) {
-        const dynamicSamplingContext = transaction.getDynamicSamplingContext();
+    if (span && transaction) {
+      const dynamicSamplingContext = transaction.getDynamicSamplingContext();
 
-        return {
-          sentryTrace: span.toTraceparent(),
-          sentryBaggage: dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext),
-        };
-      }
+      return {
+        sentryTrace: span.toTraceparent(),
+        sentryBaggage: dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext),
+      };
     }
   }
 
@@ -376,16 +365,14 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui
       const url = new URL(request.url);
       const [name, source] = getTransactionName(routes, url, pkg);
 
-      if (scope) {
-        scope.setSDKProcessingMetadata({
-          request: {
-            ...normalizedRequest,
-            route: {
-              path: name,
-            },
+      scope.setSDKProcessingMetadata({
+        request: {
+          ...normalizedRequest,
+          route: {
+            path: name,
           },
-        });
-      }
+        },
+      });
 
       if (!options || !hasTracingEnabled(options)) {
         return origRequestHandler.call(this, request, loadContext);

diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts
@@ -61,9 +61,7 @@ function wrapExpressRequestHandler(
     const options = hub.getClient()?.getOptions();
     const scope = hub.getScope();
 
-    if (scope) {
-      scope.setSDKProcessingMetadata({ request });
-    }
+    scope.setSDKProcessingMetadata({ request });
 
     if (!options || !hasTracingEnabled(options) || !request.url || !request.method) {
       return origRequestHandler.call(this, req, res, next);

diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts
@@ -19,9 +19,7 @@ export function addGlobalListeners(replay: ReplayContainer): void {
   const scope = getCurrentHub().getScope();
   const client = getCurrentHub().getClient();
 
-  if (scope) {
-    scope.addScopeListener(handleScopeListener(replay));
-  }
+  scope.addScopeListener(handleScopeListener(replay));
   addInstrumentationHandler('dom', handleDomListener(replay));
   addInstrumentationHandler('history', handleHistorySpanListener(replay));
   handleNetworkBreadcrumbs(replay);

diff --git a/packages/serverless/src/awsservices.ts b/packages/serverless/src/awsservices.ts
@@ -1,5 +1,5 @@
 import { getCurrentHub } from '@sentry/node';
-import type { Integration, Span, Transaction } from '@sentry/types';
+import type { Integration, Span } from '@sentry/types';
 import { fill } from '@sentry/utils';
 // 'aws-sdk/global' import is expected to be type-only so it's erased in the final .js file.
 // When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
@@ -56,12 +56,9 @@ function wrapMakeRequest<TService extends AWSService, TResult>(
   orig: MakeRequestFunction<GenericParams, TResult>,
 ): MakeRequestFunction<GenericParams, TResult> {
   return function (this: TService, operation: string, params?: GenericParams, callback?: MakeRequestCallback<TResult>) {
-    let transaction: Transaction | undefined;
     let span: Span | undefined;
     const scope = getCurrentHub().getScope();
-    if (scope) {
-      transaction = scope.getTransaction();
-    }
+    const transaction = scope.getTransaction();
     const req = orig.call(this, operation, params);
     req.on('afterBuild', () => {
       if (transaction) {

diff --git a/packages/serverless/src/google-cloud-grpc.ts b/packages/serverless/src/google-cloud-grpc.ts
@@ -1,5 +1,5 @@
 import { getCurrentHub } from '@sentry/node';
-import type { Integration, Span, Transaction } from '@sentry/types';
+import type { Integration, Span } from '@sentry/types';
 import { fill } from '@sentry/utils';
 import type { EventEmitter } from 'events';
 
@@ -107,12 +107,9 @@ function fillGrpcFunction(stub: Stub, serviceIdentifier: string, methodName: str
         if (typeof ret?.on !== 'function') {
           return ret;
         }
-        let transaction: Transaction | undefined;
         let span: Span | undefined;
         const scope = getCurrentHub().getScope();
-        if (scope) {
-          transaction = scope.getTransaction();
-        }
+        const transaction = scope.getTransaction();
         if (transaction) {
           span = transaction.startChild({
             description: `${callType} ${methodName}`,

diff --git a/packages/serverless/src/google-cloud-http.ts b/packages/serverless/src/google-cloud-http.ts
@@ -2,7 +2,7 @@
 // When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
 import type * as common from '@google-cloud/common';
 import { getCurrentHub } from '@sentry/node';
-import type { Integration, Span, Transaction } from '@sentry/types';
+import type { Integration, Span } from '@sentry/types';
 import { fill } from '@sentry/utils';
 
 type RequestOptions = common.DecorateRequestOptions;
@@ -51,12 +51,9 @@ export class GoogleCloudHttp implements Integration {
 /** Returns a wrapped function that makes a request with tracing enabled */
 function wrapRequestFunction(orig: RequestFunction): RequestFunction {
   return function (this: common.Service, reqOpts: RequestOptions, callback: ResponseCallback): void {
-    let transaction: Transaction | undefined;
     let span: Span | undefined;
     const scope = getCurrentHub().getScope();
-    if (scope) {
-      transaction = scope.getTransaction();
-    }
+    const transaction = scope.getTransaction();
     if (transaction) {
       const httpMethod = reqOpts.method || 'GET';
       span = transaction.startChild({

diff --git a/packages/tracing-internal/src/node/integrations/apollo.ts b/packages/tracing-internal/src/node/integrations/apollo.ts
@@ -188,7 +188,7 @@ function wrapResolver(
   fill(model[resolverGroupName], resolverName, function (orig: () => unknown | Promise<unknown>) {
     return function (this: unknown, ...args: unknown[]) {
       const scope = getCurrentHub().getScope();
-      const parentSpan = scope?.getSpan();
+      const parentSpan = scope.getSpan();
       const span = parentSpan?.startChild({
         description: `${resolverGroupName}.${resolverName}`,
         op: 'graphql.resolve',