clarify and add comments

Author: lobsterkatie

packages/core/test/lib/base.test.ts

@@ -257,8 +257,10 @@ describe('BaseClient', () => {
       ['`Error` instance', new Error('Will I get caught twice?')],
       ['plain object', { 'Will I': 'get caught twice?' }],
       ['primitive wrapper', new String('Will I get caught twice?')],
-      // primitives aren't tested directly here because they need to be wrapped with `objectify` *before*  being passed
-      // to `captureException` (which is how we'd end up with a primitive wrapper as tested above)
+      // Primitives aren't tested directly here because they need to be wrapped with `objectify` *before*  being passed
+      // to `captureException` (which is how we'd end up with a primitive wrapper as tested above) in order for the
+      // already-seen check to work . Any primitive which is passed without being wrapped will be captured each time it
+      // is encountered, so this test doesn't apply.
     ])("doesn't capture the same exception twice - %s", (_name: string, thrown: any) => {
       const client = new TestClient({ dsn: PUBLIC_DSN });
 
@@ -354,9 +356,14 @@ describe('BaseClient', () => {
       ['`Error` instance', new Error('Will I get caught twice?')],
       ['plain object', { 'Will I': 'get caught twice?' }],
       ['primitive wrapper', new String('Will I get caught twice?')],
-      // primitives aren't tested directly here because they need to be wrapped with `objectify` *before*  being passed
-      // to `captureEvent` (which is how we'd end up with a primitive wrapper as tested above)
+      // Primitives aren't tested directly here because they need to be wrapped with `objectify` *before*  being passed
+      // to `captureEvent` (which is how we'd end up with a primitive wrapper as tested above) in order for the
+      // already-seen check to work . Any primitive which is passed without being wrapped will be captured each time it
+      // is encountered, so this test doesn't apply.
     ])("doesn't capture the same exception twice - %s", (_name: string, thrown: any) => {
+      // Note: this is the same test as in `describe(captureException)`, except with the exception already wrapped in a
+      // hint and accompanying an event. Duplicated here because some methods skip `captureException` and go straight to
+      // `captureEvent`.
       const client = new TestClient({ dsn: PUBLIC_DSN });
       const event = { exception: { values: [{ type: 'Error', message: 'Will I get caught twice?' }] } };
       const hint = { originalException: thrown };

packages/utils/src/misc.ts

@@ -252,10 +252,11 @@ export function stripUrlQueryAndFragment(urlPath: string): string {
  * normally would, which may or may not lead to them being caught again by something like the global error handler.)
  * This prevents us from actually recording it twice.
  *
- * Note: It will ignore primitives, as properties can't be set on them. {@link: Object.objectify} can be used on
- * exceptions to convert any that are primitives into their equivalent object wrapper forms so that this check will
- * always work. However, because we need to flag the exact object which will get rethrown, and because that rethrowing
- * happens outside of the event processing pipeline, the objectification must be done before the exception captured.
+ * Note: It will ignore primitives (always return `false` and not mark them as seen), as properties can't be set on
+ * them. {@link: Object.objectify} can be used on exceptions to convert any that are primitives into their equivalent
+ * object wrapper forms so that this check will always work. However, because we need to flag the exact object which
+ * will get rethrown, and because that rethrowing happens outside of the event processing pipeline, the objectification
+ * must be done before the exception captured.
  *
  * @param A thrown exception to check or flag as having been seen
  * @returns `true` if the exception has already been captured, `false` if not (with the side effect of marking it seen)

packages/utils/src/object.ts

@@ -406,7 +406,7 @@ export function dropUndefinedKeys<T>(val: T): T {
  * classes (String, Boolean, Number, etc.). Acts as the identity function on non-primitives.
  *
  * @param wat The subject of the objectification
- * @returns A version of "wat` which can safely be used with `Object` class methods
+ * @returns A version of `wat` which can safely be used with `Object` class methods
  */
 export function objectify(wat: unknown): typeof Object {
   let objectified;
@@ -428,6 +428,7 @@ export function objectify(wat: unknown): typeof Object {
       objectified = new (wat as any).constructor(wat);
       break;
 
+    // by process of elimination, at this point we know that `wat` must already be an object
     default:
       objectified = wat;
       break;

packages/utils/test/misc.test.ts

@@ -243,6 +243,7 @@ describe('checkOrSetAlreadyCaught()', () => {
   });
 
   it("recognizes exceptions it's seen before", () => {
+    // `exception` can be any object - an `Error`, a class instance, or a plain object
     const exception = { message: 'Oh, no! Charlie ate the flip-flops! :-(', __sentry_captured__: true };
 
     expect(checkOrSetAlreadyCaught(exception)).toBe(true);