feat(hub): Make scope always defined on the hub

[AbhiPrasad]

While working on #7536, and trying to abstract it into a generic Sentry.trace function I realized on unwieldy it is that hub.getScope() returns Scope | undefined.

In practice this should never happen, since in the hub constructor we default to always setting a scope.

public constructor(client?: Client, scope: Scope = new Scope(), private readonly _version: number = API_VERSION) {

In this PR I change the signature of hub.getScope to return only a Scope.

This should be a bundle size decrease, and allow us to do some nice refactors across the board.

In addition, we can now add a trace function that looks like so:

/**
 * Wraps a function with a transaction/span and finishes the span after the function is done.
 *
 */
export function trace<T>(context: TransactionContext, callback: (span: Span) => T): T {
  const ctx = { ...context };
  // If a name is set and a description is not, set the description to the name.
  if (ctx.name !== undefined && ctx.description === undefined) {
    ctx.description = ctx.name;
  }

  const hub = getCurrentHub();
  const scope = hub.getScope();

  const parentSpan = scope.getSpan();

  const activeSpan = parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
  scope.setSpan(activeSpan);

  let maybePromiseResult: T;
  try {
    maybePromiseResult = callback(activeSpan);
  } catch (e) {
    activeSpan.setStatus('internal_error');
    activeSpan.finish();
    throw e;
  }

  if (isThenable(maybePromiseResult)) {
    Promise.resolve(maybePromiseResult).then(
      () => {
        activeSpan.finish();
      },
      _e => {
        activeSpan.setStatus('internal_error');
        activeSpan.finish();
      },
    );
  } else {
    activeSpan.finish();
  }

  return maybePromiseResult;
}

[Lms24]

I really like this change!

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	20.49 KB (-0.2% 🔽)
@sentry/browser - ES5 CDN Bundle (minified)	63.72 KB (-0.21% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	19.18 KB (-0.06% 🔽)
@sentry/browser - ES6 CDN Bundle (minified)	56.76 KB (-0.29% 🔽)
@sentry/browser - Webpack (gzipped + minified)	21.65 KB (-0.07% 🔽)
@sentry/browser - Webpack (minified)	71.96 KB (-0.23% 🔽)
@sentry/react - Webpack (gzipped + minified)	21.68 KB (-0.07% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified)	52.03 KB (-0.04% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	27.8 KB (-17.67% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	26.12 KB (-0.03% 🔽)
@sentry/replay ES6 CDN Bundle (gzipped + minified)	44.22 KB (-0.03% 🔽)
@sentry/replay - Webpack (gzipped + minified)	38.28 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified)	62.44 KB (-0.01% 🔽)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified)	55.98 KB (-0.02% 🔽)

[mydea]

LGTM! Good improvement, we should (eventually) also look into making client "required".

[Mr0grog]

Just saw this! This is great, and lets me simplify my tracing wrapper, too, from the gist I posted for you a few weeks ago in #7387 (comment).

FWIW, I agree with @mydea that it would be great to see client be reliably present — it would make finding out when transactions have finished much easier, per that same discussion from a few weeks ago (if client is always present, you can reliably get the transaction’s lifecycle events, which are only on the client, and not the transaction).