Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@ 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('accepts and sends non-primitive tags', async ({ getLocalTestUrl, page }) => {
// Technically, accepting and sending non-primitive tags is a specification violation.
// This slipped through because a previous version of this test should have ensured that
// we don't accept non-primitive tags. However, the test was flawed.
// Turns out, Relay and our product handle invalid tag values gracefully.
// Our type definitions for setTag(s) also only allow primitive values.
// Therefore (to save some bundle size), we'll continue accepting and sending non-primitive
// tag values for now (but not adjust types).
// This test documents this decision, so that we know why we're accepting non-primitive tags.
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: [],
Expand Down
4 changes: 1 addition & 3 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,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 });
}

/**
Expand Down
47 changes: 39 additions & 8 deletions packages/core/test/lib/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,47 @@ 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);
});
});

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', c: 1 });
expect(scope['_tags']).toEqual({ a: 'b', c: 1 });
});

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);
});
});

test('setUser', () => {
Expand Down