From 1fb0b4c3f6533c43ead2d2cb3f0dc4bc8a84e544 Mon Sep 17 00:00:00 2001
From: Katie Byers <katie.byers@sentry.io>
Date: Wed, 16 Dec 2020 02:45:48 -0800
Subject: [PATCH] ref(types): Loosen tag types, create new `Primitive` type
 (#3108)

* loosen tag types

* use new Primitive type rather than listing primitives

* use typeguard for isPrimitive and take advantage of that where possible

* deal with difficult-to-seriallize primitives

* remove obsolete linter directive

* remove unused NonPrimitive type
---
 MIGRATION.md                                  |  4 ++--
 .../src/integrations/globalhandlers.ts        | 15 ++++++++------
 packages/core/src/baseclient.ts               |  2 +-
 packages/hub/src/hub.ts                       |  5 +++--
 packages/hub/src/scope.ts                     |  7 ++++---
 packages/minimal/src/index.ts                 | 10 +++++++---
 packages/react/src/reactrouterv3.ts           |  6 ++++--
 packages/tracing/src/span.ts                  | 10 +++++-----
 packages/types/src/event.ts                   |  3 ++-
 packages/types/src/hub.ts                     | 11 +++++++---
 packages/types/src/index.ts                   |  2 +-
 packages/types/src/misc.ts                    |  2 ++
 packages/types/src/scope.ts                   | 12 +++++++----
 packages/types/src/span.ts                    | 16 +++++++++------
 packages/types/src/transaction.ts             |  4 ++--
 packages/utils/src/is.ts                      |  6 ++++--
 packages/utils/src/object.ts                  | 20 ++++++++++++++++++-
 17 files changed, 91 insertions(+), 44 deletions(-)

diff --git a/MIGRATION.md b/MIGRATION.md
index ded6e66e795d..ba995c7db873 100644
--- a/MIGRATION.md
+++ b/MIGRATION.md
@@ -88,11 +88,11 @@ like this:
 
 ## New Scope functions
 
-We realized how annoying it is to set a whole object using `setExtra`, that's why there are now a few new methods on the
+We realized how annoying it is to set a whole object using `setExtra`, so there are now a few new methods on the
 `Scope`.
 
 ```typescript
-setTags(tags: { [key: string]: string }): this;
+setTags(tags: { [key: string]: string | number | boolean | null | undefined }): this;
 setExtras(extras: { [key: string]: any }): this;
 clearBreadcrumbs(): this;
 ```
diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts
index b7768dea4e7a..0217e4226b53 100644
--- a/packages/browser/src/integrations/globalhandlers.ts
+++ b/packages/browser/src/integrations/globalhandlers.ts
@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/no-unsafe-member-access */
 import { getCurrentHub } from '@sentry/core';
-import { Event, Integration, Severity } from '@sentry/types';
+import { Event, Integration, Primitive, Severity } from '@sentry/types';
 import {
   addExceptionMechanism,
   addInstrumentationHandler,
@@ -152,7 +152,7 @@ export class GlobalHandlers implements Integration {
 
         const client = currentHub.getClient();
         const event = isPrimitive(error)
-          ? this._eventFromIncompleteRejection(error)
+          ? this._eventFromRejectionWithPrimitive(error)
           : eventFromUnknownInput(error, undefined, {
               attachStacktrace: client && client.getOptions().attachStacktrace,
               rejection: true,
@@ -211,16 +211,19 @@ export class GlobalHandlers implements Integration {
   }
 
   /**
-   * This function creates an Event from an TraceKitStackTrace that has part of it missing.
+   * Create an event from a promise rejection where the `reason` is a primitive.
+   *
+   * @param reason: The `reason` property of the promise rejection
+   * @returns An Event object with an appropriate `exception` value
    */
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  private _eventFromIncompleteRejection(error: any): Event {
+  private _eventFromRejectionWithPrimitive(reason: Primitive): Event {
     return {
       exception: {
         values: [
           {
             type: 'UnhandledRejection',
-            value: `Non-Error promise rejection captured with value: ${error}`,
+            // String() is needed because the Primitive type includes symbols (which can't be automatically stringified)
+            value: `Non-Error promise rejection captured with value: ${String(reason)}`,
           },
         ],
       },
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
index 8b985f07f7ab..c7b3e89032f4 100644
--- a/packages/core/src/baseclient.ts
+++ b/packages/core/src/baseclient.ts
@@ -119,7 +119,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     let eventId: string | undefined = hint && hint.event_id;
 
     const promisedEvent = isPrimitive(message)
-      ? this._getBackend().eventFromMessage(`${message}`, level, hint)
+      ? this._getBackend().eventFromMessage(String(message), level, hint)
       : this._getBackend().eventFromException(message, hint);
 
     this._process(
diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts
index 87b3e45e6351..1a3fb04ec00a 100644
--- a/packages/hub/src/hub.ts
+++ b/packages/hub/src/hub.ts
@@ -11,6 +11,7 @@ import {
   Hub as HubInterface,
   Integration,
   IntegrationClass,
+  Primitive,
   SessionContext,
   Severity,
   Span,
@@ -261,7 +262,7 @@ export class Hub implements HubInterface {
   /**
    * @inheritDoc
    */
-  public setTags(tags: { [key: string]: string }): void {
+  public setTags(tags: { [key: string]: Primitive }): void {
     const scope = this.getScope();
     if (scope) scope.setTags(tags);
   }
@@ -277,7 +278,7 @@ export class Hub implements HubInterface {
   /**
    * @inheritDoc
    */
-  public setTag(key: string, value: string): void {
+  public setTag(key: string, value: Primitive): void {
     const scope = this.getScope();
     if (scope) scope.setTag(key, value);
   }
diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts
index 11eadbb00654..0ae4ffe3a8c0 100644
--- a/packages/hub/src/scope.ts
+++ b/packages/hub/src/scope.ts
@@ -9,6 +9,7 @@ import {
   EventProcessor,
   Extra,
   Extras,
+  Primitive,
   Scope as ScopeInterface,
   ScopeContext,
   Severity,
@@ -41,7 +42,7 @@ export class Scope implements ScopeInterface {
   protected _user: User = {};
 
   /** Tags */
-  protected _tags: { [key: string]: string } = {};
+  protected _tags: { [key: string]: Primitive } = {};
 
   /** Extra */
   protected _extra: Extras = {};
@@ -124,7 +125,7 @@ export class Scope implements ScopeInterface {
   /**
    * @inheritDoc
    */
-  public setTags(tags: { [key: string]: string }): this {
+  public setTags(tags: { [key: string]: Primitive }): this {
     this._tags = {
       ...this._tags,
       ...tags,
@@ -136,7 +137,7 @@ export class Scope implements ScopeInterface {
   /**
    * @inheritDoc
    */
-  public setTag(key: string, value: string): this {
+  public setTag(key: string, value: Primitive): this {
     this._tags = { ...this._tags, [key]: value };
     this._notifyScopeListeners();
     return this;
diff --git a/packages/minimal/src/index.ts b/packages/minimal/src/index.ts
index 1406a4c408e1..117c8a769b92 100644
--- a/packages/minimal/src/index.ts
+++ b/packages/minimal/src/index.ts
@@ -6,6 +6,7 @@ import {
   Event,
   Extra,
   Extras,
+  Primitive,
   Severity,
   Transaction,
   TransactionContext,
@@ -127,7 +128,7 @@ export function setExtras(extras: Extras): void {
  * Set an object that will be merged sent as tags data with the event.
  * @param tags Tags context object to merge into current context.
  */
-export function setTags(tags: { [key: string]: string }): void {
+export function setTags(tags: { [key: string]: Primitive }): void {
   callOnHub<void>('setTags', tags);
 }
 
@@ -142,10 +143,13 @@ export function setExtra(key: string, extra: Extra): void {
 
 /**
  * Set key:value that will be sent as tags data with the event.
+ *
+ * Can also be used to unset a tag, by passing `undefined`.
+ *
  * @param key String key of tag
- * @param value String value of tag
+ * @param value Value of tag
  */
-export function setTag(key: string, value: string): void {
+export function setTag(key: string, value: Primitive): void {
   callOnHub<void>('setTag', key, value);
 }
 
diff --git a/packages/react/src/reactrouterv3.ts b/packages/react/src/reactrouterv3.ts
index 22f96ab46e24..eec999d1cfd0 100644
--- a/packages/react/src/reactrouterv3.ts
+++ b/packages/react/src/reactrouterv3.ts
@@ -1,4 +1,4 @@
-import { Transaction, TransactionContext } from '@sentry/types';
+import { Primitive, Transaction, TransactionContext } from '@sentry/types';
 import { getGlobalObject } from '@sentry/utils';
 
 import { Location, ReactRouterInstrumentation } from './types';
@@ -62,7 +62,9 @@ export function reactRouterV3Instrumentation(
           if (activeTransaction) {
             activeTransaction.finish();
           }
-          const tags: Record<string, string> = { 'routing.instrumentation': 'react-router-v3' };
+          const tags: Record<string, Primitive> = {
+            'routing.instrumentation': 'react-router-v3',
+          };
           if (prevName) {
             tags.from = prevName;
           }
diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts
index 9a20e4c152bf..dc9fbbe09ef0 100644
--- a/packages/tracing/src/span.ts
+++ b/packages/tracing/src/span.ts
@@ -1,5 +1,5 @@
 /* eslint-disable max-lines */
-import { Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
+import { Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
 import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils';
 
 import { SpanStatus } from './spanstatus';
@@ -86,7 +86,7 @@ export class Span implements SpanInterface {
   /**
    * @inheritDoc
    */
-  public tags: { [key: string]: string } = {};
+  public tags: { [key: string]: Primitive } = {};
 
   /**
    * @inheritDoc
@@ -187,7 +187,7 @@ export class Span implements SpanInterface {
   /**
    * @inheritDoc
    */
-  public setTag(key: string, value: string): this {
+  public setTag(key: string, value: Primitive): this {
     this.tags = { ...this.tags, [key]: value };
     return this;
   }
@@ -257,7 +257,7 @@ export class Span implements SpanInterface {
     parent_span_id?: string;
     span_id: string;
     status?: string;
-    tags?: { [key: string]: string };
+    tags?: { [key: string]: Primitive };
     trace_id: string;
   } {
     return dropUndefinedKeys({
@@ -284,7 +284,7 @@ export class Span implements SpanInterface {
     span_id: string;
     start_timestamp: number;
     status?: string;
-    tags?: { [key: string]: string };
+    tags?: { [key: string]: Primitive };
     timestamp?: number;
     trace_id: string;
   } {
diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts
index 0b274fd249d5..652e3a2cb1e3 100644
--- a/packages/types/src/event.ts
+++ b/packages/types/src/event.ts
@@ -2,6 +2,7 @@ import { Breadcrumb } from './breadcrumb';
 import { Contexts } from './context';
 import { Exception } from './exception';
 import { Extras } from './extra';
+import { Primitive } from './misc';
 import { Request } from './request';
 import { CaptureContext } from './scope';
 import { SdkInfo } from './sdkinfo';
@@ -35,7 +36,7 @@ export interface Event {
   stacktrace?: Stacktrace;
   breadcrumbs?: Breadcrumb[];
   contexts?: Contexts;
-  tags?: { [key: string]: string };
+  tags?: { [key: string]: Primitive };
   extra?: Extras;
   user?: User;
   type?: EventType;
diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts
index a73d2257e425..a2d3218c2e78 100644
--- a/packages/types/src/hub.ts
+++ b/packages/types/src/hub.ts
@@ -3,6 +3,7 @@ import { Client } from './client';
 import { Event, EventHint } from './event';
 import { Extra, Extras } from './extra';
 import { Integration, IntegrationClass } from './integration';
+import { Primitive } from './misc';
 import { Scope } from './scope';
 import { Session, SessionContext } from './session';
 import { Severity } from './severity';
@@ -124,16 +125,20 @@ export interface Hub {
 
   /**
    * Set an object that will be merged sent as tags data with the event.
+   *
    * @param tags Tags context object to merge into current context.
    */
-  setTags(tags: { [key: string]: string }): void;
+  setTags(tags: { [key: string]: Primitive }): void;
 
   /**
    * Set key:value that will be sent as tags data with the event.
+   *
+   * Can also be used to unset a tag, by passing `undefined`.
+   *
    * @param key String key of tag
-   * @param value String value of tag
+   * @param value Value of tag
    */
-  setTag(key: string, value: string): void;
+  setTag(key: string, value: Primitive): void;
 
   /**
    * Set key:value that will be sent as extra data with the event.
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
index 7f1733dddb28..fc288847b41a 100644
--- a/packages/types/src/index.ts
+++ b/packages/types/src/index.ts
@@ -11,7 +11,7 @@ export { Hub } from './hub';
 export { Integration, IntegrationClass } from './integration';
 export { LogLevel } from './loglevel';
 export { Mechanism } from './mechanism';
-export { ExtractedNodeRequestData, WorkerLocation } from './misc';
+export { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc';
 export { Options } from './options';
 export { Package } from './package';
 export { Request, SentryRequest, SentryRequestType } from './request';
diff --git a/packages/types/src/misc.ts b/packages/types/src/misc.ts
index 9e52b5aa6e22..0ad53d17fe14 100644
--- a/packages/types/src/misc.ts
+++ b/packages/types/src/misc.ts
@@ -59,3 +59,5 @@ export interface WorkerLocation {
   /** Synonym for `href` attribute */
   toString(): string;
 }
+
+export type Primitive = number | string | boolean | bigint | symbol | null | undefined;
diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts
index e851a44d19a6..77c6204e499d 100644
--- a/packages/types/src/scope.ts
+++ b/packages/types/src/scope.ts
@@ -2,6 +2,7 @@ import { Breadcrumb } from './breadcrumb';
 import { Context, Contexts } from './context';
 import { EventProcessor } from './eventprocessor';
 import { Extra, Extras } from './extra';
+import { Primitive } from './misc';
 import { Session } from './session';
 import { Severity } from './severity';
 import { Span } from './span';
@@ -17,7 +18,7 @@ export interface ScopeContext {
   level: Severity;
   extra: Extras;
   contexts: Contexts;
-  tags: { [key: string]: string };
+  tags: { [key: string]: Primitive };
   fingerprint: string[];
 }
 
@@ -45,14 +46,17 @@ export interface Scope {
    * Set an object that will be merged sent as tags data with the event.
    * @param tags Tags context object to merge into current context.
    */
-  setTags(tags: { [key: string]: string }): this;
+  setTags(tags: { [key: string]: Primitive }): this;
 
   /**
    * Set key:value that will be sent as tags data with the event.
+   *
+   * Can also be used to unset a tag by passing `undefined`.
+   *
    * @param key String key of tag
-   * @param value String value of tag
+   * @param value Value of tag
    */
-  setTag(key: string, value: string): this;
+  setTag(key: string, value: Primitive): this;
 
   /**
    * Set an object that will be merged sent as extra data with the event.
diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts
index b83b2d393d94..44ac522756c3 100644
--- a/packages/types/src/span.ts
+++ b/packages/types/src/span.ts
@@ -1,3 +1,4 @@
+import { Primitive } from './misc';
 import { Transaction } from './transaction';
 
 /** Interface holding all properties that can be set on a Span on creation. */
@@ -41,7 +42,7 @@ export interface SpanContext {
   /**
    * Tags of the Span.
    */
-  tags?: { [key: string]: string };
+  tags?: { [key: string]: Primitive };
 
   /**
    * Data of the Span.
@@ -79,7 +80,7 @@ export interface Span extends SpanContext {
   /**
    * @inheritDoc
    */
-  tags: { [key: string]: string };
+  tags: { [key: string]: Primitive };
 
   /**
    * @inheritDoc
@@ -98,11 +99,14 @@ export interface Span extends SpanContext {
   finish(endTimestamp?: number): void;
 
   /**
-   * Sets the tag attribute on the current span
+   * Sets the tag attribute on the current span.
+   *
+   * Can also be used to unset a tag, by passing `undefined`.
+   *
    * @param key Tag key
    * @param value Tag value
    */
-  setTag(key: string, value: string): this;
+  setTag(key: string, value: Primitive): this;
 
   /**
    * Sets the data attribute on the current span
@@ -156,7 +160,7 @@ export interface Span extends SpanContext {
     parent_span_id?: string;
     span_id: string;
     status?: string;
-    tags?: { [key: string]: string };
+    tags?: { [key: string]: Primitive };
     trace_id: string;
   };
   /** Convert the object to JSON */
@@ -168,7 +172,7 @@ export interface Span extends SpanContext {
     span_id: string;
     start_timestamp: number;
     status?: string;
-    tags?: { [key: string]: string };
+    tags?: { [key: string]: Primitive };
     timestamp?: number;
     trace_id: string;
   };
diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts
index f212fad6701e..faa264bec985 100644
--- a/packages/types/src/transaction.ts
+++ b/packages/types/src/transaction.ts
@@ -1,4 +1,4 @@
-import { ExtractedNodeRequestData, WorkerLocation } from './misc';
+import { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc';
 import { Span, SpanContext } from './span';
 
 /**
@@ -51,7 +51,7 @@ export interface Transaction extends TransactionContext, Span {
   /**
    * @inheritDoc
    */
-  tags: { [key: string]: string };
+  tags: { [key: string]: Primitive };
 
   /**
    * @inheritDoc
diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts
index 4e563ba43649..08b90c1cf748 100644
--- a/packages/utils/src/is.ts
+++ b/packages/utils/src/is.ts
@@ -1,5 +1,7 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 /* eslint-disable @typescript-eslint/explicit-module-boundary-types */
+
+import { Primitive } from '@sentry/types';
 /**
  * Checks whether given value's type is one of a few Error or Error-like
  * {@link isError}.
@@ -65,13 +67,13 @@ export function isString(wat: any): boolean {
 }
 
 /**
- * Checks whether given value's is a primitive (undefined, null, number, boolean, string)
+ * Checks whether given value's is a primitive (undefined, null, number, boolean, string, bigint, symbol)
  * {@link isPrimitive}.
  *
  * @param wat A value to be checked.
  * @returns A boolean representing the result.
  */
-export function isPrimitive(wat: any): boolean {
+export function isPrimitive(wat: any): wat is Primitive {
   return wat === null || (typeof wat !== 'object' && typeof wat !== 'function');
 }
 
diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts
index f1b6cc88dca1..7fe8012ab7da 100644
--- a/packages/utils/src/object.ts
+++ b/packages/utils/src/object.ts
@@ -170,7 +170,15 @@ export function normalizeToSize<T>(
   return serialized as T;
 }
 
-/** Transforms any input value into a string form, either primitive value or a type of the input */
+/**
+ * Transform any non-primitive, BigInt, or Symbol-type value into a string. Acts as a no-op on strings, numbers,
+ * booleans, null, and undefined.
+ *
+ * @param value The value to stringify
+ * @returns For non-primitive, BigInt, and Symbol-type values, a string denoting the value's type, type and value, or
+ *  type and `description` property, respectively. For non-BigInt, non-Symbol primitives, returns the original value,
+ *  unchanged.
+ */
 function serializeValue(value: any): any {
   const type = Object.prototype.toString.call(value);
 
@@ -236,6 +244,16 @@ function normalizeValue<T>(value: T, key?: any): T | string {
     return `[Function: ${getFunctionName(value)}]`;
   }
 
+  // symbols and bigints are considered primitives by TS, but aren't natively JSON-serilaizable
+
+  if (typeof value === 'symbol') {
+    return `[${String(value)}]`;
+  }
+
+  if (typeof value === 'bigint') {
+    return `[BigInt: ${String(value)}]`;
+  }
+
   return value;
 }