ref(core): Pre-beforeSendTransaction cleanup (#6135)

This is a collection of a bunch of small and mostly dumb fixes pulled from #6121 (along with a few others I happened upon in the course of working on it), in order to make that PR easier to review. Some of the changes are cosmetic (formatting and wordsmithing test names, adding comments, etc.), some are refactors for clarity (mostly renaming of variables), one or two are test changes which will be necessary for the tests introduced in the aforementioned PR (in order to keep the `beforeSend` and `beforeSendTransaction` tests as parallel as possible, in case we ever want to parameterize them), one is pulling unneeded extra data out of a test to make it simpler, and two are actual fixes, namely:

- In the outcomes test, we were recording a session being dropped by `beforeSend`, but sessions don't go through `beforeSend`, so it's now an error event.

- In many places in the base client tests, we were using `.toBe()` rather than `toEqual()` for values (like numbers and strings) which as far as I know aren't guaranteed to be singletons (the way `true` and `false` are, for example). This switches to using `toEqual()` in those cases.

Author: lobsterkatie

packages/core/src/baseclient.ts

@@ -637,7 +637,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
       .then(prepared => {
         if (prepared === null) {
           this.recordDroppedEvent('event_processor', event.type || 'error');
-          throw new SentryError('An event processor returned null, will not send event.', 'log');
+          throw new SentryError('An event processor returned `null`, will not send event.', 'log');
         }
 
         const isInternalException = hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true;
@@ -646,7 +646,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
         }
 
         const beforeSendResult = beforeSend(prepared, hint);
-        return _ensureBeforeSendRv(beforeSendResult);
+        return _validateBeforeSendResult(beforeSendResult);
       })
       .then(processedEvent => {
         if (processedEvent === null) {
@@ -764,24 +764,26 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
 }
 
 /**
- * Verifies that return value of configured `beforeSend` is of expected type.
+ * Verifies that return value of configured `beforeSend` is of expected type, and returns the value if so.
  */
-function _ensureBeforeSendRv(rv: PromiseLike<Event | null> | Event | null): PromiseLike<Event | null> | Event | null {
-  const nullErr = '`beforeSend` method has to return `null` or a valid event.';
-  if (isThenable(rv)) {
-    return rv.then(
+function _validateBeforeSendResult(
+  beforeSendResult: PromiseLike<Event | null> | Event | null,
+): PromiseLike<Event | null> | Event | null {
+  const invalidValueError = '`beforeSend` must return `null` or a valid event.';
+  if (isThenable(beforeSendResult)) {
+    return beforeSendResult.then(
       event => {
-        if (!(isPlainObject(event) || event === null)) {
-          throw new SentryError(nullErr);
+        if (!isPlainObject(event) && event !== null) {
+          throw new SentryError(invalidValueError);
         }
         return event;
       },
       e => {
         throw new SentryError(`beforeSend rejected with ${e}`);
       },
     );
-  } else if (!(isPlainObject(rv) || rv === null)) {
-    throw new SentryError(nullErr);
+  } else if (!isPlainObject(beforeSendResult) && beforeSendResult !== null) {
+    throw new SentryError(invalidValueError);
   }
-  return rv;
+  return beforeSendResult;
 }