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

feat: Add attachments API #5004

Merged
merged 34 commits into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ed65564
Mostly there
timfish Apr 28, 2022
8db6659
Fix tests
timfish Apr 28, 2022
0dd456f
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish Apr 28, 2022
7388581
Fix test spy and add implementation for creating attachment item
timfish Apr 28, 2022
8d70b13
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish Apr 28, 2022
dcef2ab
Load attachments from paths in node
timfish Apr 28, 2022
dbd70d1
Few more minor fixes and additions
timfish Apr 28, 2022
9325e24
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish Apr 30, 2022
1398bfc
Parse binary envelopes in tests
timfish Apr 30, 2022
23e1f90
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish Apr 30, 2022
93960c6
Change `_attachmentsFromScope` return type
timfish Apr 30, 2022
23f4b29
Much improved
timfish Apr 30, 2022
fc3303c
More simplify
timfish May 1, 2022
68e077a
Final newline is optional
timfish May 1, 2022
735812e
Fix gatsby test
timfish May 1, 2022
fe7f49f
Fix node v8/v10
timfish May 1, 2022
22a8f07
Fix node session tests
timfish May 1, 2022
242292f
Don't override filename if it's supplied
timfish May 3, 2022
6d7e979
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish May 3, 2022
d6a8e57
Improve tests
timfish May 3, 2022
0979245
Improve types
timfish May 3, 2022
6cc6d60
Couple more minor fixes
timfish May 3, 2022
232b75b
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish May 3, 2022
99446c7
More type improve
timfish May 3, 2022
6c404f1
Spread headers
timfish May 4, 2022
2e85356
Remove node attachment loading special case
timfish May 5, 2022
2d25f86
Remove the need for custom jest env
timfish May 5, 2022
1731945
Fix gatsby tests
timfish May 6, 2022
250260b
Pass through hint
timfish May 12, 2022
723bfdb
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish May 15, 2022
21a04d8
Lint
timfish May 15, 2022
870ae81
Merge remote-tracking branch 'upstream/7.x' into feat/attachments
timfish May 15, 2022
be927c8
Remove erroneous changes
timfish May 15, 2022
c8d43eb
Review changes
timfish May 16, 2022
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
8 changes: 4 additions & 4 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
/**
* @inheritDoc
*/
public sendEvent(event: Event): void {
public sendEvent(event: Event, hint?: EventHint): void {
// We only want to add the sentry event breadcrumb when the user has the breadcrumb integration installed and
// activated its `sentry` option.
// We also do not want to use the `Breadcrumbs` class here directly, because we do not want it to be included in
Expand Down Expand Up @@ -133,15 +133,15 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
);
}

super.sendEvent(event);
super.sendEvent(event, hint);
}

