Skip to content

Commit

Permalink
ref: Major @sentry/utils rewrite (#1902)
Browse files Browse the repository at this point in the history
* ref: Move ExtendedError to a types package
* ref: Remove assign util in favor of Object.assign
* ref: Pass Event to sendEvent instead of just a body
* ref: Remove isArray and isNaN utils
* ref: Rewrite normalization and removed unused utils
  • Loading branch information
kamilogorek committed Feb 26, 2019
1 parent dea3e22 commit db47446
Show file tree
Hide file tree
Showing 44 changed files with 601 additions and 886 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ since we removed some methods from the public API and removed some classes from
- **breaking** [core] ref: Use `SyncPromise` internally, this reduces memory pressure by a lot.
- **breaking** [browser] ref: Removed `BrowserBackend` from default export.
- **breaking** [node] ref: Removed `BrowserBackend` from default export.
- ref: Move internal `ExtendedError` to a types package
- **breaking** [core] ref: Pass `Event` to `sendEvent` instead of already stringified data
- [utils] feat: Introduce `isSyntheticEvent` util
- **breaking** [utils] ref: remove `isArray` util in favor of `Array.isArray`
- **breaking** [utils] ref: Remove `isNaN` util in favor of `Number.isNaN`
- **breaking** [utils] ref: Remove `isFunction` util in favor of `typeof === 'function'`
- **breaking** [utils] ref: Remove `isUndefined` util in favor of `=== void 0`
- **breaking** [utils] ref: Remove `assign` util in favor of `Object.assign`
- **breaking** [utils] ref: Remove `includes` util in favor of native `includes`
- **breaking** [utils] ref: Rename `serializeKeysToEventMessage` to `keysToEventMessage`
- **breaking** [utils] ref: Rename `limitObjectDepthToSize` to `normalizeToSize` and rewrite its internals
- **breaking** [utils] ref: Rename `safeNormalize` to `normalize` and rewrite its internals
- **breaking** [utils] ref: Remove `serialize`, `deserialize`, `clone` and `serializeObject` functions
- **breaking** [utils] ref: Rewrite normalization functions by removing most of them and leaving just `normalize` and `normalizeToSize`

## 4.6.4

Expand Down
22 changes: 11 additions & 11 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { API, getCurrentHub } from '@sentry/core';
import { Breadcrumb, BreadcrumbHint, Integration, Severity } from '@sentry/types';
import { isFunction, isString } from '@sentry/utils/is';
import { isString } from '@sentry/utils/is';
import { logger } from '@sentry/utils/logger';
import { getEventDescription, getGlobalObject, parseUrl } from '@sentry/utils/misc';
import { deserialize, fill, safeNormalize } from '@sentry/utils/object';
import { includes, safeJoin } from '@sentry/utils/string';
import { fill, normalize } from '@sentry/utils/object';
import { safeJoin } from '@sentry/utils/string';
import { supportsBeacon, supportsHistory, supportsNativeFetch } from '@sentry/utils/supports';
import { BrowserClient } from '../client';
import { breadcrumbEventHandler, keypressEventHandler, wrap } from './helpers';
Expand Down Expand Up @@ -87,7 +87,7 @@ export class Breadcrumbs implements Integration {
const filterUrl = new API(dsn).getStoreEndpoint();
// if Sentry key appears in URL, don't capture it as a request
// but rather as our own 'sentry' type breadcrumb
if (filterUrl && includes(url, filterUrl)) {
if (filterUrl && url.includes(filterUrl)) {
addSentryBreadcrumb(data);
return result;
}
Expand Down Expand Up @@ -132,7 +132,7 @@ export class Breadcrumbs implements Integration {
category: 'console',
data: {
extra: {
arguments: safeNormalize(args, 3),
arguments: normalize(args, 3),
},
logger: 'console',
},
Expand All @@ -143,7 +143,7 @@ export class Breadcrumbs implements Integration {
if (level === 'assert') {
if (args[0] === false) {
breadcrumbData.message = `Assertion failed: ${safeJoin(args.slice(1), ' ') || 'console.assert'}`;
breadcrumbData.data.extra.arguments = safeNormalize(args.slice(1), 3);
breadcrumbData.data.extra.arguments = normalize(args.slice(1), 3);
}
}

Expand Down Expand Up @@ -205,7 +205,7 @@ export class Breadcrumbs implements Integration {
const filterUrl = new API(dsn).getStoreEndpoint();
// if Sentry key appears in URL, don't capture it as a request
// but rather as our own 'sentry' type breadcrumb
if (filterUrl && includes(url, filterUrl)) {
if (filterUrl && url.includes(filterUrl)) {
if (method === 'POST' && args[1] && args[1].body) {
addSentryBreadcrumb(args[1].body);
}
Expand Down Expand Up @@ -338,7 +338,7 @@ export class Breadcrumbs implements Integration {
/** JSDoc */
function wrapProp(prop: string, xhr: XMLHttpRequest): void {
// TODO: Fix XHR types
if (prop in xhr && isFunction((xhr as { [key: string]: any })[prop])) {
if (prop in xhr && typeof (xhr as { [key: string]: any })[prop] === 'function') {
fill(xhr, prop, original =>
wrap(original, {
mechanism: {
Expand Down Expand Up @@ -372,7 +372,7 @@ export class Breadcrumbs implements Integration {
const filterUrl = new API(dsn).getStoreEndpoint();
// if Sentry key appears in URL, don't capture it as a request
// but rather as our own 'sentry' type breadcrumb
if (isString(url) && (filterUrl && includes(url, filterUrl))) {
if (isString(url) && (filterUrl && url.includes(filterUrl))) {
this.__sentry_own_request__ = true;
}
}
Expand Down Expand Up @@ -424,7 +424,7 @@ export class Breadcrumbs implements Integration {
wrapProp(prop, xhr);
});

if ('onreadystatechange' in xhr && isFunction(xhr.onreadystatechange)) {
if ('onreadystatechange' in xhr && typeof xhr.onreadystatechange === 'function') {
fill(xhr, 'onreadystatechange', function(original: () => void): void {
return wrap(
original,
Expand Down Expand Up @@ -496,7 +496,7 @@ export class Breadcrumbs implements Integration {
function addSentryBreadcrumb(serializedData: string): void {
// There's always something that can go wrong with deserialization...
try {
const event: { [key: string]: any } = deserialize(serializedData);
const event: { [key: string]: any } = JSON.parse(serializedData);
Breadcrumbs.addBreadcrumb(
{
category: 'sentry',
Expand Down
6 changes: 1 addition & 5 deletions packages/browser/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { getCurrentHub } from '@sentry/core';
import { Event, Integration } from '@sentry/types';
import { logger } from '@sentry/utils/logger';
import { safeNormalize, serialize } from '@sentry/utils/object';
import { truncate } from '@sentry/utils/string';
import { addExceptionTypeValue, eventFromStacktrace } from '../parsers';
import {
Expand Down Expand Up @@ -114,10 +113,7 @@ export class GlobalHandlers implements Integration {
},
};

const fallbackValue =
typeof stacktrace.original !== 'undefined'
? `${truncate(serialize(safeNormalize(stacktrace.original)), 300)}`
: '';
const fallbackValue = stacktrace.original ? truncate(JSON.stringify(stacktrace.original), 300) : '';
const fallbackType = stacktrace.mechanism === 'onunhandledrejection' ? 'UnhandledRejection' : 'Error';

// This makes sure we have type/value in every exception
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { captureException, getCurrentHub, withScope } from '@sentry/core';
import { Event as SentryEvent, Mechanism, WrappedFunction } from '@sentry/types';
import { isFunction } from '@sentry/utils/is';
import { htmlTreeAsString } from '@sentry/utils/misc';
import { safeNormalize } from '@sentry/utils/object';
import { normalize } from '@sentry/utils/object';

const debounceDuration: number = 1000;
let keypressTimeout: number | undefined;
Expand Down Expand Up @@ -42,7 +41,8 @@ export function wrap(
} = {},
before?: WrappedFunction,
): any {
if (!isFunction(fn)) {
// tslint:disable-next-line:strict-type-predicates
if (typeof fn !== 'function') {
return fn;
}

Expand All @@ -64,7 +64,8 @@ export function wrap(
}

const sentryWrapped: WrappedFunction = function(this: any): void {
if (before && isFunction(before)) {
// tslint:disable-next-line:strict-type-predicates
if (before && typeof before === 'function') {
before.apply(this, arguments);
}

Expand Down Expand Up @@ -96,7 +97,7 @@ export function wrap(

processedEvent.extra = {
...processedEvent.extra,
arguments: safeNormalize(args, 3),
arguments: normalize(args, 3),
};

return processedEvent;
Expand Down
9 changes: 1 addition & 8 deletions packages/browser/src/integrations/linkederrors.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
import { Event, EventHint, Exception, Integration } from '@sentry/types';
import { Event, EventHint, Exception, ExtendedError, Integration } from '@sentry/types';
import { exceptionFromStacktrace } from '../parsers';
import { computeStackTrace } from '../tracekit';

const DEFAULT_KEY = 'cause';
const DEFAULT_LIMIT = 5;

/**
* Just an Error object with arbitrary attributes attached to it.
*/
interface ExtendedError extends Error {
[key: string]: any;
}

/** Adds SDK info to an event. */
export class LinkedErrors implements Integration {
/**
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/integrations/pluggable/vue.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { captureException, getCurrentHub, withScope } from '@sentry/core';
import { Event, Integration } from '@sentry/types';
import { isPlainObject, isUndefined } from '@sentry/utils/is';
import { isPlainObject } from '@sentry/utils/is';
import { logger } from '@sentry/utils/logger';
import { getGlobalObject } from '@sentry/utils/misc';

Expand Down Expand Up @@ -82,7 +82,7 @@ export class Vue implements Integration {
}
}

if (!isUndefined(info)) {
if (info !== void 0) {
metadata.lifecycleHook = info;
}

Expand Down
12 changes: 6 additions & 6 deletions packages/browser/src/parsers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Event, Exception, StackFrame } from '@sentry/types';
import { limitObjectDepthToSize, serializeKeysToEventMessage } from '@sentry/utils/object';
import { includes } from '@sentry/utils/string';
import { normalizeToSize } from '@sentry/utils/object';
import { keysToEventMessage } from '@sentry/utils/string';
import { md5 } from './md5';
import { computeStackTrace, StackFrame as TraceKitStackFrame, StackTrace as TraceKitStackTrace } from './tracekit';

Expand Down Expand Up @@ -38,10 +38,10 @@ export function eventFromPlainObject(exception: {}, syntheticException: Error |
const exceptionKeys = Object.keys(exception).sort();
const event: Event = {
extra: {
__serialized__: limitObjectDepthToSize(exception),
__serialized__: normalizeToSize(exception),
},
fingerprint: [md5(exceptionKeys.join(''))],
message: `Non-Error exception captured with keys: ${serializeKeysToEventMessage(exceptionKeys)}`,
message: `Non-Error exception captured with keys: ${keysToEventMessage(exceptionKeys)}`,
};

if (syntheticException) {
Expand Down Expand Up @@ -82,12 +82,12 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
const lastFrameFunction = localStack[localStack.length - 1].func || '';

// If stack starts with one of our API calls, remove it (starts, meaning it's the top of the stack - aka last call)
if (includes(firstFrameFunction, 'captureMessage') || includes(firstFrameFunction, 'captureException')) {
if (firstFrameFunction.includes('captureMessage') || firstFrameFunction.includes('captureException')) {
localStack = localStack.slice(1);
}

// If stack ends with one of our internal API calls, remove it (ends, meaning it's the bottom of the stack - aka top-most call)
if (includes(lastFrameFunction, 'sentryWrapped')) {
if (lastFrameFunction.includes('sentryWrapped')) {
localStack = localStack.slice(0, -1);
}

Expand Down
10 changes: 5 additions & 5 deletions packages/browser/src/tracekit.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// tslint:disable

import { isUndefined, isError, isErrorEvent } from '@sentry/utils/is';
import { isError, isErrorEvent } from '@sentry/utils/is';
import { getGlobalObject } from '@sentry/utils/misc';

/**
Expand Down Expand Up @@ -712,7 +712,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
for (var i = 0; i < maxLines; ++i) {
line = source[lineNo - i] + line;

if (!isUndefined(line)) {
if (line !== void 0) {
if ((m = reGuessFunction.exec(line))) {
return m[1];
} else if ((m = reFunctionArgNames.exec(line))) {
Expand Down Expand Up @@ -751,7 +751,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
line -= 1; // convert to 0-based index

for (var i = start; i < end; ++i) {
if (!isUndefined(source[i])) {
if (source[i] !== void 0) {
context.push(source[i]);
}
}
Expand Down Expand Up @@ -845,7 +845,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
* @memberof TraceKit.computeStackTrace
*/
function findSourceByFunctionBody(func: any) {
if (isUndefined(window && window.document)) {
if (window && window.document === void 0) {
return;
}

Expand Down Expand Up @@ -1005,7 +1005,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
// NOTE: It's messing out our integration tests in Karma, let's see if we can live with it – Kamil
// parts[4] = submatch[2];
// parts[5] = null; // no column when eval
} else if (i === 0 && !parts[5] && !isUndefined(ex.columnNumber)) {
} else if (i === 0 && !parts[5] && ex.columnNumber !== void 0) {
// FireFox uses this awesome columnNumber property for its top frame
// Also note, Firefox's column number is 0-based and everything else expects 1-based,
// so adding 1
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/transports/base.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { API } from '@sentry/core';
import { Response, Transport, TransportOptions } from '@sentry/types';
import { Event, Response, Transport, TransportOptions } from '@sentry/types';
import { SentryError } from '@sentry/utils/error';
import { PromiseBuffer } from '@sentry/utils/promisebuffer';

Expand All @@ -20,7 +20,7 @@ export abstract class BaseTransport implements Transport {
/**
* @inheritDoc
*/
public async sendEvent(_: string): Promise<Response> {
public async sendEvent(_: Event): Promise<Response> {
throw new SentryError('Transport Class has to implement `sendEvent` method');
}

Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/transports/beacon.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Response, Status } from '@sentry/types';
import { Event, Response, Status } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils/misc';
import { BaseTransport } from './base';

Expand All @@ -9,8 +9,8 @@ export class BeaconTransport extends BaseTransport {
/**
* @inheritDoc
*/
public async sendEvent(body: string): Promise<Response> {
const result = global.navigator.sendBeacon(this.url, body);
public async sendEvent(event: Event): Promise<Response> {
const result = global.navigator.sendBeacon(this.url, JSON.stringify(event));

return this.buffer.add(
Promise.resolve({
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/transports/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Response, Status } from '@sentry/types';
import { Event, Response, Status } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils/misc';
import { supportsReferrerPolicy } from '@sentry/utils/supports';
import { BaseTransport } from './base';
Expand All @@ -10,9 +10,9 @@ export class FetchTransport extends BaseTransport {
/**
* @inheritDoc
*/
public async sendEvent(body: string): Promise<Response> {
public async sendEvent(event: Event): Promise<Response> {
const defaultOptions: RequestInit = {
body,
body: JSON.stringify(event),
method: 'POST',
// Despite all stars in the sky saying that Edge supports old draft syntax, aka 'never', 'always', 'origin' and 'default
// https://caniuse.com/#feat=referrer-policy
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/transports/xhr.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Response, Status } from '@sentry/types';
import { Event, Response, Status } from '@sentry/types';
import { BaseTransport } from './base';

/** `XHR` based transport */
export class XHRTransport extends BaseTransport {
/**
* @inheritDoc
*/
public async sendEvent(body: string): Promise<Response> {
public async sendEvent(event: Event): Promise<Response> {
return this.buffer.add(
new Promise<Response>((resolve, reject) => {
const request = new XMLHttpRequest();
Expand All @@ -26,7 +26,7 @@ export class XHRTransport extends BaseTransport {
};

request.open('POST', this.url);
request.send(body);
request.send(JSON.stringify(event));
}),
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/test/integration/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function initSDK() {
// stub transport so we don't actually transmit any data
function DummyTransport() {}
DummyTransport.prototype.sendEvent = function(event) {
sentryData.push(JSON.parse(event));
sentryData.push(event);
done(sentryData);
return Promise.resolve({
status: 'success',
Expand Down
5 changes: 1 addition & 4 deletions packages/browser/test/integrations/linkederrors.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { ExtendedError } from '@sentry/types';
import { expect } from 'chai';
import { stub } from 'sinon';
import { BrowserBackend } from '../../src/backend';
import { LinkedErrors } from '../../src/integrations/linkederrors';

let linkedErrors: LinkedErrors;

interface ExtendedError extends Error {
[key: string]: any;
}

describe('LinkedErrors', () => {
beforeEach(() => {
linkedErrors = new LinkedErrors();
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/test/mocks/simpletransport.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Response, Status } from '../../src';
import { Event, Response, Status } from '../../src';
import { BaseTransport } from '../../src/transports';

export class SimpleTransport extends BaseTransport {
public async sendEvent(_: string): Promise<Response> {
public async sendEvent(_: Event): Promise<Response> {
return this.buffer.add(
Promise.resolve({
status: Status.fromHttpCode(200),
Expand Down
Loading

0 comments on commit db47446

Please sign in to comment.