Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(types): Loosen tag types, create new Primitive type #3108

Merged
merged 6 commits into from
Dec 16, 2020
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
4 changes: 2 additions & 2 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
```
Expand Down
15 changes: 9 additions & 6 deletions packages/browser/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)}`,
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why the change here?

Copy link
Member Author

@lobsterkatie lobsterkatie Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because symbols and bigints can't be automatically coerced to string form (see links in PR description) - you have to do it manually with String(x).

: this._getBackend().eventFromException(message, hint);

this._process(
Expand Down
5 changes: 3 additions & 2 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
Hub as HubInterface,
Integration,
IntegrationClass,
Primitive,
SessionContext,
Severity,
Span,
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
7 changes: 4 additions & 3 deletions packages/hub/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
EventProcessor,
Extra,
Extras,
Primitive,
Scope as ScopeInterface,
ScopeContext,
Severity,
Expand Down Expand Up @@ -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 = {};
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions packages/minimal/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Event,
Extra,
Extras,
Primitive,
Severity,
Transaction,
TransactionContext,
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
6 changes: 4 additions & 2 deletions packages/react/src/reactrouterv3.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down
10 changes: 5 additions & 5 deletions packages/tracing/src/span.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -86,7 +86,7 @@ export class Span implements SpanInterface {
/**
* @inheritDoc
*/
public tags: { [key: string]: string } = {};
public tags: { [key: string]: Primitive } = {};

/**
* @inheritDoc
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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({
Expand All @@ -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;
} {
Expand Down
3 changes: 2 additions & 1 deletion packages/types/src/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 8 additions & 3 deletions packages/types/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,5 @@ export interface WorkerLocation {
/** Synonym for `href` attribute */
toString(): string;
}

export type Primitive = number | string | boolean | bigint | symbol | null | undefined;
12 changes: 8 additions & 4 deletions packages/types/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -17,7 +18,7 @@ export interface ScopeContext {
level: Severity;
extra: Extras;
contexts: Contexts;
tags: { [key: string]: string };
tags: { [key: string]: Primitive };
fingerprint: string[];
}

Expand Down Expand Up @@ -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.
Expand Down
16 changes: 10 additions & 6 deletions packages/types/src/span.ts
Original file line number Diff line number Diff line change
@@ -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. */
Expand Down Expand Up @@ -41,7 +42,7 @@ export interface SpanContext {
/**
* Tags of the Span.
*/
tags?: { [key: string]: string };
tags?: { [key: string]: Primitive };

/**
* Data of the Span.
Expand Down Expand Up @@ -79,7 +80,7 @@ export interface Span extends SpanContext {
/**
* @inheritDoc
*/
tags: { [key: string]: string };
tags: { [key: string]: Primitive };

/**
* @inheritDoc
Expand All @@ -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
Expand Down Expand Up @@ -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 */
Expand All @@ -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;
};
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/transaction.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ExtractedNodeRequestData, WorkerLocation } from './misc';
import { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc';
import { Span, SpanContext } from './span';

/**
Expand Down Expand Up @@ -51,7 +51,7 @@ export interface Transaction extends TransactionContext, Span {
/**
* @inheritDoc
*/
tags: { [key: string]: string };
tags: { [key: string]: Primitive };

/**
* @inheritDoc
Expand Down
Loading