/**
* @inheritDoc
*/
protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event | null> {
event.platform = event.platform || 'javascript';
return super._prepareEvent(event, scope, hint);
return super._prepareEvent(event, hint, scope);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/transports/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function getNativeFetchImplementation(): FetchImpl {
* @param url report endpoint
* @param body report payload
*/
export function sendReport(url: string, body: string): void {
export function sendReport(url: string, body: string | Uint8Array): void {
Copy link
Member

Choose a reason for hiding this comment

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

Why are we changing the func signature here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

serializeEnvelope now returns string | Uint8Array. In this case it will never actually be a Uint8Array because there's no attachment but both sendBeacon and fetch take body: string | Uint8Array so it seemed the best thing to do.

const isRealNavigator = Object.prototype.toString.call(global && global.navigator) === '[object Navigator]';
const hasSendBeacon = isRealNavigator && typeof global.navigator.sendBeacon === 'function';

Expand Down
6 changes: 4 additions & 2 deletions packages/browser/test/unit/transports/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { EventEnvelope, EventItem } from '@sentry/types';
import { createEnvelope, serializeEnvelope } from '@sentry/utils';
import { TextEncoder } from 'util';

import { makeFetchTransport } from '../../../src/transports/fetch';
import { BrowserTransportOptions } from '../../../src/transports/types';
Expand All @@ -8,6 +9,7 @@ import { FetchImpl } from '../../../src/transports/utils';
const DEFAULT_FETCH_TRANSPORT_OPTIONS: BrowserTransportOptions = {
url: 'https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7',
recordDroppedEvent: () => undefined,
textEncoder: new TextEncoder(),
};

const ERROR_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
Expand Down Expand Up @@ -40,7 +42,7 @@ describe('NewFetchTransport', () => {
expect(mockFetch).toHaveBeenCalledTimes(1);

expect(mockFetch).toHaveBeenLastCalledWith(DEFAULT_FETCH_TRANSPORT_OPTIONS.url, {
body: serializeEnvelope(ERROR_ENVELOPE),
body: serializeEnvelope(ERROR_ENVELOPE, new TextEncoder()),
method: 'POST',
referrerPolicy: 'origin',
});
Expand Down Expand Up @@ -90,7 +92,7 @@ describe('NewFetchTransport', () => {

await transport.send(ERROR_ENVELOPE);
expect(mockFetch).toHaveBeenLastCalledWith(DEFAULT_FETCH_TRANSPORT_OPTIONS.url, {
body: serializeEnvelope(ERROR_ENVELOPE),
body: serializeEnvelope(ERROR_ENVELOPE, new TextEncoder()),
method: 'POST',
...REQUEST_OPTIONS,
});
Expand Down
4 changes: 3 additions & 1 deletion packages/browser/test/unit/transports/xhr.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { EventEnvelope, EventItem } from '@sentry/types';
import { createEnvelope, serializeEnvelope } from '@sentry/utils';
import { TextEncoder } from 'util';

import { BrowserTransportOptions } from '../../../src/transports/types';
import { makeXHRTransport } from '../../../src/transports/xhr';

const DEFAULT_XHR_TRANSPORT_OPTIONS: BrowserTransportOptions = {
url: 'https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7',
recordDroppedEvent: () => undefined,
textEncoder: new TextEncoder(),
};

const ERROR_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
Expand Down Expand Up @@ -64,7 +66,7 @@ describe('NewXHRTransport', () => {
expect(xhrMock.open).toHaveBeenCalledTimes(1);
expect(xhrMock.open).toHaveBeenCalledWith('POST', DEFAULT_XHR_TRANSPORT_OPTIONS.url);
expect(xhrMock.send).toHaveBeenCalledTimes(1);
expect(xhrMock.send).toHaveBeenCalledWith(serializeEnvelope(ERROR_ENVELOPE));
expect(xhrMock.send).toHaveBeenCalledWith(serializeEnvelope(ERROR_ENVELOPE, new TextEncoder()));
});

it('sets rate limit response headers', async () => {
Expand Down
32 changes: 23 additions & 9 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import {
Transport,
} from '@sentry/types';
import {
addItemToEnvelope,
checkOrSetAlreadyCaught,
createAttachmentEnvelopeItem,
dateTimestampInSeconds,
isPlainObject,
isPrimitive,
Expand Down Expand Up @@ -283,9 +285,14 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/**
* @inheritDoc
*/
public sendEvent(event: Event): void {
public sendEvent(event: Event, hint: EventHint = {}): void {
if (this._dsn) {
const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel);

for (const attachment of hint.attachments || []) {
addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment, this._options.transportOptions?.textEncoder));
}

this._sendEnvelope(env);
}
}
Expand Down Expand Up @@ -401,11 +408,11 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @param scope A scope containing event metadata.
* @returns A new event with more information.
*/
protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event | null> {
const { normalizeDepth = 3, normalizeMaxBreadth = 1_000 } = this.getOptions();
const prepared: Event = {
...event,
event_id: event.event_id || (hint && hint.event_id ? hint.event_id : uuid4()),
event_id: event.event_id || hint.event_id || uuid4(),
timestamp: event.timestamp || dateTimestampInSeconds(),
};

Expand All @@ -415,7 +422,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
// If we have scope given to us, use it as the base for further modifications.
// This allows us to prevent unnecessary copying of data if `captureContext` is not provided.
let finalScope = scope;
if (hint && hint.captureContext) {
if (hint.captureContext) {
finalScope = Scope.clone(finalScope).update(hint.captureContext);
}

Expand All @@ -425,6 +432,13 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
// This should be the last thing called, since we want that
// {@link Hub.addEventProcessor} gets the finished prepared event.
if (finalScope) {
// Collect attachments from the hint and scope
const attachments = [...(hint.attachments || []), ...finalScope.getAttachments()];

if (attachments.length) {
hint.attachments = attachments;
}

// In case we have a hub we reassign it.
result = finalScope.applyToEvent(prepared, hint);
}
Expand Down Expand Up @@ -552,7 +566,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @param hint
* @param scope
*/
protected _captureEvent(event: Event, hint?: EventHint, scope?: Scope): PromiseLike<string | undefined> {
protected _captureEvent(event: Event, hint: EventHint = {}, scope?: Scope): PromiseLike<string | undefined> {
return this._processEvent(event, hint, scope).then(
finalEvent => {
return finalEvent.event_id;
Expand All @@ -577,7 +591,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @param scope A scope containing event metadata.
* @returns A SyncPromise that resolves with the event or rejects in case event was/will not be send.
*/
protected _processEvent(event: Event, hint?: EventHint, scope?: Scope): PromiseLike<Event> {
protected _processEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event> {
const { beforeSend, sampleRate } = this.getOptions();

if (!this._isEnabled()) {
Expand All @@ -597,14 +611,14 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
);
}

return this._prepareEvent(event, scope, hint)
return this._prepareEvent(event, hint, scope)
.then(prepared => {
if (prepared === null) {
this.recordDroppedEvent('event_processor', event.type || 'error');
throw new SentryError('An event processor returned null, will not send event.');
}

const isInternalException = hint && hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true;
const isInternalException = hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true;
if (isInternalException || isTransaction || !beforeSend) {
return prepared;
}
Expand All @@ -623,7 +637,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
this._updateSessionFromEvent(session, processedEvent);
}

this.sendEvent(processedEvent);
this.sendEvent(processedEvent, hint);
return processedEvent;
})
.then(null, reason => {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export function createTransport(
};

const requestTask = (): PromiseLike<void> =>
makeRequest({ body: serializeEnvelope(filteredEnvelope) }).then(
makeRequest({ body: serializeEnvelope(filteredEnvelope, options.textEncoder) }).then(
response => {
// We don't want to throw on NOK responses, but we want to at least log them
if (response.statusCode !== undefined && (response.statusCode < 200 || response.statusCode >= 300)) {
Expand Down
11 changes: 8 additions & 3 deletions packages/core/test/lib/transports/base.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { EventEnvelope, EventItem, TransportMakeRequestResponse } from '@sentry/types';
import { createEnvelope, PromiseBuffer, resolvedSyncPromise, serializeEnvelope } from '@sentry/utils';
import { TextEncoder } from 'util';

import { createTransport } from '../../../src/transports/base';

Expand All @@ -14,6 +15,7 @@ const TRANSACTION_ENVELOPE = createEnvelope<EventEnvelope>(

const transportOptions = {
recordDroppedEvent: () => undefined, // noop
textEncoder: new TextEncoder(),
};

describe('createTransport', () => {
Expand All @@ -36,7 +38,7 @@ describe('createTransport', () => {
it('constructs a request to send to Sentry', async () => {
expect.assertions(1);
const transport = createTransport(transportOptions, req => {
expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE));
expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE, new TextEncoder()));
return resolvedSyncPromise({});
});
await transport.send(ERROR_ENVELOPE);
Expand All @@ -46,7 +48,7 @@ describe('createTransport', () => {
expect.assertions(2);

const transport = createTransport(transportOptions, req => {
expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE));
expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE, new TextEncoder()));
throw new Error();
});

Expand Down Expand Up @@ -82,7 +84,10 @@ describe('createTransport', () => {

const mockRecordDroppedEventCallback = jest.fn();

const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, mockRequestExecutor);
const transport = createTransport(
{ recordDroppedEvent: mockRecordDroppedEventCallback, textEncoder: new TextEncoder() },
mockRequestExecutor,
);

return [transport, setTransportResponse, mockRequestExecutor, mockRecordDroppedEventCallback] as const;
}
Expand Down
13 changes: 9 additions & 4 deletions packages/core/test/mocks/client.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ClientOptions, Event, Integration, Outcome, Session, Severity, SeverityLevel } from '@sentry/types';
import { ClientOptions, Event, EventHint, Integration, Outcome, Session, Severity, SeverityLevel } from '@sentry/types';
import { resolvedSyncPromise } from '@sentry/utils';
import { TextEncoder } from 'util';

import { BaseClient } from '../../src/baseclient';
import { initAndBind } from '../../src/sdk';
Expand All @@ -9,9 +10,13 @@ export function getDefaultTestClientOptions(options: Partial<TestClientOptions>
return {
integrations: [],
sendClientReports: true,
transportOptions: { textEncoder: new TextEncoder() },
transport: () =>
createTransport(
{ recordDroppedEvent: () => undefined }, // noop
{
recordDroppedEvent: () => undefined,
textEncoder: new TextEncoder(),
}, // noop
_ => resolvedSyncPromise({}),
),
stackParser: () => [],
Expand Down Expand Up @@ -62,10 +67,10 @@ export class TestClient extends BaseClient<TestClientOptions> {
return resolvedSyncPromise({ message, level });
}

public sendEvent(event: Event): void {
public sendEvent(event: Event, hint?: EventHint): void {
this.event = event;
if (this._options.enableSend) {
super.sendEvent(event);
super.sendEvent(event, hint);
return;
}
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
Expand Down
3 changes: 2 additions & 1 deletion packages/core/test/mocks/transport.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { SyncPromise } from '@sentry/utils';
import { TextEncoder } from 'util';

import { createTransport } from '../../src/transports/base';

Expand All @@ -10,7 +11,7 @@ export function makeFakeTransport(delay: number = 2000) {
let sendCalled = 0;
let sentCount = 0;
const makeTransport = () =>
createTransport({ recordDroppedEvent: () => undefined }, () => {
createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, () => {
sendCalled += 1;
return new SyncPromise(async res => {
await sleep(delay);
Expand Down
3 changes: 3 additions & 0 deletions packages/gatsby/test/integration.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import { render } from '@testing-library/react';
import { useEffect } from 'react';
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import * as React from 'react';
import { TextDecoder,TextEncoder } from 'util';

import { onClientEntry } from '../gatsby-browser';
import * as Sentry from '../src';

beforeAll(() => {
(global as any).__SENTRY_RELEASE__ = '683f3a6ab819d47d23abfca9a914c81f0524d35b';
(global as any).__SENTRY_DSN__ = 'https://examplePublicKey@o0.ingest.sentry.io/0';
(global as any).TextEncoder = TextEncoder;
(global as any).TextDecoder = TextDecoder;
});

describe('useEffect', () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/hub/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
CaptureContext,
CustomSamplingContext,
Event,
EventHint,
Extra,
Extras,
Primitive,
Expand Down Expand Up @@ -59,8 +60,8 @@ export function captureMessage(
* @param event The event to send to Sentry.
* @returns The generated eventId.
*/
export function captureEvent(event: Event): ReturnType<Hub['captureEvent']> {
return getCurrentHub().captureEvent(event);
export function captureEvent(event: Event, hint?: EventHint): ReturnType<Hub['captureEvent']> {
return getCurrentHub().captureEvent(event, hint);
}

/**
Expand Down
Loading