fix(core): Ignore non-primitive values passed to setTag(s)

dev-packages/browser-integration-tests/suites/public-api/setTag/with_non_primitives/test.ts

@@ -3,19 +3,11 @@ import type { Event } from '@sentry/core';
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
 
-sentryTest('[bug] accepts non-primitive tags', async ({ getLocalTestUrl, page }) => {
-  // this is a bug that went unnoticed due to type definitions and a bad assertion
-  // TODO: We should not accept non-primitive tags. Fix this as a follow-up.
+sentryTest("doesn't accept non-primitive tags", async ({ getLocalTestUrl, page }) => {
   const url = await getLocalTestUrl({ testDir: __dirname });
 
   const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
 
   expect(eventData.message).toBe('non_primitives');
-
-  // TODO: This should be an empty object but instead, it is:
-  expect(eventData.tags).toEqual({
-    tag_1: {},
-    tag_2: [],
-    tag_3: ['a', {}],
-  });
+  expect(eventData.tags).toEqual({});
 });

dev-packages/browser-integration-tests/suites/public-api/setTags/with_non_primitives/test.ts

@@ -9,5 +9,5 @@ sentryTest('should not accept non-primitive tags', async ({ getLocalTestUrl, pag
   const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
 
   expect(eventData.message).toBe('non_primitives');
-  expect(eventData.tags).toMatchObject({});
+  expect(eventData.tags).toBeUndefined();
 });

dev-packages/node-integration-tests/suites/public-api/setTag/with-primitives/scenario.ts

@@ -14,4 +14,4 @@ Sentry.setTag('tag_4', null);
 Sentry.setTag('tag_5', undefined);
 Sentry.setTag('tag_6', -1);
 
-Sentry.captureMessage('primitive_tags');
+Sentry.captureMessage('primitive_tags-set-tag');

dev-packages/node-integration-tests/suites/public-api/setTag/with-primitives/test.ts

@@ -1,4 +1,4 @@
-import { afterAll, test } from 'vitest';
+import { afterAll, expect, test } from 'vitest';
 import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
 
 afterAll(() => {
@@ -8,15 +8,15 @@ afterAll(() => {
 test('should set primitive tags', async () => {
   await createRunner(__dirname, 'scenario.ts')
     .expect({
-      event: {
-        message: 'primitive_tags',
-        tags: {
+      event: event => {
+        expect(event.message).toBe('primitive_tags-set-tag');
+        expect(event.tags).toEqual({
           tag_1: 'foo',
           tag_2: 3.141592653589793,
           tag_3: false,
           tag_4: null,
           tag_6: -1,
-        },
+        });
       },
     })
     .start()

dev-packages/node-integration-tests/suites/public-api/setTags/with-primitives/scenario.ts

@@ -7,11 +7,6 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-Sentry.setTag('tag_1', 'foo');
-Sentry.setTag('tag_2', Math.PI);
-Sentry.setTag('tag_3', false);
-Sentry.setTag('tag_4', null);
-Sentry.setTag('tag_5', undefined);
-Sentry.setTag('tag_6', -1);
+Sentry.setTags({ tag_1: 'foo', tag_2: Math.PI, tag_3: false, tag_4: null, tag_5: undefined, tag_6: -1 });
 
-Sentry.captureMessage('primitive_tags');
+Sentry.captureMessage('primitive_tags-set-tags');

dev-packages/node-integration-tests/suites/public-api/setTags/with-primitives/test.ts

@@ -1,4 +1,4 @@
-import { afterAll, test } from 'vitest';
+import { afterAll, expect, test } from 'vitest';
 import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
 
 afterAll(() => {
@@ -8,15 +8,15 @@ afterAll(() => {
 test('should set primitive tags', async () => {
   await createRunner(__dirname, 'scenario.ts')
     .expect({
-      event: {
-        message: 'primitive_tags',
-        tags: {
+      event: event => {
+        expect(event.message).toBe('primitive_tags-set-tags');
+        expect(event.tags).toEqual({
           tag_1: 'foo',
           tag_2: 3.141592653589793,
           tag_3: false,
           tag_4: null,
           tag_6: -1,
-        },
+        });
       },
     })
     .start()

packages/core/src/scope.ts

@@ -17,7 +17,7 @@ import type { Span } from './types-hoist/span';
 import type { PropagationContext } from './types-hoist/tracing';
 import type { User } from './types-hoist/user';
 import { debug } from './utils/debug-logger';
-import { isPlainObject } from './utils/is';
+import { isPlainObject, isPrimitive } from './utils/is';
 import { merge } from './utils/merge';
 import { uuid4 } from './utils/misc';
 import { generateTraceId } from './utils/propagationContext';
@@ -279,10 +279,11 @@ export class Scope {
    * and will be sent as tags data with the event.
    */
   public setTags(tags: { [key: string]: Primitive }): this {
-    this._tags = {
-      ...this._tags,
-      ...tags,
-    };
+    for (const [key, value] of Object.entries(tags)) {
+      if (isPrimitive(value)) {
+        this._tags[key] = value;
+      }
+    }
     this._notifyScopeListeners();
     return this;
   }
@@ -291,9 +292,7 @@ export class Scope {
    * Set a single tag that will be sent as tags data with the event.
    */
   public setTag(key: string, value: Primitive): this {
-    this._tags = { ...this._tags, [key]: value };
-    this._notifyScopeListeners();
-    return this;
+    return this.setTags({ [key]: value });
   }
 
   /**

packages/core/test/lib/scope.test.ts

@@ -140,16 +140,61 @@ describe('Scope', () => {
       expect(scope['_extra']).toEqual({ a: undefined });
     });
 
-    test('setTag', () => {
-      const scope = new Scope();
-      scope.setTag('a', 'b');
-      expect(scope['_tags']).toEqual({ a: 'b' });
+    describe('setTag', () => {
+      it('sets a tag', () => {
+        const scope = new Scope();
+        scope.setTag('a', 'b');
+        expect(scope['_tags']).toEqual({ a: 'b' });
+      });
+
+      it('sets a tag with undefined', () => {
+        const scope = new Scope();
+        scope.setTag('a', 'b');
+        scope.setTag('a', undefined);
+        expect(scope['_tags']).toEqual({ a: undefined });
+      });
+
+      it('notifies scope listeners once per call', () => {
+        const scope = new Scope();
+        const listener = vi.fn();
+
+        scope.addScopeListener(listener);
+        scope.setTag('a', 'b');
+        scope.setTag('a', 'c');
+
+        expect(listener).toHaveBeenCalledTimes(2);
+      });
+
+      it('discards non-primitive values', () => {
+        const scope = new Scope();
+        // @ts-expect-error we want to test with a non-primitive value despite types not allowing it
+        scope.setTag('a', { b: 'c' });
+        expect(scope['_tags']).toEqual({});
+      });
     });
 
-    test('setTags', () => {
-      const scope = new Scope();
-      scope.setTags({ a: 'b' });
-      expect(scope['_tags']).toEqual({ a: 'b' });
+    describe('setTags', () => {
+      it('sets tags', () => {
+        const scope = new Scope();
+        scope.setTags({ a: 'b' });
+        expect(scope['_tags']).toEqual({ a: 'b' });
+      });
+
+      it('notifies scope listeners once per call', () => {
+        const scope = new Scope();
+        const listener = vi.fn();
+        scope.addScopeListener(listener);
+        scope.setTags({ a: 'b', c: 'd' });
+        scope.setTags({ a: 'e', f: 'g' });
+        expect(listener).toHaveBeenCalledTimes(2);
+      });
+
+      it('discards non-primitive values', () => {
+        const scope = new Scope();
+        // @ts-expect-error we want to test with a non-primitive value despite types not allowing it
+        scope.setTags({ a: { b: 'c' }, b: [1, 2, 3], c: new Map(), d: () => {} });
+        expect(scope['_tags']).toEqual({});
+      });
     });
 
     test('setUser', () => {