From ed65564d41202c52a8c793649d6ddcb48cf3b043 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Apr 2022 02:55:52 +0100 Subject: [PATCH 01/26] Mostly there --- packages/browser/src/client.ts | 15 ++++++-- packages/core/src/baseclient.ts | 14 ++++--- packages/core/src/envelope.ts | 4 +- packages/hub/src/scope.ts | 30 +++++++++++++++ packages/node/src/client.ts | 12 +++++- packages/node/src/transports/http.ts | 30 ++++++++++++++- packages/types/src/attachment.ts | 7 ++++ packages/types/src/envelope.ts | 10 ++++- packages/types/src/index.ts | 1 + packages/types/src/scope.ts | 7 ++++ packages/types/src/transport.ts | 2 +- packages/utils/src/attachment.ts | 6 +++ packages/utils/src/envelope.ts | 55 ++++++++++++++++++++++++++-- packages/utils/src/index.ts | 1 + 14 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 packages/types/src/attachment.ts create mode 100644 packages/utils/src/attachment.ts diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index d57b35c02d31..8f938a0e1ff0 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,6 +1,6 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; -import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; -import { getGlobalObject, logger } from '@sentry/utils'; +import { AttachmentItem, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; +import { attachmentItemFromAttachment, getGlobalObject, logger } from '@sentry/utils'; import { eventFromException, eventFromMessage } from './eventbuilder'; import { IS_DEBUG_BUILD } from './flags'; @@ -115,11 +115,18 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - protected _sendEvent(event: Event): void { + protected _sendEvent(event: Event, attachments: AttachmentItem[]): void { const integration = this.getIntegration(Breadcrumbs); if (integration) { integration.addSentryBreadcrumb(event); } - super._sendEvent(event); + super._sendEvent(event, attachments); + } + + /** + * @inheritDoc + */ + protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { + return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); } } diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 1969f2d8ef0a..fa7ceae6cced 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines */ import { Scope, Session } from '@sentry/hub'; import { + AttachmentItem, Client, ClientOptions, DsnComponents, @@ -263,9 +264,9 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public sendEvent(event: Event): void { + public sendEvent(event: Event, attachments?: AttachmentItem[]): void { if (this._dsn) { - const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); + const env = createEventEnvelope(event, this._dsn, attachments, this._options._metadata, this._options.tunnel); this.sendEnvelope(env); } } @@ -525,8 +526,8 @@ export abstract class BaseClient implements Client { * @param event The Sentry event to send */ // TODO(v7): refactor: get rid of method? - protected _sendEvent(event: Event): void { - this.sendEvent(event); + protected _sendEvent(event: Event, attachments: AttachmentItem[]): void { + this.sendEvent(event, attachments); } /** @@ -618,7 +619,7 @@ export abstract class BaseClient implements Client { this._updateSessionFromEvent(session, processedEvent); } - this._sendEvent(processedEvent); + this._sendEvent(processedEvent, this._getAttachments(scope)); return processedEvent; }) .then(null, reason => { @@ -670,6 +671,9 @@ export abstract class BaseClient implements Client { _level?: Severity | SeverityLevel, _hint?: EventHint, ): PromiseLike; + + /** */ + protected abstract _getAttachments(scope: Scope | undefined): AttachmentItem[]; } /** diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 53572c844a3e..c50a6c52b0d0 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,4 +1,5 @@ import { + AttachmentItem, DsnComponents, Event, EventEnvelope, @@ -68,6 +69,7 @@ export function createSessionEnvelope( export function createEventEnvelope( event: Event, dsn: DsnComponents, + attachments: AttachmentItem[] = [], metadata?: SdkMetadata, tunnel?: string, ): EventEnvelope { @@ -119,5 +121,5 @@ export function createEventEnvelope( }, event, ]; - return createEnvelope(envelopeHeaders, [eventItem]); + return createEnvelope(envelopeHeaders, [eventItem, ...attachments]); } diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 271827519792..36f56a9d6d8b 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,5 +1,7 @@ /* eslint-disable max-lines */ import { + Attachment, + AttachmentOptions, Breadcrumb, CaptureContext, Context, @@ -85,6 +87,9 @@ export class Scope implements ScopeInterface { /** Request Mode Session Status */ protected _requestSession?: RequestSession; + /** Attachments */ + protected _attachments: Attachment[] = []; + /** * A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get * sent to Sentry @@ -110,6 +115,7 @@ export class Scope implements ScopeInterface { newScope._fingerprint = scope._fingerprint; newScope._eventProcessors = [...scope._eventProcessors]; newScope._requestSession = scope._requestSession; + newScope._attachments = [...scope._attachments]; } return newScope; } @@ -365,6 +371,7 @@ export class Scope implements ScopeInterface { this._span = undefined; this._session = undefined; this._notifyScopeListeners(); + this._attachments = []; return this; } @@ -398,6 +405,29 @@ export class Scope implements ScopeInterface { return this; } + /** + * @inheritDoc + */ + public addAttachment(pathOrData: string | Uint8Array, options?: AttachmentOptions): this { + this._attachments.push([pathOrData, options]); + return this; + } + + /** + * @inheritDoc + */ + public getAttachments(): Attachment[] { + return this._attachments; + } + + /** + * @inheritDoc + */ + public clearAttachments(): this { + this._attachments = []; + return this; + } + /** * Applies the current context and fingerprint to the event. * Note that breadcrumbs will be added by the client. diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index e6a0fc7a43a0..447f3b133ca0 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,7 +1,7 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; -import { Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; -import { logger, resolvedSyncPromise } from '@sentry/utils'; +import { AttachmentItem, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; +import { attachmentItemFromAttachment, logger, resolvedSyncPromise } from '@sentry/utils'; import { eventFromMessage, eventFromUnknownInput } from './eventbuilder'; import { IS_DEBUG_BUILD } from './flags'; @@ -150,4 +150,12 @@ export class NodeClient extends BaseClient { this._sessionFlusher.incrementSessionStatusCount(); } } + + /** + * @inheritDoc + */ + protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { + // TODO: load attachment from path... + return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); + } } diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 0352a8232e04..27b09123c189 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -9,7 +9,9 @@ import { import { eventStatusFromHttpCode } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; +import { Readable, Writable } from 'stream'; import { URL } from 'url'; +import { createGzip } from 'zlib'; import { HTTPModule } from './http-module'; @@ -24,6 +26,22 @@ export interface NodeTransportOptions extends BaseTransportOptions { httpModule?: HTTPModule; } +// Estimated maximum size for reasonable standalone event +const GZIP_THRESHOLD = 1024 * 32; + +/** + * Gets a stream from a Uint8Array or string + * We don't have Readable.from in earlier versions of node + */ +function streamFromBody(body: Uint8Array | string): Readable { + return new Readable({ + read() { + this.push(body); + this.push(null); + }, + }); +} + /** * Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry. */ @@ -86,6 +104,14 @@ function createRequestExecutor( const { hostname, pathname, port, protocol, search } = new URL(options.url); return function makeRequest(request: TransportRequest): Promise { return new Promise((resolve, reject) => { + let bodyStream = streamFromBody(request.body); + + if (request.body.length > GZIP_THRESHOLD) { + options.headers = options.headers || {}; + options.headers['Content-Encoding'] = 'gzip'; + bodyStream = bodyStream.pipe(createGzip()); + } + const req = httpModule.request( { method: 'POST', @@ -128,7 +154,9 @@ function createRequestExecutor( ); req.on('error', reject); - req.end(request.body); + + // The docs say that HTTPModuleClientRequest is Writable but the types don't match exactly + bodyStream.pipe(req as unknown as Writable); }); }; } diff --git a/packages/types/src/attachment.ts b/packages/types/src/attachment.ts new file mode 100644 index 000000000000..f4e0d3dec666 --- /dev/null +++ b/packages/types/src/attachment.ts @@ -0,0 +1,7 @@ +export interface AttachmentOptions { + filename?: string; + contentType?: string; + attachmentType?: string; +} + +export type Attachment = [string | Uint8Array, AttachmentOptions | undefined]; diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 8e5091dbbfa9..1c52cac9f0ab 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -30,7 +30,13 @@ type EventItemHeaders = { type: 'event' | 'transaction'; sample_rates?: [{ id?: TransactionSamplingMethod; rate?: number }]; }; -type AttachmentItemHeaders = { type: 'attachment'; filename: string }; +type AttachmentItemHeaders = { + type: 'attachment'; + length: number; + filename: string; + content_type?: string; + attachment_type?: string; +}; type UserFeedbackItemHeaders = { type: 'user_report' }; type SessionItemHeaders = { type: 'session' }; type SessionAggregatesItemHeaders = { type: 'sessions' }; @@ -40,7 +46,7 @@ type ClientReportItemHeaders = { type: 'client_report' }; // We have to allow this hack for now as we pre-serialize events because we support // both store and envelope endpoints. export type EventItem = BaseEnvelopeItem; -export type AttachmentItem = BaseEnvelopeItem; +export type AttachmentItem = BaseEnvelopeItem; export type UserFeedbackItem = BaseEnvelopeItem; export type SessionItem = | BaseEnvelopeItem diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index ab2592cfe02e..b2a5f45eb2ad 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,3 +1,4 @@ +export type { Attachment, AttachmentOptions } from './attachment'; export type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport } from './clientreport'; diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index c3ee56a2a763..4d834d382697 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -1,3 +1,4 @@ +import { Attachment, AttachmentOptions } from './attachment'; import { Breadcrumb } from './breadcrumb'; import { Context, Contexts } from './context'; import { EventProcessor } from './eventprocessor'; @@ -158,4 +159,10 @@ export interface Scope { * Clears all currently set Breadcrumbs. */ clearBreadcrumbs(): this; + + addAttachment(pathOrData: string | Uint8Array, options?: AttachmentOptions): this; + + getAttachments(): Attachment[]; + + clearAttachments(): this; } diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts index 1c2449a386b5..c1d99302829f 100644 --- a/packages/types/src/transport.ts +++ b/packages/types/src/transport.ts @@ -12,7 +12,7 @@ export type Outcome = export type TransportCategory = 'error' | 'transaction' | 'attachment' | 'session'; export type TransportRequest = { - body: string; + body: string | Uint8Array; category: TransportCategory; }; diff --git a/packages/utils/src/attachment.ts b/packages/utils/src/attachment.ts new file mode 100644 index 000000000000..5d6abbc1f349 --- /dev/null +++ b/packages/utils/src/attachment.ts @@ -0,0 +1,6 @@ +import { Attachment, AttachmentItem } from '@sentry/types'; + +/** */ +export function attachmentItemFromAttachment(_attachment: Attachment): AttachmentItem { + throw new Error('Not implemented'); +} diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 0000e6007aee..712a6389b1d7 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -30,17 +30,29 @@ export function getEnvelopeType(envelope: E): string { } /** - * Serializes an envelope into a string. + * Serializes an envelope. */ -export function serializeEnvelope(envelope: Envelope): string { - const [headers, items] = envelope; - const serializedHeaders = JSON.stringify(headers); +export function serializeEnvelope(envelope: Envelope): string | Uint8Array { + const [, items] = envelope; // Have to cast items to any here since Envelope is a union type // Fixed in Typescript 4.2 // TODO: Remove any[] cast when we upgrade to TS 4.2 // https://github.com/microsoft/TypeScript/issues/36390 // eslint-disable-next-line @typescript-eslint/no-explicit-any + const hasBinaryAttachment = (items as any[]).some( + (item: typeof items[number]) => item[0].type === 'attachment' && item[1] instanceof Uint8Array, + ); + + return hasBinaryAttachment ? serializeBinaryEnvelope(envelope) : serializeStringEnvelope(envelope); +} + +function serializeStringEnvelope(envelope: Envelope): string { + const [headers, items] = envelope; + const serializedHeaders = JSON.stringify(headers); + + // TODO: Remove any[] cast when we upgrade to TS 4.2 + // eslint-disable-next-line @typescript-eslint/no-explicit-any return (items as any[]).reduce((acc, item: typeof items[number]) => { const [itemHeaders, payload] = item; // We do not serialize payloads that are primitives @@ -48,3 +60,38 @@ export function serializeEnvelope(envelope: Envelope): string { return `${acc}\n${JSON.stringify(itemHeaders)}\n${serializedPayload}`; }, serializedHeaders); } + +function serializeBinaryEnvelope(envelope: Envelope): Uint8Array { + const encoder = new TextEncoder(); + const [headers, items] = envelope; + const serializedHeaders = JSON.stringify(headers); + + const chunks = [encoder.encode(serializedHeaders)]; + + for (const item of items) { + const [itemHeaders, payload] = item as typeof items[number]; + chunks.push(encoder.encode(`\n${JSON.stringify(itemHeaders)}\n`)); + if (typeof payload === 'string') { + chunks.push(encoder.encode(payload)); + } else if (payload instanceof Uint8Array) { + chunks.push(payload); + } else { + chunks.push(encoder.encode(JSON.stringify(payload))); + } + } + + return concatBuffers(chunks); +} + +function concatBuffers(buffers: Uint8Array[]): Uint8Array { + const totalLength = buffers.reduce((acc, buf) => acc + buf.length, 0); + + const merged = new Uint8Array(totalLength); + let offset = 0; + for (const buffer of buffers) { + merged.set(buffer, offset); + offset += buffer.length; + } + + return merged; +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 0f004218c052..7bcd1b4b4edb 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -24,3 +24,4 @@ export * from './env'; export * from './envelope'; export * from './clientreport'; export * from './ratelimit'; +export * from './attachment'; From 8db66595300350a903007a06ae9002776a59f750 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Apr 2022 11:17:05 +0100 Subject: [PATCH 02/26] Fix tests --- packages/browser/src/client.ts | 9 +-------- packages/core/src/baseclient.ts | 9 ++++++--- packages/node/src/client.ts | 2 +- packages/utils/src/envelope.ts | 5 ++++- packages/utils/test/envelope.test.ts | 12 ++++++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 8f938a0e1ff0..33537c71cdb7 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,6 +1,6 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { AttachmentItem, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; -import { attachmentItemFromAttachment, getGlobalObject, logger } from '@sentry/utils'; +import { getGlobalObject, logger } from '@sentry/utils'; import { eventFromException, eventFromMessage } from './eventbuilder'; import { IS_DEBUG_BUILD } from './flags'; @@ -122,11 +122,4 @@ export class BrowserClient extends BaseClient { } super._sendEvent(event, attachments); } - - /** - * @inheritDoc - */ - protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { - return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); - } } diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index fa7ceae6cced..4ec5296f25b4 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -16,6 +16,7 @@ import { Transport, } from '@sentry/types'; import { + attachmentItemFromAttachment, checkOrSetAlreadyCaught, dateTimestampInSeconds, isPlainObject, @@ -656,6 +657,11 @@ export abstract class BaseClient implements Client { ); } + /** */ + protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { + return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); + } + /** * @inheritDoc */ @@ -671,9 +677,6 @@ export abstract class BaseClient implements Client { _level?: Severity | SeverityLevel, _hint?: EventHint, ): PromiseLike; - - /** */ - protected abstract _getAttachments(scope: Scope | undefined): AttachmentItem[]; } /** diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 447f3b133ca0..e491aec23960 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -155,7 +155,7 @@ export class NodeClient extends BaseClient { * @inheritDoc */ protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { - // TODO: load attachment from path... + // TODO: load any attachments from paths... return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); } } diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 712a6389b1d7..f28fb4264128 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -47,7 +47,10 @@ export function serializeEnvelope(envelope: Envelope): string | Uint8Array { return hasBinaryAttachment ? serializeBinaryEnvelope(envelope) : serializeStringEnvelope(envelope); } -function serializeStringEnvelope(envelope: Envelope): string { +/** + * Serializes an envelope to a string. + */ +export function serializeStringEnvelope(envelope: Envelope): string { const [headers, items] = envelope; const serializedHeaders = JSON.stringify(headers); diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index dab2b92d5f47..8c1ea71e9c7a 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -1,6 +1,6 @@ import { EventEnvelope } from '@sentry/types'; -import { addItemToEnvelope, createEnvelope, getEnvelopeType, serializeEnvelope } from '../src/envelope'; +import { addItemToEnvelope, createEnvelope, getEnvelopeType, serializeStringEnvelope } from '../src/envelope'; import { parseEnvelope } from './testutils'; describe('envelope', () => { @@ -17,10 +17,10 @@ describe('envelope', () => { }); }); - describe('serializeEnvelope()', () => { + describe('serializeStringEnvelope()', () => { it('serializes an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - expect(serializeEnvelope(env)).toMatchInlineSnapshot( + expect(serializeStringEnvelope(env)).toMatchInlineSnapshot( '"{\\"event_id\\":\\"aa3ff046696b4bc6b609ce6d28fde9e2\\",\\"sent_at\\":\\"123\\"}"', ); }); @@ -29,7 +29,7 @@ describe('envelope', () => { describe('addItemToEnvelope()', () => { it('adds an item to an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - const parsedEnvelope = parseEnvelope(serializeEnvelope(env)); + const parsedEnvelope = parseEnvelope(serializeStringEnvelope(env)); expect(parsedEnvelope).toHaveLength(1); expect(parsedEnvelope[0]).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); @@ -37,7 +37,7 @@ describe('envelope', () => { { type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }, ]); - const parsedNewEnvelope = parseEnvelope(serializeEnvelope(newEnv)); + const parsedNewEnvelope = parseEnvelope(serializeStringEnvelope(newEnv)); expect(parsedNewEnvelope).toHaveLength(3); expect(parsedNewEnvelope[0]).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); expect(parsedNewEnvelope[1]).toEqual({ type: 'event' }); @@ -49,7 +49,7 @@ describe('envelope', () => { it('returns the type of the envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], - [{ type: 'attachment', filename: 'me.txt' }, '123456'], + [{ type: 'attachment', filename: 'me.txt', length: 6 }, new Uint8Array(6)], ]); expect(getEnvelopeType(env)).toEqual('event'); }); From 73885816df89023faf66a23c45a56d7f44474cb3 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Apr 2022 11:54:24 +0100 Subject: [PATCH 03/26] Fix test spy and add implementation for creating attachment item --- packages/core/src/baseclient.ts | 4 ++-- packages/node/src/client.ts | 4 ++-- packages/node/test/index.test.ts | 6 +++--- packages/utils/src/attachment.ts | 17 +++++++++++++++-- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 4ec5296f25b4..0fa2f2e998ef 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -16,8 +16,8 @@ import { Transport, } from '@sentry/types'; import { - attachmentItemFromAttachment, checkOrSetAlreadyCaught, + createAttachmentEnvelopeItem, dateTimestampInSeconds, isPlainObject, isPrimitive, @@ -659,7 +659,7 @@ export abstract class BaseClient implements Client { /** */ protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { - return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); + return (scope?.getAttachments() || []).map(a => createAttachmentEnvelopeItem(a)); } /** diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index e491aec23960..f14663b0ecba 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,7 +1,7 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; import { AttachmentItem, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; -import { attachmentItemFromAttachment, logger, resolvedSyncPromise } from '@sentry/utils'; +import { createAttachmentEnvelopeItem, logger, resolvedSyncPromise } from '@sentry/utils'; import { eventFromMessage, eventFromUnknownInput } from './eventbuilder'; import { IS_DEBUG_BUILD } from './flags'; @@ -156,6 +156,6 @@ export class NodeClient extends BaseClient { */ protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { // TODO: load any attachments from paths... - return (scope?.getAttachments() || []).map(a => attachmentItemFromAttachment(a)); + return (scope?.getAttachments() || []).map(a => createAttachmentEnvelopeItem(a)); } } diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 502542a25491..1caed705577e 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,6 +1,6 @@ import { initAndBind, SDK_VERSION } from '@sentry/core'; import { getMainCarrier } from '@sentry/hub'; -import { Integration } from '@sentry/types'; +import { AttachmentItem, Integration } from '@sentry/types'; import { createStackParser } from '@sentry/utils'; import * as domain from 'domain'; @@ -79,7 +79,7 @@ describe('SentryNode', () => { }); describe('breadcrumbs', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); @@ -110,7 +110,7 @@ describe('SentryNode', () => { }); describe('capture', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); diff --git a/packages/utils/src/attachment.ts b/packages/utils/src/attachment.ts index 5d6abbc1f349..eb5ebc4b91fa 100644 --- a/packages/utils/src/attachment.ts +++ b/packages/utils/src/attachment.ts @@ -1,6 +1,19 @@ import { Attachment, AttachmentItem } from '@sentry/types'; /** */ -export function attachmentItemFromAttachment(_attachment: Attachment): AttachmentItem { - throw new Error('Not implemented'); +export function createAttachmentEnvelopeItem(attachment: Attachment): AttachmentItem { + const [pathOrData, options] = attachment; + + const buffer = typeof pathOrData === 'string' ? new TextEncoder().encode(pathOrData) : pathOrData; + + return [ + { + type: 'attachment', + length: buffer.length, + filename: options?.filename || 'No filename', + content_type: options?.contentType, + attachment_type: options?.attachmentType, + }, + buffer, + ]; } From dcef2ab51a6f698f2e156f5bb50661644e3b3960 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Apr 2022 12:06:05 +0100 Subject: [PATCH 04/26] Load attachments from paths in node --- packages/core/src/baseclient.ts | 6 +++--- packages/node/src/client.ts | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 0fa2f2e998ef..542fe25b8263 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -527,7 +527,7 @@ export abstract class BaseClient implements Client { * @param event The Sentry event to send */ // TODO(v7): refactor: get rid of method? - protected _sendEvent(event: Event, attachments: AttachmentItem[]): void { + protected _sendEvent(event: Event, attachments?: AttachmentItem[]): void { this.sendEvent(event, attachments); } @@ -658,8 +658,8 @@ export abstract class BaseClient implements Client { } /** */ - protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { - return (scope?.getAttachments() || []).map(a => createAttachmentEnvelopeItem(a)); + protected _getAttachments(scope: Scope | undefined): AttachmentItem[] | undefined { + return scope?.getAttachments()?.map(a => createAttachmentEnvelopeItem(a)); } /** diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index f14663b0ecba..a08707fb4d19 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,7 +1,8 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; import { AttachmentItem, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; -import { createAttachmentEnvelopeItem, logger, resolvedSyncPromise } from '@sentry/utils'; +import { basename, createAttachmentEnvelopeItem, logger, resolvedSyncPromise } from '@sentry/utils'; +import { existsSync, readFileSync } from 'fs'; import { eventFromMessage, eventFromUnknownInput } from './eventbuilder'; import { IS_DEBUG_BUILD } from './flags'; @@ -154,8 +155,17 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _getAttachments(scope: Scope | undefined): AttachmentItem[] { - // TODO: load any attachments from paths... - return (scope?.getAttachments() || []).map(a => createAttachmentEnvelopeItem(a)); + protected _getAttachments(scope: Scope | undefined): AttachmentItem[] | undefined { + return scope?.getAttachments()?.map(attachment => { + let [pathOrData, options] = attachment; + + if (typeof pathOrData === 'string' && existsSync(pathOrData)) { + options = options || {}; + options.filename = basename(pathOrData); + pathOrData = readFileSync(pathOrData); + } + + return createAttachmentEnvelopeItem([pathOrData, options]); + }); } } From dbd70d11dddae2c806d96976691edd4d27daa67f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Apr 2022 12:15:55 +0100 Subject: [PATCH 05/26] Few more minor fixes and additions --- packages/core/src/baseclient.ts | 8 +++++--- packages/node/src/client.ts | 2 +- packages/node/src/transports/http.ts | 2 +- packages/types/src/scope.ts | 11 +++++++++++ packages/utils/src/attachment.ts | 19 ------------------- packages/utils/src/envelope.ts | 22 +++++++++++++++++++++- packages/utils/src/index.ts | 1 - 7 files changed, 39 insertions(+), 26 deletions(-) delete mode 100644 packages/utils/src/attachment.ts diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 542fe25b8263..b13496b5da4b 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -620,7 +620,7 @@ export abstract class BaseClient implements Client { this._updateSessionFromEvent(session, processedEvent); } - this._sendEvent(processedEvent, this._getAttachments(scope)); + this._sendEvent(processedEvent, this._attachmentsFromScope(scope)); return processedEvent; }) .then(null, reason => { @@ -657,8 +657,10 @@ export abstract class BaseClient implements Client { ); } - /** */ - protected _getAttachments(scope: Scope | undefined): AttachmentItem[] | undefined { + /** + * Loads attachment items from scope + */ + protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] | undefined { return scope?.getAttachments()?.map(a => createAttachmentEnvelopeItem(a)); } diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index a08707fb4d19..0383c5ad1b3c 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -155,7 +155,7 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _getAttachments(scope: Scope | undefined): AttachmentItem[] | undefined { + protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] | undefined { return scope?.getAttachments()?.map(attachment => { let [pathOrData, options] = attachment; diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 3ba17e9b6cbf..67a8b870ddf5 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -30,7 +30,7 @@ const GZIP_THRESHOLD = 1024 * 32; /** * Gets a stream from a Uint8Array or string - * We don't have Readable.from in earlier versions of node + * Readable.from was added in node.sj v12.3.0, v10.17.0 */ function streamFromBody(body: Uint8Array | string): Readable { return new Readable({ diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index 4d834d382697..f3b990dd591c 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -160,9 +160,20 @@ export interface Scope { */ clearBreadcrumbs(): this; + /** + * Adds an attachment to the scope + * @param pathOrData A Uint8Array containing the attachment bytes + * @param options Attachment options + */ addAttachment(pathOrData: string | Uint8Array, options?: AttachmentOptions): this; + /** + * Returns an array of attachments on the scope + */ getAttachments(): Attachment[]; + /** + * Clears attachments from the scope + */ clearAttachments(): this; } diff --git a/packages/utils/src/attachment.ts b/packages/utils/src/attachment.ts deleted file mode 100644 index eb5ebc4b91fa..000000000000 --- a/packages/utils/src/attachment.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Attachment, AttachmentItem } from '@sentry/types'; - -/** */ -export function createAttachmentEnvelopeItem(attachment: Attachment): AttachmentItem { - const [pathOrData, options] = attachment; - - const buffer = typeof pathOrData === 'string' ? new TextEncoder().encode(pathOrData) : pathOrData; - - return [ - { - type: 'attachment', - length: buffer.length, - filename: options?.filename || 'No filename', - content_type: options?.contentType, - attachment_type: options?.attachmentType, - }, - buffer, - ]; -} diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index f28fb4264128..4316bee05a39 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -1,4 +1,4 @@ -import { Envelope } from '@sentry/types'; +import { Attachment, AttachmentItem, Envelope } from '@sentry/types'; import { isPrimitive } from './is'; @@ -98,3 +98,23 @@ function concatBuffers(buffers: Uint8Array[]): Uint8Array { return merged; } + +/** + * Creates attachment envelope items + */ +export function createAttachmentEnvelopeItem(attachment: Attachment): AttachmentItem { + const [pathOrData, options] = attachment; + + const buffer = typeof pathOrData === 'string' ? new TextEncoder().encode(pathOrData) : pathOrData; + + return [ + { + type: 'attachment', + length: buffer.length, + filename: options?.filename || 'No filename', + content_type: options?.contentType, + attachment_type: options?.attachmentType, + }, + buffer, + ]; +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index dff873a4c4a8..01b875dc31c3 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -23,4 +23,3 @@ export * from './env'; export * from './envelope'; export * from './clientreport'; export * from './ratelimit'; -export * from './attachment'; From 1398bfcec33441dfce34777c52d1535c90dcd67f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 30 Apr 2022 21:19:59 +0100 Subject: [PATCH 06/26] Parse binary envelopes in tests --- packages/utils/test/envelope.test.ts | 43 +++++++++++++++------ packages/utils/test/testutils.ts | 57 +++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 14 deletions(-) diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index 855485760fe9..32cb88aefd71 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -1,6 +1,12 @@ import { EventEnvelope } from '@sentry/types'; -import { addItemToEnvelope, createEnvelope, forEachEnvelopeItem, serializeStringEnvelope } from '../src/envelope'; +import { + addItemToEnvelope, + createEnvelope, + forEachEnvelopeItem, + serializeStringEnvelope, + serializeEnvelope, +} from '../src/envelope'; import { parseEnvelope } from './testutils'; describe('envelope', () => { @@ -29,19 +35,19 @@ describe('envelope', () => { describe('addItemToEnvelope()', () => { it('adds an item to an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - const parsedEnvelope = parseEnvelope(serializeStringEnvelope(env)); - expect(parsedEnvelope).toHaveLength(1); - expect(parsedEnvelope[0]).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); + let [envHeaders, items] = parseEnvelope(serializeEnvelope(env)); + expect(items).toHaveLength(0); + expect(envHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); const newEnv = addItemToEnvelope(env, [ { type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }, ]); - const parsedNewEnvelope = parseEnvelope(serializeStringEnvelope(newEnv)); - expect(parsedNewEnvelope).toHaveLength(3); - expect(parsedNewEnvelope[0]).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); - expect(parsedNewEnvelope[1]).toEqual({ type: 'event' }); - expect(parsedNewEnvelope[2]).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }); + + [envHeaders, items] = parseEnvelope(serializeEnvelope(newEnv)); + expect(envHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); + expect(items).toHaveLength(1); + expect(items[0]).toEqual([{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]); }); }); @@ -49,8 +55,8 @@ describe('envelope', () => { it('loops through an envelope', () => { const items: EventEnvelope[1] = [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], - [{ type: 'attachment', filename: 'bar.txt', length: 6 }, new Uint8Array(6)], - [{ type: 'attachment', filename: 'foo.txt', length: 6 }, new Uint8Array(6)], + [{ type: 'attachment', filename: 'bar.txt', length: 6 }, Uint8Array.from([1, 2, 3, 4, 5, 6])], + [{ type: 'attachment', filename: 'foo.txt', length: 6 }, Uint8Array.from([7, 8, 9, 10, 11, 12])], ]; const env = createEnvelope( @@ -58,7 +64,7 @@ describe('envelope', () => { items, ); - expect.assertions(6); + expect.assertions(11); let iteration = 0; forEachEnvelopeItem(env, (item, type) => { @@ -66,6 +72,19 @@ describe('envelope', () => { expect(type).toBe(items[iteration][0].type); iteration = iteration + 1; }); + + const [parsedHeaders, parsedItems] = parseEnvelope(serializeEnvelope(env)); + expect(parsedHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); + expect(parsedItems).toHaveLength(3); + expect(items[0]).toEqual([{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]); + expect(items[1]).toEqual([ + { type: 'attachment', filename: 'bar.txt', length: 6 }, + Uint8Array.from([1, 2, 3, 4, 5, 6]), + ]); + expect(items[2]).toEqual([ + { type: 'attachment', filename: 'foo.txt', length: 6 }, + Uint8Array.from([7, 8, 9, 10, 11, 12]), + ]); }); }); }); diff --git a/packages/utils/test/testutils.ts b/packages/utils/test/testutils.ts index aa3c5485eec1..c87aa0384b53 100644 --- a/packages/utils/test/testutils.ts +++ b/packages/utils/test/testutils.ts @@ -1,3 +1,5 @@ +import { Envelope, BaseEnvelopeHeaders, BaseEnvelopeItemHeaders } from '@sentry/types'; + export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { const currentNodeVersion = process.env.NODE_VERSION; @@ -12,6 +14,57 @@ export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { return it; }; -export function parseEnvelope(env: string): Array> { - return env.split('\n').map(e => JSON.parse(e)); +/** + * A naive binary envelope parser + */ +export function parseEnvelope(env: string | Uint8Array): Envelope { + if (typeof env === 'string') { + env = new TextEncoder().encode(env); + } + + let envelopeHeaders: BaseEnvelopeHeaders | undefined; + let lastItemHeader: BaseEnvelopeItemHeaders | undefined; + const items: [any, any][] = []; + + let binaryLength = 0; + while (env.length) { + // Next length is either the binary length from the previous header + // or the next newline character + let i = binaryLength || env.indexOf(0xa); + + // If no newline was found, assume this is the last block + if (i < 0) { + i = env.length; + } + + // If we read out a length in the previous header, assume binary + if (binaryLength > 0) { + const bin = env.slice(0, binaryLength); + binaryLength = 0; + items.push([lastItemHeader, bin]); + } else { + const json = JSON.parse(new TextDecoder().decode(env.slice(0, i + 1))); + + if (typeof json.length === 'number') { + binaryLength = json.length; + } + + // First json is always the envelope headers + if (!envelopeHeaders) { + envelopeHeaders = json; + } else { + // If there is a type property, assume this is an item header + if ('type' in json) { + lastItemHeader = json; + } else { + items.push([lastItemHeader, json]); + } + } + } + + // Replace the buffer with the previous block and newline removed + env = env.slice(i + 1); + } + + return [envelopeHeaders as BaseEnvelopeHeaders, items]; } From 93960c658e0b68881662443ac7461a5228484ada Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 30 Apr 2022 21:27:52 +0100 Subject: [PATCH 07/26] Change `_attachmentsFromScope` return type --- packages/core/src/baseclient.ts | 4 ++-- packages/node/src/client.ts | 24 +++++++++++++----------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index d832a61e992d..a315283cebb4 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -664,8 +664,8 @@ export abstract class BaseClient implements Client { /** * Loads attachment items from scope */ - protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] | undefined { - return scope?.getAttachments()?.map(a => createAttachmentEnvelopeItem(a)); + protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] { + return scope?.getAttachments()?.map(a => createAttachmentEnvelopeItem(a)) || []; } /** diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 0383c5ad1b3c..5ddafe8efdfd 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -155,17 +155,19 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] | undefined { - return scope?.getAttachments()?.map(attachment => { - let [pathOrData, options] = attachment; - - if (typeof pathOrData === 'string' && existsSync(pathOrData)) { - options = options || {}; - options.filename = basename(pathOrData); - pathOrData = readFileSync(pathOrData); - } + protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] { + return ( + scope?.getAttachments()?.map(attachment => { + let [pathOrData, options] = attachment; + + if (typeof pathOrData === 'string' && existsSync(pathOrData)) { + options = options || {}; + options.filename = basename(pathOrData); + pathOrData = readFileSync(pathOrData); + } - return createAttachmentEnvelopeItem([pathOrData, options]); - }); + return createAttachmentEnvelopeItem([pathOrData, options]); + }) || [] + ); } } From 23f4b29a86d71dfd10fc9d88a0cbf10bfe99a15f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 30 Apr 2022 23:23:27 +0100 Subject: [PATCH 08/26] Much improved --- packages/browser/jest.config.js | 2 +- packages/browser/jest.env.js | 15 +++ packages/browser/src/client.ts | 4 +- packages/core/src/baseclient.ts | 20 ++-- packages/core/src/envelope.ts | 4 +- packages/core/src/transports/base.ts | 2 +- packages/hub/src/scope.ts | 5 +- packages/node/src/client.ts | 17 ++-- packages/node/src/transports/http.ts | 3 + packages/node/test/index.test.ts | 6 +- packages/node/test/transports/http.test.ts | 3 +- packages/node/test/transports/https.test.ts | 3 +- packages/types/src/attachment.ts | 6 +- packages/types/src/index.ts | 2 +- packages/types/src/scope.ts | 7 +- packages/types/src/transport.ts | 1 + packages/utils/src/envelope.ts | 104 +++++++++----------- packages/utils/test/clientreport.test.ts | 23 +++-- packages/utils/test/envelope.test.ts | 15 +-- packages/utils/test/testutils.ts | 18 ++-- 20 files changed, 137 insertions(+), 123 deletions(-) create mode 100644 packages/browser/jest.env.js diff --git a/packages/browser/jest.config.js b/packages/browser/jest.config.js index f9cd8056a454..264feb13e33a 100644 --- a/packages/browser/jest.config.js +++ b/packages/browser/jest.config.js @@ -2,6 +2,6 @@ const baseConfig = require('../../jest/jest.config.js'); module.exports = { ...baseConfig, - testEnvironment: 'jsdom', + testEnvironment: './jest.env.js', testMatch: ['/test/unit/**/*.test.ts'], }; diff --git a/packages/browser/jest.env.js b/packages/browser/jest.env.js new file mode 100644 index 000000000000..3a7ac5019468 --- /dev/null +++ b/packages/browser/jest.env.js @@ -0,0 +1,15 @@ +const Environment = require('jest-environment-jsdom'); + +// Looks like jsdom does not support global TextEncoder/TextDecoder +// https://github.com/jsdom/jsdom/issues/2524 + +module.exports = class CustomTestEnvironment extends Environment { + async setup() { + await super.setup(); + if (typeof this.global.TextEncoder === 'undefined') { + const { TextEncoder, TextDecoder } = require('util'); + this.global.TextEncoder = TextEncoder; + this.global.TextDecoder = TextDecoder; + } + } +}; diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 7601856df186..3eaf26a73066 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,5 +1,5 @@ import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core'; -import { AttachmentItem, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; +import { Attachment, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; import { createClientReportEnvelope, dsnToString, getGlobalObject, logger, serializeEnvelope } from '@sentry/utils'; import { eventFromException, eventFromMessage } from './eventbuilder'; @@ -103,7 +103,7 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - protected _sendEvent(event: Event, attachments: AttachmentItem[]): void { + protected _sendEvent(event: Event, attachments: Attachment[]): void { const integration = this.getIntegration(Breadcrumbs); if (integration) { integration.addSentryBreadcrumb(event); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index a315283cebb4..d9393c3e938e 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ import { Scope, Session } from '@sentry/hub'; import { - AttachmentItem, + Attachment, Client, ClientOptions, DataCategory, @@ -19,6 +19,7 @@ import { Transport, } from '@sentry/types'; import { + addItemToEnvelope, checkOrSetAlreadyCaught, createAttachmentEnvelopeItem, dateTimestampInSeconds, @@ -275,9 +276,14 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public sendEvent(event: Event, attachments?: AttachmentItem[]): void { + public sendEvent(event: Event, attachments?: Attachment[]): void { if (this._dsn) { - const env = createEventEnvelope(event, this._dsn, attachments, this._options._metadata, this._options.tunnel); + const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); + + for (const attachment of attachments || []) { + addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment)); + } + this._sendEnvelope(env); } } @@ -543,7 +549,7 @@ export abstract class BaseClient implements Client { * @param event The Sentry event to send */ // TODO(v7): refactor: get rid of method? - protected _sendEvent(event: Event, attachments?: AttachmentItem[]): void { + protected _sendEvent(event: Event, attachments?: Attachment[]): void { this.sendEvent(event, attachments); } @@ -662,10 +668,10 @@ export abstract class BaseClient implements Client { } /** - * Loads attachment items from scope + * Loads attachments from scope */ - protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] { - return scope?.getAttachments()?.map(a => createAttachmentEnvelopeItem(a)) || []; + protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { + return scope?.getAttachments() || []; } /** diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 4378809b2ad9..2f69884e8bc3 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,5 +1,4 @@ import { - AttachmentItem, DsnComponents, Event, EventEnvelope, @@ -64,7 +63,6 @@ export function createSessionEnvelope( export function createEventEnvelope( event: Event, dsn: DsnComponents, - attachments: AttachmentItem[] = [], metadata?: SdkMetadata, tunnel?: string, ): EventEnvelope { @@ -116,5 +114,5 @@ export function createEventEnvelope( }, event, ]; - return createEnvelope(envelopeHeaders, [eventItem, ...attachments]); + return createEnvelope(envelopeHeaders, [eventItem]); } diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 4e6c04e8c516..410705fdf497 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -69,7 +69,7 @@ export function createTransport( }; const requestTask = (): PromiseLike => - makeRequest({ body: serializeEnvelope(filteredEnvelope) }).then( + makeRequest({ body: (options.serializeEnvelope || serializeEnvelope)(filteredEnvelope) }).then( ({ headers }): void => { if (headers) { rateLimits = updateRateLimits(rateLimits, headers); diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 36f56a9d6d8b..28187671a054 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,7 +1,6 @@ /* eslint-disable max-lines */ import { Attachment, - AttachmentOptions, Breadcrumb, CaptureContext, Context, @@ -408,8 +407,8 @@ export class Scope implements ScopeInterface { /** * @inheritDoc */ - public addAttachment(pathOrData: string | Uint8Array, options?: AttachmentOptions): this { - this._attachments.push([pathOrData, options]); + public addAttachment(attachment: Attachment): this { + this._attachments.push(attachment); return this; } diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 5ddafe8efdfd..a7b837c1e9ac 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,7 +1,7 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; -import { AttachmentItem, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; -import { basename, createAttachmentEnvelopeItem, logger, resolvedSyncPromise } from '@sentry/utils'; +import { Attachment, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; +import { basename, logger, resolvedSyncPromise } from '@sentry/utils'; import { existsSync, readFileSync } from 'fs'; import { eventFromMessage, eventFromUnknownInput } from './eventbuilder'; @@ -155,18 +155,15 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _attachmentsFromScope(scope: Scope | undefined): AttachmentItem[] { + protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { return ( scope?.getAttachments()?.map(attachment => { - let [pathOrData, options] = attachment; - - if (typeof pathOrData === 'string' && existsSync(pathOrData)) { - options = options || {}; - options.filename = basename(pathOrData); - pathOrData = readFileSync(pathOrData); + if (attachment.path && existsSync(attachment.path)) { + attachment.filename = basename(attachment.path); + attachment.data = readFileSync(attachment.path); } - return createAttachmentEnvelopeItem([pathOrData, options]); + return attachment; }) || [] ); } diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index dbd51448e0e0..9e141cc0e10f 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -6,6 +6,7 @@ import { TransportRequest, TransportRequestExecutor, } from '@sentry/types'; +import { serializeEnvelope } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; import { Readable, Writable } from 'stream'; @@ -45,6 +46,8 @@ function streamFromBody(body: Uint8Array | string): Readable { * Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry. */ export function makeNodeTransport(options: NodeTransportOptions): Transport { + options.serializeEnvelope = s => serializeEnvelope(s, () => new TextEncoder()); + const urlSegments = new URL(options.url); const isHttps = urlSegments.protocol === 'https:'; diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 1caed705577e..edef1b70fc19 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,6 +1,6 @@ import { initAndBind, SDK_VERSION } from '@sentry/core'; import { getMainCarrier } from '@sentry/hub'; -import { AttachmentItem, Integration } from '@sentry/types'; +import { Attachment, Integration } from '@sentry/types'; import { createStackParser } from '@sentry/utils'; import * as domain from 'domain'; @@ -79,7 +79,7 @@ describe('SentryNode', () => { }); describe('breadcrumbs', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); @@ -110,7 +110,7 @@ describe('SentryNode', () => { }); describe('capture', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index 013468c3eb5d..b9ed28585b1c 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -2,6 +2,7 @@ import { createTransport } from '@sentry/core'; import { EventEnvelope, EventItem } from '@sentry/types'; import { createEnvelope, serializeEnvelope } from '@sentry/utils'; import * as http from 'http'; +import { TextEncoder } from 'util'; import { makeNodeTransport } from '../../src/transports'; @@ -66,7 +67,7 @@ const EVENT_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4b [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, ]); -const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE); +const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, () => new TextEncoder()); const defaultOptions = { url: TEST_SERVER_URL, diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts index 118a9a70dc88..8edec23145b2 100644 --- a/packages/node/test/transports/https.test.ts +++ b/packages/node/test/transports/https.test.ts @@ -3,6 +3,7 @@ import { EventEnvelope, EventItem } from '@sentry/types'; import { createEnvelope, serializeEnvelope } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; +import { TextEncoder } from 'util'; import { makeNodeTransport } from '../../src/transports'; import { HTTPModule, HTTPModuleRequestIncomingMessage } from '../../src/transports/http-module'; @@ -69,7 +70,7 @@ const EVENT_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4b [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, ]); -const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE); +const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, () => new TextEncoder()); const unsafeHttpsModule: HTTPModule = { request: jest diff --git a/packages/types/src/attachment.ts b/packages/types/src/attachment.ts index f4e0d3dec666..3f6cb9089182 100644 --- a/packages/types/src/attachment.ts +++ b/packages/types/src/attachment.ts @@ -1,7 +1,7 @@ -export interface AttachmentOptions { +export interface Attachment { + path?: string; + data?: string | Uint8Array; filename?: string; contentType?: string; attachmentType?: string; } - -export type Attachment = [string | Uint8Array, AttachmentOptions | undefined]; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 3ee9892f6496..8e4a3e533253 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,4 +1,4 @@ -export type { Attachment, AttachmentOptions } from './attachment'; +export type { Attachment } from './attachment'; export type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport, Outcome, EventDropReason } from './clientreport'; diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index f3b990dd591c..a2a14ffd4664 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -1,4 +1,4 @@ -import { Attachment, AttachmentOptions } from './attachment'; +import { Attachment } from './attachment'; import { Breadcrumb } from './breadcrumb'; import { Context, Contexts } from './context'; import { EventProcessor } from './eventprocessor'; @@ -162,10 +162,9 @@ export interface Scope { /** * Adds an attachment to the scope - * @param pathOrData A Uint8Array containing the attachment bytes - * @param options Attachment options + * @param attachment Attachment options */ - addAttachment(pathOrData: string | Uint8Array, options?: AttachmentOptions): this; + addAttachment(attachment: Attachment): this; /** * Returns an array of attachments on the scope diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts index dd0133083253..d2e6d599d28d 100644 --- a/packages/types/src/transport.ts +++ b/packages/types/src/transport.ts @@ -17,6 +17,7 @@ export type TransportMakeRequestResponse = { export interface InternalBaseTransportOptions { bufferSize?: number; recordDroppedEvent: (reason: EventDropReason, dataCategory: DataCategory) => void; + serializeEnvelope?: (env: Envelope) => string | Uint8Array; } export interface BaseTransportOptions extends InternalBaseTransportOptions { diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 9b07a06b5ea1..396302bdbdbf 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -1,7 +1,5 @@ import { Attachment, AttachmentItem, DataCategory, Envelope, EnvelopeItem, EnvelopeItemType } from '@sentry/types'; -import { isPrimitive } from './is'; - /** * Creates an envelope. * Make sure to always explicitly provide the generic to this function @@ -36,61 +34,57 @@ export function forEachEnvelopeItem( }); } -/** - * Serializes an envelope. - */ -export function serializeEnvelope(envelope: Envelope): string | Uint8Array { - const [, items] = envelope; - - // Have to cast items to any here since Envelope is a union type - // Fixed in Typescript 4.2 - // TODO: Remove any[] cast when we upgrade to TS 4.2 - // https://github.com/microsoft/TypeScript/issues/36390 - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const hasBinaryAttachment = (items as any[]).some( - (item: typeof items[number]) => item[0].type === 'attachment' && item[1] instanceof Uint8Array, - ); - - return hasBinaryAttachment ? serializeBinaryEnvelope(envelope) : serializeStringEnvelope(envelope); -} +// Cached UTF8 string encoder +let encoder: TextEncoder | undefined; -/** - * Serializes an envelope to a string. - */ -export function serializeStringEnvelope(envelope: Envelope): string { - const [headers, items] = envelope; - const serializedHeaders = JSON.stringify(headers); - - // TODO: Remove any[] cast when we upgrade to TS 4.2 - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (items as any[]).reduce((acc, item: typeof items[number]) => { - const [itemHeaders, payload] = item; - // We do not serialize payloads that are primitives - const serializedPayload = isPrimitive(payload) ? String(payload) : JSON.stringify(payload); - return `${acc}\n${JSON.stringify(itemHeaders)}\n${serializedPayload}`; - }, serializedHeaders); +function getCachedEncoder(): TextEncoder { + if (!encoder) { + encoder = new TextEncoder(); + } + + return encoder; } -function serializeBinaryEnvelope(envelope: Envelope): Uint8Array { - const encoder = new TextEncoder(); - const [headers, items] = envelope; - const serializedHeaders = JSON.stringify(headers); +// Combination of global TextEncoder and Node require('util').TextEncoder +interface TextEncoderInternal extends TextEncoderCommon { + encode(input?: string): Uint8Array; +} - const chunks = [encoder.encode(serializedHeaders)]; +/** + * Serializes an envelope. + */ +export function serializeEnvelope( + envelope: Envelope, + textEncoderOverride?: () => TextEncoderInternal, +): string | Uint8Array { + const textEncoder = textEncoderOverride || getCachedEncoder; + + const [envHeaders, items] = envelope; + + // Initially we construct our envelope as a string and only convert to binary if we encounter binary data + let parts: string | Uint8Array[] = JSON.stringify(envHeaders); + + function append(next: string | Uint8Array): void { + if (typeof parts === 'string') { + if (typeof next === 'string') { + parts += next; + } else { + parts = [textEncoder().encode(parts), next]; + } + } else { + parts.push(typeof next === 'string' ? textEncoder().encode(next) : next); + } + } for (const item of items) { const [itemHeaders, payload] = item as typeof items[number]; - chunks.push(encoder.encode(`\n${JSON.stringify(itemHeaders)}\n`)); - if (typeof payload === 'string') { - chunks.push(encoder.encode(payload)); - } else if (payload instanceof Uint8Array) { - chunks.push(payload); - } else { - chunks.push(encoder.encode(JSON.stringify(payload))); - } + append(`\n${JSON.stringify(itemHeaders)}\n`); + append(typeof payload === 'string' || payload instanceof Uint8Array ? payload : JSON.stringify(payload)); } - return concatBuffers(chunks); + append('\n'); + + return typeof parts === 'string' ? parts : concatBuffers(parts); } function concatBuffers(buffers: Uint8Array[]): Uint8Array { @@ -110,19 +104,17 @@ function concatBuffers(buffers: Uint8Array[]): Uint8Array { * Creates attachment envelope items */ export function createAttachmentEnvelopeItem(attachment: Attachment): AttachmentItem { - const [pathOrData, options] = attachment; - - const buffer = typeof pathOrData === 'string' ? new TextEncoder().encode(pathOrData) : pathOrData; + const buffer = typeof attachment.data === 'string' ? new TextEncoder().encode(attachment.data) : attachment.data; return [ { type: 'attachment', - length: buffer.length, - filename: options?.filename || 'No filename', - content_type: options?.contentType, - attachment_type: options?.attachmentType, + length: buffer?.length || 0, + filename: attachment?.filename || 'No filename', + content_type: attachment?.contentType, + attachment_type: attachment?.attachmentType, }, - buffer, + buffer || new Uint8Array(0), ]; } diff --git a/packages/utils/test/clientreport.test.ts b/packages/utils/test/clientreport.test.ts index f2abca98e798..f52c51d6f578 100644 --- a/packages/utils/test/clientreport.test.ts +++ b/packages/utils/test/clientreport.test.ts @@ -2,6 +2,7 @@ import { ClientReport } from '@sentry/types'; import { createClientReportEnvelope } from '../src/clientreport'; import { serializeEnvelope } from '../src/envelope'; +import { parseEnvelope } from './testutils'; const DEFAULT_DISCARDED_EVENTS: ClientReport['discarded_events'] = [ { @@ -41,11 +42,21 @@ describe('createClientReportEnvelope', () => { it('serializes an envelope', () => { const env = createClientReportEnvelope(DEFAULT_DISCARDED_EVENTS, MOCK_DSN, 123456); - const serializedEnv = serializeEnvelope(env); - expect(serializedEnv).toMatchInlineSnapshot(` - "{\\"dsn\\":\\"https://public@example.com/1\\"} - {\\"type\\":\\"client_report\\"} - {\\"timestamp\\":123456,\\"discarded_events\\":[{\\"reason\\":\\"before_send\\",\\"category\\":\\"error\\",\\"quantity\\":30},{\\"reason\\":\\"network_error\\",\\"category\\":\\"transaction\\",\\"quantity\\":23}]}" - `); + + const [headers, items] = parseEnvelope(serializeEnvelope(env)); + + expect(headers).toEqual({ dsn: 'https://public@example.com/1' }); + expect(items).toEqual([ + [ + { type: 'client_report' }, + { + timestamp: 123456, + discarded_events: [ + { reason: 'before_send', category: 'error', quantity: 30 }, + { reason: 'network_error', category: 'transaction', quantity: 23 }, + ], + }, + ], + ]); }); }); diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index 32cb88aefd71..b105e4111aa5 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -1,12 +1,6 @@ import { EventEnvelope } from '@sentry/types'; -import { - addItemToEnvelope, - createEnvelope, - forEachEnvelopeItem, - serializeStringEnvelope, - serializeEnvelope, -} from '../src/envelope'; +import { addItemToEnvelope, createEnvelope, forEachEnvelopeItem, serializeEnvelope } from '../src/envelope'; import { parseEnvelope } from './testutils'; describe('envelope', () => { @@ -23,12 +17,11 @@ describe('envelope', () => { }); }); - describe('serializeStringEnvelope()', () => { + describe('serializeEnvelope()', () => { it('serializes an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - expect(serializeStringEnvelope(env)).toMatchInlineSnapshot( - '"{\\"event_id\\":\\"aa3ff046696b4bc6b609ce6d28fde9e2\\",\\"sent_at\\":\\"123\\"}"', - ); + const [headers] = parseEnvelope(serializeEnvelope(env)); + expect(headers).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); }); }); diff --git a/packages/utils/test/testutils.ts b/packages/utils/test/testutils.ts index c87aa0384b53..b94fbd854892 100644 --- a/packages/utils/test/testutils.ts +++ b/packages/utils/test/testutils.ts @@ -1,4 +1,4 @@ -import { Envelope, BaseEnvelopeHeaders, BaseEnvelopeItemHeaders } from '@sentry/types'; +import { BaseEnvelopeHeaders, BaseEnvelopeItemHeaders, Envelope } from '@sentry/types'; export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { const currentNodeVersion = process.env.NODE_VERSION; @@ -18,32 +18,30 @@ export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { * A naive binary envelope parser */ export function parseEnvelope(env: string | Uint8Array): Envelope { - if (typeof env === 'string') { - env = new TextEncoder().encode(env); - } + let buf = typeof env === 'string' ? new TextEncoder().encode(env) : env; let envelopeHeaders: BaseEnvelopeHeaders | undefined; let lastItemHeader: BaseEnvelopeItemHeaders | undefined; const items: [any, any][] = []; let binaryLength = 0; - while (env.length) { + while (buf.length) { // Next length is either the binary length from the previous header // or the next newline character - let i = binaryLength || env.indexOf(0xa); + let i = binaryLength || buf.indexOf(0xa); // If no newline was found, assume this is the last block if (i < 0) { - i = env.length; + i = buf.length; } // If we read out a length in the previous header, assume binary if (binaryLength > 0) { - const bin = env.slice(0, binaryLength); + const bin = buf.slice(0, binaryLength); binaryLength = 0; items.push([lastItemHeader, bin]); } else { - const json = JSON.parse(new TextDecoder().decode(env.slice(0, i + 1))); + const json = JSON.parse(new TextDecoder().decode(buf.slice(0, i + 1))); if (typeof json.length === 'number') { binaryLength = json.length; @@ -63,7 +61,7 @@ export function parseEnvelope(env: string | Uint8Array): Envelope { } // Replace the buffer with the previous block and newline removed - env = env.slice(i + 1); + buf = buf.slice(i + 1); } return [envelopeHeaders as BaseEnvelopeHeaders, items]; From fc3303c0ee0defdd360b4218115b0034a8299214 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 1 May 2022 13:44:46 +0100 Subject: [PATCH 09/26] More simplify --- packages/core/src/baseclient.ts | 2 +- packages/core/src/transports/base.ts | 2 +- packages/node/src/client.ts | 6 +++++ packages/node/src/transports/http.ts | 3 --- packages/node/test/transports/http.test.ts | 2 +- packages/node/test/transports/https.test.ts | 2 +- packages/types/src/envelope.ts | 7 ++--- packages/types/src/transport.ts | 7 ++++- packages/utils/src/envelope.ts | 30 +++++++-------------- 9 files changed, 28 insertions(+), 33 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index d9393c3e938e..abdc41b52f2e 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -281,7 +281,7 @@ export abstract class BaseClient implements Client { const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); for (const attachment of attachments || []) { - addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment)); + addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment, this._options.transportOptions?.textEncoder)); } this._sendEnvelope(env); diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 410705fdf497..27ea5b194b9e 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -69,7 +69,7 @@ export function createTransport( }; const requestTask = (): PromiseLike => - makeRequest({ body: (options.serializeEnvelope || serializeEnvelope)(filteredEnvelope) }).then( + makeRequest({ body: serializeEnvelope(filteredEnvelope, options.textEncoder) }).then( ({ headers }): void => { if (headers) { rateLimits = updateRateLimits(rateLimits, headers); diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index a7b837c1e9ac..3453e5001b67 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -3,6 +3,7 @@ import { SessionFlusher } from '@sentry/hub'; import { Attachment, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; import { basename, logger, resolvedSyncPromise } from '@sentry/utils'; import { existsSync, readFileSync } from 'fs'; +import { TextEncoder } from 'util'; import { eventFromMessage, eventFromUnknownInput } from './eventbuilder'; import { IS_DEBUG_BUILD } from './flags'; @@ -34,6 +35,11 @@ export class NodeClient extends BaseClient { version: SDK_VERSION, }; + options.transportOptions = { + textEncoder: new TextEncoder(), + ...options.transportOptions, + }; + super(options); } diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 9e141cc0e10f..dbd51448e0e0 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -6,7 +6,6 @@ import { TransportRequest, TransportRequestExecutor, } from '@sentry/types'; -import { serializeEnvelope } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; import { Readable, Writable } from 'stream'; @@ -46,8 +45,6 @@ function streamFromBody(body: Uint8Array | string): Readable { * Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry. */ export function makeNodeTransport(options: NodeTransportOptions): Transport { - options.serializeEnvelope = s => serializeEnvelope(s, () => new TextEncoder()); - const urlSegments = new URL(options.url); const isHttps = urlSegments.protocol === 'https:'; diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index b9ed28585b1c..7e69feaa884c 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -67,7 +67,7 @@ const EVENT_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4b [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, ]); -const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, () => new TextEncoder()); +const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()); const defaultOptions = { url: TEST_SERVER_URL, diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts index 8edec23145b2..41f75a214208 100644 --- a/packages/node/test/transports/https.test.ts +++ b/packages/node/test/transports/https.test.ts @@ -70,7 +70,7 @@ const EVENT_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4b [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, ]); -const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, () => new TextEncoder()); +const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()); const unsafeHttpsModule: HTTPModule = { request: jest diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 98f2d316000b..2f47689fe27a 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -51,11 +51,8 @@ type SessionItemHeaders = { type: 'session' }; type SessionAggregatesItemHeaders = { type: 'sessions' }; type ClientReportItemHeaders = { type: 'client_report' }; -// TODO(v7): Remove the string union from `Event | string` -// We have to allow this hack for now as we pre-serialize events because we support -// both store and envelope endpoints. -export type EventItem = BaseEnvelopeItem; -export type AttachmentItem = BaseEnvelopeItem; +export type EventItem = BaseEnvelopeItem; +export type AttachmentItem = BaseEnvelopeItem; export type UserFeedbackItem = BaseEnvelopeItem; export type SessionItem = | BaseEnvelopeItem diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts index d2e6d599d28d..78ba74cdf5ad 100644 --- a/packages/types/src/transport.ts +++ b/packages/types/src/transport.ts @@ -14,10 +14,15 @@ export type TransportMakeRequestResponse = { }; }; +// Combination of global TextEncoder and Node require('util').TextEncoder +interface TextEncoderInternal extends TextEncoderCommon { + encode(input?: string): Uint8Array; +} + export interface InternalBaseTransportOptions { bufferSize?: number; recordDroppedEvent: (reason: EventDropReason, dataCategory: DataCategory) => void; - serializeEnvelope?: (env: Envelope) => string | Uint8Array; + textEncoder?: TextEncoderInternal; } export interface BaseTransportOptions extends InternalBaseTransportOptions { diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 396302bdbdbf..be2d01636689 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -34,17 +34,6 @@ export function forEachEnvelopeItem( }); } -// Cached UTF8 string encoder -let encoder: TextEncoder | undefined; - -function getCachedEncoder(): TextEncoder { - if (!encoder) { - encoder = new TextEncoder(); - } - - return encoder; -} - // Combination of global TextEncoder and Node require('util').TextEncoder interface TextEncoderInternal extends TextEncoderCommon { encode(input?: string): Uint8Array; @@ -53,11 +42,8 @@ interface TextEncoderInternal extends TextEncoderCommon { /** * Serializes an envelope. */ -export function serializeEnvelope( - envelope: Envelope, - textEncoderOverride?: () => TextEncoderInternal, -): string | Uint8Array { - const textEncoder = textEncoderOverride || getCachedEncoder; +export function serializeEnvelope(envelope: Envelope, textEncoder?: TextEncoderInternal): string | Uint8Array { + const utf8 = textEncoder || new TextEncoder(); const [envHeaders, items] = envelope; @@ -69,10 +55,10 @@ export function serializeEnvelope( if (typeof next === 'string') { parts += next; } else { - parts = [textEncoder().encode(parts), next]; + parts = [utf8.encode(parts), next]; } } else { - parts.push(typeof next === 'string' ? textEncoder().encode(next) : next); + parts.push(typeof next === 'string' ? utf8.encode(next) : next); } } @@ -103,8 +89,12 @@ function concatBuffers(buffers: Uint8Array[]): Uint8Array { /** * Creates attachment envelope items */ -export function createAttachmentEnvelopeItem(attachment: Attachment): AttachmentItem { - const buffer = typeof attachment.data === 'string' ? new TextEncoder().encode(attachment.data) : attachment.data; +export function createAttachmentEnvelopeItem( + attachment: Attachment, + textEncoder?: TextEncoderInternal, +): AttachmentItem { + const utf8 = textEncoder || new TextEncoder(); + const buffer = typeof attachment.data === 'string' ? utf8.encode(attachment.data) : attachment.data; return [ { From 68e077a1fd8bf158d3b2f9d3f4a3eef2fa94d82d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 1 May 2022 14:04:38 +0100 Subject: [PATCH 10/26] Final newline is optional --- packages/node/src/transports/http.ts | 2 +- packages/utils/src/envelope.ts | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index dbd51448e0e0..94450485ff7b 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -30,7 +30,7 @@ const GZIP_THRESHOLD = 1024 * 32; /** * Gets a stream from a Uint8Array or string - * Readable.from was added in node.sj v12.3.0, v10.17.0 + * Readable.from was added in node.js v12.3.0 and v10.17.0 */ function streamFromBody(body: Uint8Array | string): Readable { return new Readable({ diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index be2d01636689..290a0b21f913 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -68,8 +68,6 @@ export function serializeEnvelope(envelope: Envelope, textEncoder?: TextEncoderI append(typeof payload === 'string' || payload instanceof Uint8Array ? payload : JSON.stringify(payload)); } - append('\n'); - return typeof parts === 'string' ? parts : concatBuffers(parts); } From 735812ea2ad4214b77922a0e3636eef061085138 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 1 May 2022 15:25:24 +0100 Subject: [PATCH 11/26] Fix gatsby test --- packages/gatsby/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/jest.config.js b/packages/gatsby/jest.config.js index cc7d8162cd59..fffe12d6a4d8 100644 --- a/packages/gatsby/jest.config.js +++ b/packages/gatsby/jest.config.js @@ -3,5 +3,5 @@ const baseConfig = require('../../jest/jest.config.js'); module.exports = { ...baseConfig, setupFiles: ['/test/setEnvVars.ts'], - testEnvironment: 'jsdom', + testEnvironment: '../browser/jest.env.js', }; From fe7f49f90de5cd73d47a80d29a97758b3ffbfc05 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 1 May 2022 15:46:13 +0100 Subject: [PATCH 12/26] Fix node v8/v10 --- packages/browser/test/unit/transports/fetch.test.ts | 5 +++-- packages/browser/test/unit/transports/xhr.test.ts | 3 ++- packages/core/test/lib/transports/base.test.ts | 11 ++++++++--- packages/core/test/mocks/client.ts | 7 ++++++- packages/core/test/mocks/transport.ts | 3 ++- .../test/manual/express-scope-separation/start.js | 3 ++- packages/node/test/manual/webpack-domain/index.js | 4 ++-- packages/node/test/transports/http.test.ts | 9 +++++---- packages/node/test/transports/https.test.ts | 9 +++++---- packages/utils/test/clientreport.test.ts | 3 ++- packages/utils/test/envelope.test.ts | 9 +++++---- packages/utils/test/testutils.ts | 1 + 12 files changed, 43 insertions(+), 24 deletions(-) diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 63fe62814628..9f885f0843f9 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -1,5 +1,6 @@ import { EventEnvelope, EventItem } from '@sentry/types'; import { createEnvelope, serializeEnvelope } from '@sentry/utils'; +import { TextEncoder } from 'util'; import { FetchTransportOptions, makeFetchTransport } from '../../../src/transports/fetch'; import { FetchImpl } from '../../../src/transports/utils'; @@ -39,7 +40,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', }); @@ -89,7 +90,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, }); diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts index 35ca842db3d1..bb2fab932344 100644 --- a/packages/browser/test/unit/transports/xhr.test.ts +++ b/packages/browser/test/unit/transports/xhr.test.ts @@ -1,5 +1,6 @@ import { EventEnvelope, EventItem } from '@sentry/types'; import { createEnvelope, serializeEnvelope } from '@sentry/utils'; +import { TextEncoder } from 'util'; import { makeXHRTransport, XHRTransportOptions } from '../../../src/transports/xhr'; @@ -63,7 +64,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 () => { diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index b57cc36f6e6f..94d83751a21d 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -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'; @@ -14,6 +15,7 @@ const TRANSACTION_ENVELOPE = createEnvelope( const transportOptions = { recordDroppedEvent: () => undefined, // noop + textEncoder: new TextEncoder(), }; describe('createTransport', () => { @@ -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); @@ -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(); }); @@ -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; } diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index d46eb000d480..c0aadaa5cbf8 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -1,6 +1,7 @@ import { Session } from '@sentry/hub'; import { ClientOptions, Event, Integration, Outcome, Severity, SeverityLevel } from '@sentry/types'; import { resolvedSyncPromise } from '@sentry/utils'; +import { TextEncoder } from 'util'; import { BaseClient } from '../../src/baseclient'; import { initAndBind } from '../../src/sdk'; @@ -10,9 +11,13 @@ export function getDefaultTestClientOptions(options: Partial return { integrations: [], sendClientReports: true, + transportOptions: { textEncoder: new TextEncoder() }, transport: () => createTransport( - { recordDroppedEvent: () => undefined }, // noop + { + recordDroppedEvent: () => undefined, + textEncoder: new TextEncoder(), + }, // noop _ => resolvedSyncPromise({}), ), stackParser: () => [], diff --git a/packages/core/test/mocks/transport.ts b/packages/core/test/mocks/transport.ts index f59e72a516a1..90d35e2a0247 100644 --- a/packages/core/test/mocks/transport.ts +++ b/packages/core/test/mocks/transport.ts @@ -1,4 +1,5 @@ import { SyncPromise } from '@sentry/utils'; +import { TextEncoder } from 'util'; import { createTransport } from '../../src/transports/base'; @@ -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); diff --git a/packages/node/test/manual/express-scope-separation/start.js b/packages/node/test/manual/express-scope-separation/start.js index faf7b381ecbc..4e903a33bde4 100644 --- a/packages/node/test/manual/express-scope-separation/start.js +++ b/packages/node/test/manual/express-scope-separation/start.js @@ -2,6 +2,7 @@ const http = require('http'); const express = require('express'); const app = express(); const Sentry = require('../../../build/cjs'); +const { TextEncoder } = require('util'); // don't log the test errors we're going to throw, so at a quick glance it doesn't look like the test itself has failed global.console.error = () => null; @@ -16,7 +17,7 @@ function assertTags(actual, expected) { let remaining = 3; function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { --remaining; if (!remaining) { diff --git a/packages/node/test/manual/webpack-domain/index.js b/packages/node/test/manual/webpack-domain/index.js index 09ed18f3166f..edd13ef6ee6e 100644 --- a/packages/node/test/manual/webpack-domain/index.js +++ b/packages/node/test/manual/webpack-domain/index.js @@ -1,9 +1,9 @@ const Sentry = require('../../../build/cjs'); - +const { TextEncoder } = require('util'); let remaining = 2; function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { --remaining; if (!remaining) { diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index 7e69feaa884c..7ff6d450fdae 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -72,6 +72,7 @@ const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, new TextEnco const defaultOptions = { url: TEST_SERVER_URL, recordDroppedEvent: () => undefined, + textEncoder: new TextEncoder(), }; describe('makeNewHttpTransport()', () => { @@ -248,7 +249,7 @@ describe('makeNewHttpTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); @@ -271,7 +272,7 @@ describe('makeNewHttpTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); @@ -298,7 +299,7 @@ describe('makeNewHttpTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); @@ -325,7 +326,7 @@ describe('makeNewHttpTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts index 41f75a214208..1d258656008a 100644 --- a/packages/node/test/transports/https.test.ts +++ b/packages/node/test/transports/https.test.ts @@ -84,6 +84,7 @@ const defaultOptions = { httpModule: unsafeHttpsModule, url: TEST_SERVER_URL, recordDroppedEvent: () => undefined, // noop + textEncoder: new TextEncoder(), }; describe('makeNewHttpsTransport()', () => { @@ -301,7 +302,7 @@ describe('makeNewHttpsTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); @@ -324,7 +325,7 @@ describe('makeNewHttpsTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); @@ -351,7 +352,7 @@ describe('makeNewHttpsTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); @@ -378,7 +379,7 @@ describe('makeNewHttpsTransport()', () => { const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ - body: serializeEnvelope(EVENT_ENVELOPE), + body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()), category: 'error', }); diff --git a/packages/utils/test/clientreport.test.ts b/packages/utils/test/clientreport.test.ts index f52c51d6f578..442df49c6edf 100644 --- a/packages/utils/test/clientreport.test.ts +++ b/packages/utils/test/clientreport.test.ts @@ -1,4 +1,5 @@ import { ClientReport } from '@sentry/types'; +import { TextEncoder } from 'util'; import { createClientReportEnvelope } from '../src/clientreport'; import { serializeEnvelope } from '../src/envelope'; @@ -43,7 +44,7 @@ describe('createClientReportEnvelope', () => { it('serializes an envelope', () => { const env = createClientReportEnvelope(DEFAULT_DISCARDED_EVENTS, MOCK_DSN, 123456); - const [headers, items] = parseEnvelope(serializeEnvelope(env)); + const [headers, items] = parseEnvelope(serializeEnvelope(env, new TextEncoder())); expect(headers).toEqual({ dsn: 'https://public@example.com/1' }); expect(items).toEqual([ diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index b105e4111aa5..8f7fce2825a7 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -1,4 +1,5 @@ import { EventEnvelope } from '@sentry/types'; +import { TextEncoder } from 'util'; import { addItemToEnvelope, createEnvelope, forEachEnvelopeItem, serializeEnvelope } from '../src/envelope'; import { parseEnvelope } from './testutils'; @@ -20,7 +21,7 @@ describe('envelope', () => { describe('serializeEnvelope()', () => { it('serializes an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - const [headers] = parseEnvelope(serializeEnvelope(env)); + const [headers] = parseEnvelope(serializeEnvelope(env, new TextEncoder())); expect(headers).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); }); }); @@ -28,7 +29,7 @@ describe('envelope', () => { describe('addItemToEnvelope()', () => { it('adds an item to an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - let [envHeaders, items] = parseEnvelope(serializeEnvelope(env)); + let [envHeaders, items] = parseEnvelope(serializeEnvelope(env, new TextEncoder())); expect(items).toHaveLength(0); expect(envHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); @@ -37,7 +38,7 @@ describe('envelope', () => { { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }, ]); - [envHeaders, items] = parseEnvelope(serializeEnvelope(newEnv)); + [envHeaders, items] = parseEnvelope(serializeEnvelope(newEnv, new TextEncoder())); expect(envHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); expect(items).toHaveLength(1); expect(items[0]).toEqual([{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]); @@ -66,7 +67,7 @@ describe('envelope', () => { iteration = iteration + 1; }); - const [parsedHeaders, parsedItems] = parseEnvelope(serializeEnvelope(env)); + const [parsedHeaders, parsedItems] = parseEnvelope(serializeEnvelope(env, new TextEncoder())); expect(parsedHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); expect(parsedItems).toHaveLength(3); expect(items[0]).toEqual([{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]); diff --git a/packages/utils/test/testutils.ts b/packages/utils/test/testutils.ts index b94fbd854892..b708d77064eb 100644 --- a/packages/utils/test/testutils.ts +++ b/packages/utils/test/testutils.ts @@ -1,4 +1,5 @@ import { BaseEnvelopeHeaders, BaseEnvelopeItemHeaders, Envelope } from '@sentry/types'; +import { TextDecoder, TextEncoder } from 'util'; export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { const currentNodeVersion = process.env.NODE_VERSION; From 22a8f079308268b8a6d3d5b080bd3804df8a3413 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 2 May 2022 00:29:02 +0100 Subject: [PATCH 13/26] Fix node session tests --- .../aggregates-disable-single-session.js | 8 ++++++-- .../single-session/caught-exception-errored-session.js | 8 ++++++-- .../single-session/errors-in-session-capped-to-one.js | 8 ++++++-- .../release-health/single-session/healthy-session.js | 9 +++++++-- .../single-session/terminal-state-sessions-sent-once.js | 8 ++++++-- .../single-session/uncaught-exception-crashed-session.js | 8 ++++++-- .../unhandled-rejection-crashed-session.js | 8 ++++++-- 7 files changed, 43 insertions(+), 14 deletions(-) diff --git a/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js b/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js index ae6a660607b1..300870d21fac 100644 --- a/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js +++ b/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js @@ -3,6 +3,7 @@ const express = require('express'); const app = express(); const Sentry = require('../../../../build/cjs'); const { assertSessions } = require('../test-utils'); +const { TextEncoder } = require('util'); function cleanUpAndExitSuccessfully() { server.close(); @@ -28,8 +29,11 @@ function assertSessionAggregates(session, expected) { } function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { - const sessionEnv = req.body.split('\n').map(e => JSON.parse(e)); + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { + const sessionEnv = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); assertSessionAggregates(sessionEnv[2], { attrs: { release: '1.1' }, aggregates: [{ crashed: 2, errored: 1, exited: 1 }], diff --git a/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js b/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js index b7a9538a3fa2..0cecc8ee75e4 100644 --- a/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js +++ b/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js @@ -1,5 +1,6 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject, validateSessionCountFunction } = require('../test-utils'); +const { TextEncoder } = require('util'); const sessionCounts = { sessionCounter: 0, @@ -9,8 +10,11 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { - const payload = req.body.split('\n').map(e => JSON.parse(e)); + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { + const payload = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; if (isSessionPayload) { diff --git a/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js index dae307182ed1..18ec6fefdc78 100644 --- a/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js +++ b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js @@ -1,5 +1,6 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject, validateSessionCountFunction } = require('../test-utils'); +const { TextEncoder } = require('util'); const sessionCounts = { sessionCounter: 0, @@ -9,8 +10,11 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { - const payload = req.body.split('\n').map(e => JSON.parse(e)); + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { + const payload = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; if (isSessionPayload) { diff --git a/packages/node/test/manual/release-health/single-session/healthy-session.js b/packages/node/test/manual/release-health/single-session/healthy-session.js index 0533b8a28728..11c3092dfcad 100644 --- a/packages/node/test/manual/release-health/single-session/healthy-session.js +++ b/packages/node/test/manual/release-health/single-session/healthy-session.js @@ -1,5 +1,6 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject, validateSessionCountFunction } = require('../test-utils'); +const { TextEncoder } = require('util'); const sessionCounts = { sessionCounter: 0, @@ -9,9 +10,13 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { sessionCounts.sessionCounter++; - const sessionEnv = req.body.split('\n').map(e => JSON.parse(e)); + + const sessionEnv = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); assertSessions(constructStrippedSessionObject(sessionEnv[2]), { init: true, diff --git a/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js b/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js index aa0796782c2f..571af19d0f94 100644 --- a/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js +++ b/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js @@ -1,5 +1,6 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject, validateSessionCountFunction } = require('../test-utils'); +const { TextEncoder } = require('util'); const sessionCounts = { sessionCounter: 0, @@ -9,8 +10,11 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { - const payload = req.body.split('\n').map(e => JSON.parse(e)); + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { + const payload = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; if (isSessionPayload) { diff --git a/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js b/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js index 6fa3ac0a6821..1759c1cc2d0f 100644 --- a/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js +++ b/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js @@ -1,11 +1,15 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject } = require('../test-utils'); +const { TextEncoder } = require('util'); function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { if (req.category === 'session') { sessionCounts.sessionCounter++; - const sessionEnv = req.body.split('\n').map(e => JSON.parse(e)); + const sessionEnv = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); assertSessions(constructStrippedSessionObject(sessionEnv[2]), { init: true, diff --git a/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js b/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js index b550260b62b7..cc8cf921c5d0 100644 --- a/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js +++ b/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js @@ -1,5 +1,6 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject, validateSessionCountFunction } = require('../test-utils'); +const { TextEncoder } = require('util'); const sessionCounts = { sessionCounter: 0, @@ -9,8 +10,11 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { - const payload = req.body.split('\n').map(e => JSON.parse(e)); + return Sentry.createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, req => { + const payload = req.body + .split('\n') + .filter(l => !!l) + .map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; if (isSessionPayload) { From 242292f773b4bc411e765c5900bc27f27960858f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 3 May 2022 12:20:36 +0100 Subject: [PATCH 14/26] Don't override filename if it's supplied --- packages/node/src/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 3453e5001b67..a634c68cde8d 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -165,7 +165,7 @@ export class NodeClient extends BaseClient { return ( scope?.getAttachments()?.map(attachment => { if (attachment.path && existsSync(attachment.path)) { - attachment.filename = basename(attachment.path); + attachment.filename = attachment.filename || basename(attachment.path); attachment.data = readFileSync(attachment.path); } From d6a8e5741a5a8faa09c2848814c46e0717f526c7 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 3 May 2022 13:29:35 +0100 Subject: [PATCH 15/26] Improve tests --- packages/utils/test/envelope.test.ts | 51 ++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index 8f7fce2825a7..af63b5bfb9bf 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -21,9 +21,43 @@ describe('envelope', () => { describe('serializeEnvelope()', () => { it('serializes an envelope', () => { const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, []); - const [headers] = parseEnvelope(serializeEnvelope(env, new TextEncoder())); + const serializedEnvelope = serializeEnvelope(env, new TextEncoder()); + expect(typeof serializedEnvelope).toBe('string'); + + const [headers] = parseEnvelope(serializedEnvelope); expect(headers).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); }); + + it('serializes an envelope with attachments', () => { + const items: EventEnvelope[1] = [ + [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], + [{ type: 'attachment', filename: 'bar.txt', length: 6 }, Uint8Array.from([1, 2, 3, 4, 5, 6])], + [{ type: 'attachment', filename: 'foo.txt', length: 6 }, Uint8Array.from([7, 8, 9, 10, 11, 12])], + ]; + + const env = createEnvelope( + { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, + items, + ); + + expect.assertions(6); + + const serializedEnvelope = serializeEnvelope(env, new TextEncoder()); + expect(serializedEnvelope).toBeInstanceOf(Uint8Array); + + const [parsedHeaders, parsedItems] = parseEnvelope(serializedEnvelope); + expect(parsedHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); + expect(parsedItems).toHaveLength(3); + expect(items[0]).toEqual([{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]); + expect(items[1]).toEqual([ + { type: 'attachment', filename: 'bar.txt', length: 6 }, + Uint8Array.from([1, 2, 3, 4, 5, 6]), + ]); + expect(items[2]).toEqual([ + { type: 'attachment', filename: 'foo.txt', length: 6 }, + Uint8Array.from([7, 8, 9, 10, 11, 12]), + ]); + }); }); describe('addItemToEnvelope()', () => { @@ -58,7 +92,7 @@ describe('envelope', () => { items, ); - expect.assertions(11); + expect.assertions(6); let iteration = 0; forEachEnvelopeItem(env, (item, type) => { @@ -66,19 +100,6 @@ describe('envelope', () => { expect(type).toBe(items[iteration][0].type); iteration = iteration + 1; }); - - const [parsedHeaders, parsedItems] = parseEnvelope(serializeEnvelope(env, new TextEncoder())); - expect(parsedHeaders).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); - expect(parsedItems).toHaveLength(3); - expect(items[0]).toEqual([{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]); - expect(items[1]).toEqual([ - { type: 'attachment', filename: 'bar.txt', length: 6 }, - Uint8Array.from([1, 2, 3, 4, 5, 6]), - ]); - expect(items[2]).toEqual([ - { type: 'attachment', filename: 'foo.txt', length: 6 }, - Uint8Array.from([7, 8, 9, 10, 11, 12]), - ]); }); }); }); From 097924582ee865a2d897d6be552e4861e88b6769 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 3 May 2022 14:37:27 +0100 Subject: [PATCH 16/26] Improve types --- packages/browser/src/client.ts | 4 ++-- packages/core/src/baseclient.ts | 20 +++++++++++++++----- packages/node/src/client.ts | 23 ++++++++++++++++++----- packages/types/src/attachment.ts | 14 +++++++++++--- packages/types/src/index.ts | 2 +- packages/utils/src/envelope.ts | 30 +++++++++++++++++------------- 6 files changed, 64 insertions(+), 29 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 3eaf26a73066..7aa25965f137 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,5 +1,5 @@ import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core'; -import { Attachment, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; +import { AttachmentWithData, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; import { createClientReportEnvelope, dsnToString, getGlobalObject, logger, serializeEnvelope } from '@sentry/utils'; import { eventFromException, eventFromMessage } from './eventbuilder'; @@ -103,7 +103,7 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - protected _sendEvent(event: Event, attachments: Attachment[]): void { + protected _sendEvent(event: Event, attachments: AttachmentWithData[]): void { const integration = this.getIntegration(Breadcrumbs); if (integration) { integration.addSentryBreadcrumb(event); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index abdc41b52f2e..a1236d168208 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ import { Scope, Session } from '@sentry/hub'; import { - Attachment, + AttachmentWithData, Client, ClientOptions, DataCategory, @@ -276,7 +276,7 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public sendEvent(event: Event, attachments?: Attachment[]): void { + public sendEvent(event: Event, attachments?: AttachmentWithData[]): void { if (this._dsn) { const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); @@ -549,7 +549,7 @@ export abstract class BaseClient implements Client { * @param event The Sentry event to send */ // TODO(v7): refactor: get rid of method? - protected _sendEvent(event: Event, attachments?: Attachment[]): void { + protected _sendEvent(event: Event, attachments?: AttachmentWithData[]): void { this.sendEvent(event, attachments); } @@ -670,8 +670,18 @@ export abstract class BaseClient implements Client { /** * Loads attachments from scope */ - protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { - return scope?.getAttachments() || []; + protected _attachmentsFromScope(scope: Scope | undefined): AttachmentWithData[] { + return ( + scope?.getAttachments()?.map(attachment => { + if ('path' in attachment || !('data' in attachment)) { + throw new SentryError( + 'This SDK does not support loading attachments from file paths and no data was supplied', + ); + } + + return attachment; + }) || [] + ); } /** diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index a634c68cde8d..6015d1af3040 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,6 +1,6 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; -import { Attachment, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; +import { AttachmentWithData, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; import { basename, logger, resolvedSyncPromise } from '@sentry/utils'; import { existsSync, readFileSync } from 'fs'; import { TextEncoder } from 'util'; @@ -35,6 +35,7 @@ export class NodeClient extends BaseClient { version: SDK_VERSION, }; + // Until node supports global TextEncoder in all versions we support, we are forced to pass it from util options.transportOptions = { textEncoder: new TextEncoder(), ...options.transportOptions, @@ -161,12 +162,24 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { + protected _attachmentsFromScope(scope: Scope | undefined): AttachmentWithData[] { return ( scope?.getAttachments()?.map(attachment => { - if (attachment.path && existsSync(attachment.path)) { - attachment.filename = attachment.filename || basename(attachment.path); - attachment.data = readFileSync(attachment.path); + if ('path' in attachment && !('data' in attachment)) { + if (existsSync(attachment.path)) { + return { + filename: attachment.filename || basename(attachment.path), + data: readFileSync(attachment.path), + contentType: attachment.contentType, + attachmentType: attachment.attachmentType, + }; + } else { + return { + filename: attachment.filename || basename(attachment.path), + data: 'Missing attachment - file not found', + contentType: 'text/plain', + }; + } } return attachment; diff --git a/packages/types/src/attachment.ts b/packages/types/src/attachment.ts index 3f6cb9089182..5940a6789cb5 100644 --- a/packages/types/src/attachment.ts +++ b/packages/types/src/attachment.ts @@ -1,6 +1,14 @@ -export interface Attachment { - path?: string; - data?: string | Uint8Array; +export type Attachment = AttachmentWithPath | AttachmentWithData; + +export interface AttachmentWithData { + data: string | Uint8Array; + filename: string; + contentType?: string; + attachmentType?: string; +} + +interface AttachmentWithPath { + path: string; filename?: string; contentType?: string; attachmentType?: string; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 8e4a3e533253..ab9ae0444007 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,4 +1,4 @@ -export type { Attachment } from './attachment'; +export type { Attachment, AttachmentWithData } from './attachment'; export type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport, Outcome, EventDropReason } from './clientreport'; diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 290a0b21f913..faf36da83d5c 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -1,4 +1,11 @@ -import { Attachment, AttachmentItem, DataCategory, Envelope, EnvelopeItem, EnvelopeItemType } from '@sentry/types'; +import { + AttachmentItem, + AttachmentWithData, + DataCategory, + Envelope, + EnvelopeItem, + EnvelopeItemType, +} from '@sentry/types'; /** * Creates an envelope. @@ -47,16 +54,12 @@ export function serializeEnvelope(envelope: Envelope, textEncoder?: TextEncoderI const [envHeaders, items] = envelope; - // Initially we construct our envelope as a string and only convert to binary if we encounter binary data + // Initially we construct our envelope as a string and only convert to binary chunks if we encounter binary data let parts: string | Uint8Array[] = JSON.stringify(envHeaders); function append(next: string | Uint8Array): void { if (typeof parts === 'string') { - if (typeof next === 'string') { - parts += next; - } else { - parts = [utf8.encode(parts), next]; - } + parts = typeof next === 'string' ? parts + next : [utf8.encode(parts), next]; } else { parts.push(typeof next === 'string' ? utf8.encode(next) : next); } @@ -88,21 +91,22 @@ function concatBuffers(buffers: Uint8Array[]): Uint8Array { * Creates attachment envelope items */ export function createAttachmentEnvelopeItem( - attachment: Attachment, + attachment: AttachmentWithData, textEncoder?: TextEncoderInternal, ): AttachmentItem { const utf8 = textEncoder || new TextEncoder(); + const buffer = typeof attachment.data === 'string' ? utf8.encode(attachment.data) : attachment.data; return [ { type: 'attachment', - length: buffer?.length || 0, - filename: attachment?.filename || 'No filename', - content_type: attachment?.contentType, - attachment_type: attachment?.attachmentType, + length: buffer.length, + filename: attachment.filename, + content_type: attachment.contentType, + attachment_type: attachment.attachmentType, }, - buffer || new Uint8Array(0), + buffer, ]; } From 6cc6d60afd8f1e1f649c2148340786f846bc08e8 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 3 May 2022 14:51:12 +0100 Subject: [PATCH 17/26] Couple more minor fixes --- packages/core/src/baseclient.ts | 12 ++++++------ packages/utils/src/envelope.ts | 6 ++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index a1236d168208..7e6d46b7216b 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -672,15 +672,15 @@ export abstract class BaseClient implements Client { */ protected _attachmentsFromScope(scope: Scope | undefined): AttachmentWithData[] { return ( - scope?.getAttachments()?.map(attachment => { + scope?.getAttachments()?.reduce((acc, attachment) => { if ('path' in attachment || !('data' in attachment)) { - throw new SentryError( - 'This SDK does not support loading attachments from file paths and no data was supplied', - ); + logger.error('This SDK does not support loading attachments from file paths and no data was supplied'); + } else { + acc.push(attachment); } - return attachment; - }) || [] + return acc; + }, [] as AttachmentWithData[]) || [] ); } diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index faf36da83d5c..0ad4d345c40d 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -7,6 +7,8 @@ import { EnvelopeItemType, } from '@sentry/types'; +import { dropUndefinedKeys } from './object'; + /** * Creates an envelope. * Make sure to always explicitly provide the generic to this function @@ -99,13 +101,13 @@ export function createAttachmentEnvelopeItem( const buffer = typeof attachment.data === 'string' ? utf8.encode(attachment.data) : attachment.data; return [ - { + dropUndefinedKeys({ type: 'attachment', length: buffer.length, filename: attachment.filename, content_type: attachment.contentType, attachment_type: attachment.attachmentType, - }, + }), buffer, ]; } From 99446c72560c57dcd6ca1651dfff757241403049 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 3 May 2022 15:12:13 +0100 Subject: [PATCH 18/26] More type improve --- packages/browser/src/client.ts | 4 ++-- packages/core/src/baseclient.ts | 12 +++++------- packages/hub/src/scope.ts | 8 ++++---- packages/node/src/client.ts | 9 ++++++--- packages/node/test/index.test.ts | 6 +++--- packages/types/src/attachment.ts | 6 +++--- packages/types/src/index.ts | 2 +- packages/types/src/scope.ts | 6 +++--- packages/utils/src/envelope.ts | 11 ++--------- 9 files changed, 29 insertions(+), 35 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 285ddccd58bc..cdbf0dada9d5 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,5 +1,5 @@ import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core'; -import { AttachmentWithData,ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; +import { Attachment, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; import { createClientReportEnvelope, dsnToString, @@ -103,7 +103,7 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - public sendEvent(event: Event, attachments?: AttachmentWithData[]): void { + public sendEvent(event: Event, attachments?: Attachment[]): 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 diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index ad86466f9f44..8c0d437795c5 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ import { Scope, Session } from '@sentry/hub'; import { - AttachmentWithData, + Attachment, Client, ClientOptions, DataCategory, @@ -285,7 +285,7 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public sendEvent(event: Event, attachments?: AttachmentWithData[]): void { + public sendEvent(event: Event, attachments?: Attachment[]): void { if (this._dsn) { const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); @@ -670,17 +670,15 @@ export abstract class BaseClient implements Client { /** * Loads attachments from scope */ - protected _attachmentsFromScope(scope: Scope | undefined): AttachmentWithData[] { + protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { return ( scope?.getAttachments()?.reduce((acc, attachment) => { - if ('path' in attachment || !('data' in attachment)) { - logger.error('This SDK does not support loading attachments from file paths and no data was supplied'); - } else { + if ('data' in attachment) { acc.push(attachment); } return acc; - }, [] as AttachmentWithData[]) || [] + }, [] as Attachment[]) || [] ); } diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 28187671a054..6ea763ee55ac 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ import { - Attachment, + AttachmentOptions, Breadcrumb, CaptureContext, Context, @@ -87,7 +87,7 @@ export class Scope implements ScopeInterface { protected _requestSession?: RequestSession; /** Attachments */ - protected _attachments: Attachment[] = []; + protected _attachments: AttachmentOptions[] = []; /** * A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get @@ -407,7 +407,7 @@ export class Scope implements ScopeInterface { /** * @inheritDoc */ - public addAttachment(attachment: Attachment): this { + public addAttachment(attachment: AttachmentOptions): this { this._attachments.push(attachment); return this; } @@ -415,7 +415,7 @@ export class Scope implements ScopeInterface { /** * @inheritDoc */ - public getAttachments(): Attachment[] { + public getAttachments(): AttachmentOptions[] { return this._attachments; } diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 6015d1af3040..ef1bd2f4bbcf 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,6 +1,6 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; -import { AttachmentWithData, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; +import { Attachment, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; import { basename, logger, resolvedSyncPromise } from '@sentry/utils'; import { existsSync, readFileSync } from 'fs'; import { TextEncoder } from 'util'; @@ -162,7 +162,7 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _attachmentsFromScope(scope: Scope | undefined): AttachmentWithData[] { + protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { return ( scope?.getAttachments()?.map(attachment => { if ('path' in attachment && !('data' in attachment)) { @@ -174,9 +174,12 @@ export class NodeClient extends BaseClient { attachmentType: attachment.attachmentType, }; } else { + const msg = `Missing attachment - file not found: ${attachment.path}`; + logger.warn(msg); + return { filename: attachment.filename || basename(attachment.path), - data: 'Missing attachment - file not found', + data: msg, contentType: 'text/plain', }; } diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 027678d17100..f1cba940d81f 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,6 +1,6 @@ import { initAndBind, SDK_VERSION } from '@sentry/core'; import { getMainCarrier } from '@sentry/hub'; -import { AttachmentWithData,Integration } from '@sentry/types'; +import { Attachment, Integration } from '@sentry/types'; import * as domain from 'domain'; import { @@ -76,7 +76,7 @@ describe('SentryNode', () => { }); describe('breadcrumbs', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); @@ -107,7 +107,7 @@ describe('SentryNode', () => { }); describe('capture', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); diff --git a/packages/types/src/attachment.ts b/packages/types/src/attachment.ts index 5940a6789cb5..89270a53fc07 100644 --- a/packages/types/src/attachment.ts +++ b/packages/types/src/attachment.ts @@ -1,13 +1,13 @@ -export type Attachment = AttachmentWithPath | AttachmentWithData; +export type AttachmentOptions = Attachment | AttachmentFromPath; -export interface AttachmentWithData { +export interface Attachment { data: string | Uint8Array; filename: string; contentType?: string; attachmentType?: string; } -interface AttachmentWithPath { +interface AttachmentFromPath { path: string; filename?: string; contentType?: string; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index ab9ae0444007..3ee9892f6496 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,4 +1,4 @@ -export type { Attachment, AttachmentWithData } from './attachment'; +export type { Attachment, AttachmentOptions } from './attachment'; export type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport, Outcome, EventDropReason } from './clientreport'; diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index a2a14ffd4664..929721171c2d 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -1,4 +1,4 @@ -import { Attachment } from './attachment'; +import { AttachmentOptions } from './attachment'; import { Breadcrumb } from './breadcrumb'; import { Context, Contexts } from './context'; import { EventProcessor } from './eventprocessor'; @@ -164,12 +164,12 @@ export interface Scope { * Adds an attachment to the scope * @param attachment Attachment options */ - addAttachment(attachment: Attachment): this; + addAttachment(attachment: AttachmentOptions): this; /** * Returns an array of attachments on the scope */ - getAttachments(): Attachment[]; + getAttachments(): AttachmentOptions[]; /** * Clears attachments from the scope diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 0ad4d345c40d..62d94636ffde 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -1,11 +1,4 @@ -import { - AttachmentItem, - AttachmentWithData, - DataCategory, - Envelope, - EnvelopeItem, - EnvelopeItemType, -} from '@sentry/types'; +import { Attachment, AttachmentItem, DataCategory, Envelope, EnvelopeItem, EnvelopeItemType } from '@sentry/types'; import { dropUndefinedKeys } from './object'; @@ -93,7 +86,7 @@ function concatBuffers(buffers: Uint8Array[]): Uint8Array { * Creates attachment envelope items */ export function createAttachmentEnvelopeItem( - attachment: AttachmentWithData, + attachment: Attachment, textEncoder?: TextEncoderInternal, ): AttachmentItem { const utf8 = textEncoder || new TextEncoder(); From 6c404f1870cbbaa1ca9e47782db0e8383decca75 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 4 May 2022 12:25:52 +0100 Subject: [PATCH 19/26] Spread headers --- packages/node/src/transports/http.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 94450485ff7b..3b4496b491b9 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -106,8 +106,11 @@ function createRequestExecutor( let bodyStream = streamFromBody(request.body); if (request.body.length > GZIP_THRESHOLD) { - options.headers = options.headers || {}; - options.headers['Content-Encoding'] = 'gzip'; + options.headers = { + ...options.headers, + 'content-encoding': 'gzip', + }; + bodyStream = bodyStream.pipe(createGzip()); } From 2e85356861e1031be0b5f8e7cac777356dcc83fe Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 5 May 2022 16:00:06 +0100 Subject: [PATCH 20/26] Remove node attachment loading special case --- packages/core/src/baseclient.ts | 17 +-------------- packages/hub/src/scope.ts | 8 +++---- packages/node/src/client.ts | 36 ++------------------------------ packages/types/src/attachment.ts | 9 -------- packages/types/src/index.ts | 2 +- packages/types/src/scope.ts | 6 +++--- 6 files changed, 11 insertions(+), 67 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 8c0d437795c5..21dc0e9952ab 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -630,7 +630,7 @@ export abstract class BaseClient implements Client { this._updateSessionFromEvent(session, processedEvent); } - this.sendEvent(processedEvent, this._attachmentsFromScope(scope)); + this.sendEvent(processedEvent, scope?.getAttachments() || []); return processedEvent; }) .then(null, reason => { @@ -667,21 +667,6 @@ export abstract class BaseClient implements Client { ); } - /** - * Loads attachments from scope - */ - protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { - return ( - scope?.getAttachments()?.reduce((acc, attachment) => { - if ('data' in attachment) { - acc.push(attachment); - } - - return acc; - }, [] as Attachment[]) || [] - ); - } - /** * @inheritdoc */ diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 6ea763ee55ac..28187671a054 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ import { - AttachmentOptions, + Attachment, Breadcrumb, CaptureContext, Context, @@ -87,7 +87,7 @@ export class Scope implements ScopeInterface { protected _requestSession?: RequestSession; /** Attachments */ - protected _attachments: AttachmentOptions[] = []; + protected _attachments: Attachment[] = []; /** * A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get @@ -407,7 +407,7 @@ export class Scope implements ScopeInterface { /** * @inheritDoc */ - public addAttachment(attachment: AttachmentOptions): this { + public addAttachment(attachment: Attachment): this { this._attachments.push(attachment); return this; } @@ -415,7 +415,7 @@ export class Scope implements ScopeInterface { /** * @inheritDoc */ - public getAttachments(): AttachmentOptions[] { + public getAttachments(): Attachment[] { return this._attachments; } diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index ef1bd2f4bbcf..78903ea68f06 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,8 +1,7 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core'; import { SessionFlusher } from '@sentry/hub'; -import { Attachment, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; -import { basename, logger, resolvedSyncPromise } from '@sentry/utils'; -import { existsSync, readFileSync } from 'fs'; +import { Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; +import { logger, resolvedSyncPromise } from '@sentry/utils'; import { TextEncoder } from 'util'; import { eventFromMessage, eventFromUnknownInput } from './eventbuilder'; @@ -158,35 +157,4 @@ export class NodeClient extends BaseClient { this._sessionFlusher.incrementSessionStatusCount(); } } - - /** - * @inheritDoc - */ - protected _attachmentsFromScope(scope: Scope | undefined): Attachment[] { - return ( - scope?.getAttachments()?.map(attachment => { - if ('path' in attachment && !('data' in attachment)) { - if (existsSync(attachment.path)) { - return { - filename: attachment.filename || basename(attachment.path), - data: readFileSync(attachment.path), - contentType: attachment.contentType, - attachmentType: attachment.attachmentType, - }; - } else { - const msg = `Missing attachment - file not found: ${attachment.path}`; - logger.warn(msg); - - return { - filename: attachment.filename || basename(attachment.path), - data: msg, - contentType: 'text/plain', - }; - } - } - - return attachment; - }) || [] - ); - } } diff --git a/packages/types/src/attachment.ts b/packages/types/src/attachment.ts index 89270a53fc07..55cc795732ea 100644 --- a/packages/types/src/attachment.ts +++ b/packages/types/src/attachment.ts @@ -1,15 +1,6 @@ -export type AttachmentOptions = Attachment | AttachmentFromPath; - export interface Attachment { data: string | Uint8Array; filename: string; contentType?: string; attachmentType?: string; } - -interface AttachmentFromPath { - path: string; - filename?: string; - contentType?: string; - attachmentType?: string; -} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 3ee9892f6496..8e4a3e533253 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,4 +1,4 @@ -export type { Attachment, AttachmentOptions } from './attachment'; +export type { Attachment } from './attachment'; export type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport, Outcome, EventDropReason } from './clientreport'; diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index 929721171c2d..a2a14ffd4664 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -1,4 +1,4 @@ -import { AttachmentOptions } from './attachment'; +import { Attachment } from './attachment'; import { Breadcrumb } from './breadcrumb'; import { Context, Contexts } from './context'; import { EventProcessor } from './eventprocessor'; @@ -164,12 +164,12 @@ export interface Scope { * Adds an attachment to the scope * @param attachment Attachment options */ - addAttachment(attachment: AttachmentOptions): this; + addAttachment(attachment: Attachment): this; /** * Returns an array of attachments on the scope */ - getAttachments(): AttachmentOptions[]; + getAttachments(): Attachment[]; /** * Clears attachments from the scope From 2d25f862c672d86ef4a7e7b7a8d04e58c01f23fb Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 5 May 2022 23:54:23 +0100 Subject: [PATCH 21/26] Remove the need for custom jest env --- packages/browser/jest.config.js | 2 +- packages/browser/jest.env.js | 15 --------------- packages/browser/test/unit/index.test.ts | 15 +++++++++------ .../browser/test/unit/transports/fetch.test.ts | 1 + packages/browser/test/unit/transports/xhr.test.ts | 1 + packages/gatsby/jest.config.js | 2 +- 6 files changed, 13 insertions(+), 23 deletions(-) delete mode 100644 packages/browser/jest.env.js diff --git a/packages/browser/jest.config.js b/packages/browser/jest.config.js index 264feb13e33a..f9cd8056a454 100644 --- a/packages/browser/jest.config.js +++ b/packages/browser/jest.config.js @@ -2,6 +2,6 @@ const baseConfig = require('../../jest/jest.config.js'); module.exports = { ...baseConfig, - testEnvironment: './jest.env.js', + testEnvironment: 'jsdom', testMatch: ['/test/unit/**/*.test.ts'], }; diff --git a/packages/browser/jest.env.js b/packages/browser/jest.env.js deleted file mode 100644 index 3a7ac5019468..000000000000 --- a/packages/browser/jest.env.js +++ /dev/null @@ -1,15 +0,0 @@ -const Environment = require('jest-environment-jsdom'); - -// Looks like jsdom does not support global TextEncoder/TextDecoder -// https://github.com/jsdom/jsdom/issues/2524 - -module.exports = class CustomTestEnvironment extends Environment { - async setup() { - await super.setup(); - if (typeof this.global.TextEncoder === 'undefined') { - const { TextEncoder, TextDecoder } = require('util'); - this.global.TextEncoder = TextEncoder; - this.global.TextDecoder = TextDecoder; - } - } -}; diff --git a/packages/browser/test/unit/index.test.ts b/packages/browser/test/unit/index.test.ts index 37fa972512a1..758059928f58 100644 --- a/packages/browser/test/unit/index.test.ts +++ b/packages/browser/test/unit/index.test.ts @@ -40,6 +40,7 @@ describe('SentryBrowser', () => { beforeSend, dsn, transport: makeSimpleTransport, + autoSessionTracking: false, }); }); @@ -218,20 +219,20 @@ describe('SentryBrowser', () => { describe('SentryBrowser initialization', () => { it('should use window.SENTRY_RELEASE to set release on initialization if available', () => { global.SENTRY_RELEASE = { id: 'foobar' }; - init({ dsn }); + init({ dsn, autoSessionTracking: false }); expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBe('foobar'); delete global.SENTRY_RELEASE; }); it('should use initialScope', () => { - init({ dsn, initialScope: { tags: { a: 'b' } } }); + init({ dsn, initialScope: { tags: { a: 'b' } }, autoSessionTracking: false }); expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ a: 'b' }); }); it('should use initialScope Scope', () => { const scope = new Scope(); scope.setTags({ a: 'b' }); - init({ dsn, initialScope: scope }); + init({ dsn, initialScope: scope, autoSessionTracking: false }); expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ a: 'b' }); }); @@ -242,19 +243,20 @@ describe('SentryBrowser initialization', () => { scope.setTags({ a: 'b' }); return scope; }, + autoSessionTracking: false, }); expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ a: 'b' }); }); it('should have initialization proceed as normal if window.SENTRY_RELEASE is not set', () => { // This is mostly a happy-path test to ensure that the initialization doesn't throw an error. - init({ dsn }); + init({ dsn, autoSessionTracking: false }); expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBeUndefined(); }); describe('SDK metadata', () => { it('should set SDK data when Sentry.init() is called', () => { - init({ dsn }); + init({ dsn, autoSessionTracking: false }); const sdkData = (getCurrentHub().getClient() as any).getOptions()._metadata.sdk; @@ -265,7 +267,7 @@ describe('SentryBrowser initialization', () => { }); it('should set SDK data when instantiating a client directly', () => { - const options = getDefaultBrowserClientOptions({ dsn }); + const options = getDefaultBrowserClientOptions({ dsn, autoSessionTracking: false }); const client = new BrowserClient(options); const sdkData = client.getOptions()._metadata?.sdk as any; @@ -281,6 +283,7 @@ describe('SentryBrowser initialization', () => { it("shouldn't overwrite SDK data that's already there", () => { init({ dsn, + autoSessionTracking: false, // this would normally be set by the wrapper SDK in init() _metadata: { sdk: { diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 9f885f0843f9..be0ee72afc45 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -8,6 +8,7 @@ import { FetchImpl } from '../../../src/transports/utils'; const DEFAULT_FETCH_TRANSPORT_OPTIONS: FetchTransportOptions = { url: 'https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7', recordDroppedEvent: () => undefined, + textEncoder: new TextEncoder(), }; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts index bb2fab932344..76ac0d2dbc92 100644 --- a/packages/browser/test/unit/transports/xhr.test.ts +++ b/packages/browser/test/unit/transports/xhr.test.ts @@ -7,6 +7,7 @@ import { makeXHRTransport, XHRTransportOptions } from '../../../src/transports/x const DEFAULT_XHR_TRANSPORT_OPTIONS: XHRTransportOptions = { url: 'https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7', recordDroppedEvent: () => undefined, + textEncoder: new TextEncoder(), }; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ diff --git a/packages/gatsby/jest.config.js b/packages/gatsby/jest.config.js index fffe12d6a4d8..cc7d8162cd59 100644 --- a/packages/gatsby/jest.config.js +++ b/packages/gatsby/jest.config.js @@ -3,5 +3,5 @@ const baseConfig = require('../../jest/jest.config.js'); module.exports = { ...baseConfig, setupFiles: ['/test/setEnvVars.ts'], - testEnvironment: '../browser/jest.env.js', + testEnvironment: 'jsdom', }; From 1731945f6e344b9215bdc4b09886a78b8cd4e83b Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 6 May 2022 10:31:52 +0100 Subject: [PATCH 22/26] Fix gatsby tests --- packages/gatsby/test/integration.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/gatsby/test/integration.test.tsx b/packages/gatsby/test/integration.test.tsx index 18c738ba2ac2..3fd06dc21990 100644 --- a/packages/gatsby/test/integration.test.tsx +++ b/packages/gatsby/test/integration.test.tsx @@ -3,6 +3,7 @@ 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'; @@ -10,6 +11,8 @@ 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', () => { From 250260b73c2cb0efe8170b161d99138773b5eb02 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 12 May 2022 10:21:47 +0100 Subject: [PATCH 23/26] Pass through hint --- packages/browser/src/client.ts | 10 +++---- packages/browser/test/unit/index.test.ts | 17 ++++++------ packages/core/src/baseclient.ts | 34 ++++++++++++++---------- packages/core/test/mocks/client.ts | 6 ++--- packages/hub/src/exports.ts | 5 ++-- packages/node/src/client.ts | 4 +-- packages/node/test/index.test.ts | 6 ++--- packages/types/src/client.ts | 2 +- packages/types/src/event.ts | 2 ++ packages/utils/src/envelope.ts | 11 +++++--- 10 files changed, 54 insertions(+), 43 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index cdbf0dada9d5..68b6aabd6d0c 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,5 +1,5 @@ import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core'; -import { Attachment, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; +import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; import { createClientReportEnvelope, dsnToString, @@ -103,7 +103,7 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - public sendEvent(event: Event, attachments?: Attachment[]): 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 @@ -132,15 +132,15 @@ export class BrowserClient extends BaseClient { ); } - super.sendEvent(event, attachments); + super.sendEvent(event, hint); } /** * @inheritDoc */ - protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike { + protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { event.platform = event.platform || 'javascript'; - return super._prepareEvent(event, scope, hint); + return super._prepareEvent(event, hint, scope); } /** diff --git a/packages/browser/test/unit/index.test.ts b/packages/browser/test/unit/index.test.ts index 758059928f58..b4d02cbc18c1 100644 --- a/packages/browser/test/unit/index.test.ts +++ b/packages/browser/test/unit/index.test.ts @@ -40,7 +40,7 @@ describe('SentryBrowser', () => { beforeSend, dsn, transport: makeSimpleTransport, - autoSessionTracking: false, + // }); }); @@ -219,20 +219,20 @@ describe('SentryBrowser', () => { describe('SentryBrowser initialization', () => { it('should use window.SENTRY_RELEASE to set release on initialization if available', () => { global.SENTRY_RELEASE = { id: 'foobar' }; - init({ dsn, autoSessionTracking: false }); + init({ dsn }); expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBe('foobar'); delete global.SENTRY_RELEASE; }); it('should use initialScope', () => { - init({ dsn, initialScope: { tags: { a: 'b' } }, autoSessionTracking: false }); + init({ dsn, initialScope: { tags: { a: 'b' } } }); expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ a: 'b' }); }); it('should use initialScope Scope', () => { const scope = new Scope(); scope.setTags({ a: 'b' }); - init({ dsn, initialScope: scope, autoSessionTracking: false }); + init({ dsn, initialScope: scope }); expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ a: 'b' }); }); @@ -243,20 +243,19 @@ describe('SentryBrowser initialization', () => { scope.setTags({ a: 'b' }); return scope; }, - autoSessionTracking: false, }); expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ a: 'b' }); }); it('should have initialization proceed as normal if window.SENTRY_RELEASE is not set', () => { // This is mostly a happy-path test to ensure that the initialization doesn't throw an error. - init({ dsn, autoSessionTracking: false }); + init({ dsn }); expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBeUndefined(); }); describe('SDK metadata', () => { it('should set SDK data when Sentry.init() is called', () => { - init({ dsn, autoSessionTracking: false }); + init({ dsn }); const sdkData = (getCurrentHub().getClient() as any).getOptions()._metadata.sdk; @@ -267,7 +266,7 @@ describe('SentryBrowser initialization', () => { }); it('should set SDK data when instantiating a client directly', () => { - const options = getDefaultBrowserClientOptions({ dsn, autoSessionTracking: false }); + const options = getDefaultBrowserClientOptions({ dsn }); const client = new BrowserClient(options); const sdkData = client.getOptions()._metadata?.sdk as any; @@ -283,7 +282,7 @@ describe('SentryBrowser initialization', () => { it("shouldn't overwrite SDK data that's already there", () => { init({ dsn, - autoSessionTracking: false, + // this would normally be set by the wrapper SDK in init() _metadata: { sdk: { diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 21dc0e9952ab..43153e3bb8d0 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -1,7 +1,6 @@ /* eslint-disable max-lines */ import { Scope, Session } from '@sentry/hub'; import { - Attachment, Client, ClientOptions, DataCategory, @@ -131,7 +130,7 @@ export abstract class BaseClient implements Client { this._process( this.eventFromException(exception, hint) - .then(event => this._captureEvent(event, hint, scope)) + .then(event => this._captureEvent(event, hint || {}, scope)) .then(result => { eventId = result; }), @@ -158,7 +157,7 @@ export abstract class BaseClient implements Client { this._process( promisedEvent - .then(event => this._captureEvent(event, hint, scope)) + .then(event => this._captureEvent(event, hint || {}, scope)) .then(result => { eventId = result; }), @@ -180,7 +179,7 @@ export abstract class BaseClient implements Client { let eventId: string | undefined = hint && hint.event_id; this._process( - this._captureEvent(event, hint, scope).then(result => { + this._captureEvent(event, hint || {}, scope).then(result => { eventId = result; }), ); @@ -285,11 +284,11 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public sendEvent(event: Event, attachments?: Attachment[]): 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 attachments || []) { + for (const attachment of hint?.attachments || []) { addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment, this._options.transportOptions?.textEncoder)); } @@ -408,11 +407,11 @@ export abstract class BaseClient implements Client { * @param scope A scope containing event metadata. * @returns A new event with more information. */ - protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike { + protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { 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(), }; @@ -422,7 +421,7 @@ export abstract class BaseClient implements Client { // 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); } @@ -432,6 +431,13 @@ export abstract class BaseClient implements Client { // 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); } @@ -559,7 +565,7 @@ export abstract class BaseClient implements Client { * @param hint * @param scope */ - protected _captureEvent(event: Event, hint?: EventHint, scope?: Scope): PromiseLike { + protected _captureEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { return this._processEvent(event, hint, scope).then( finalEvent => { return finalEvent.event_id; @@ -584,7 +590,7 @@ export abstract class BaseClient implements Client { * @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 { + protected _processEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { const { beforeSend, sampleRate } = this.getOptions(); if (!this._isEnabled()) { @@ -604,14 +610,14 @@ export abstract class BaseClient implements Client { ); } - 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; } @@ -630,7 +636,7 @@ export abstract class BaseClient implements Client { this._updateSessionFromEvent(session, processedEvent); } - this.sendEvent(processedEvent, scope?.getAttachments() || []); + this.sendEvent(processedEvent, hint); return processedEvent; }) .then(null, reason => { diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index c0aadaa5cbf8..b6a305fa762b 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -1,5 +1,5 @@ import { Session } from '@sentry/hub'; -import { ClientOptions, Event, Integration, Outcome, Severity, SeverityLevel } from '@sentry/types'; +import { ClientOptions, Event, Integration, Outcome, Severity, SeverityLevel, EventHint } from '@sentry/types'; import { resolvedSyncPromise } from '@sentry/utils'; import { TextEncoder } from 'util'; @@ -68,10 +68,10 @@ export class TestClient extends BaseClient { 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 diff --git a/packages/hub/src/exports.ts b/packages/hub/src/exports.ts index 2ae3000df1c1..528da6289d3e 100644 --- a/packages/hub/src/exports.ts +++ b/packages/hub/src/exports.ts @@ -3,6 +3,7 @@ import { CaptureContext, CustomSamplingContext, Event, + EventHint, Extra, Extras, Primitive, @@ -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 { - return getCurrentHub().captureEvent(event); +export function captureEvent(event: Event, hint?: EventHint): ReturnType { + return getCurrentHub().captureEvent(event, hint); } /** diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 78903ea68f06..9ebad827d3e4 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -138,12 +138,12 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike { + protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { event.platform = event.platform || 'node'; if (this.getOptions().serverName) { event.server_name = this.getOptions().serverName; } - return super._prepareEvent(event, scope, hint); + return super._prepareEvent(event, hint, scope); } /** diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index f1cba940d81f..a0c18ece93cf 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,6 +1,6 @@ import { initAndBind, SDK_VERSION } from '@sentry/core'; import { getMainCarrier } from '@sentry/hub'; -import { Attachment, Integration } from '@sentry/types'; +import { Integration, EventHint } from '@sentry/types'; import * as domain from 'domain'; import { @@ -76,7 +76,7 @@ describe('SentryNode', () => { }); describe('breadcrumbs', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); @@ -107,7 +107,7 @@ describe('SentryNode', () => { }); describe('capture', () => { - let s: jest.SpyInstance; + let s: jest.SpyInstance; beforeEach(() => { s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 })); diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 3425006e645d..b09557ccba75 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -116,7 +116,7 @@ export interface Client { ): PromiseLike; /** Submits the event to Sentry */ - sendEvent(event: Event): void; + sendEvent(event: Event, hint?: EventHint): void; /** Submits the session to Sentry */ sendSession(session: Session | SessionAggregates): void; diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 7cb3047ed4d5..c769feee65ff 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -1,3 +1,4 @@ +import { Attachment } from './attachment'; import { Breadcrumb } from './breadcrumb'; import { Contexts } from './context'; import { DebugMeta } from './debugMeta'; @@ -56,5 +57,6 @@ export interface EventHint { captureContext?: CaptureContext; syntheticException?: Error | null; originalException?: Error | string | null; + attachments?: Attachment[]; data?: any; } diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 62d94636ffde..633a1146013d 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -41,12 +41,15 @@ interface TextEncoderInternal extends TextEncoderCommon { encode(input?: string): Uint8Array; } +function encodeUTF8(input: string, textEncoder?: TextEncoderInternal): Uint8Array { + const utf8 = textEncoder || new TextEncoder(); + return utf8.encode(input); +} + /** * Serializes an envelope. */ export function serializeEnvelope(envelope: Envelope, textEncoder?: TextEncoderInternal): string | Uint8Array { - const utf8 = textEncoder || new TextEncoder(); - const [envHeaders, items] = envelope; // Initially we construct our envelope as a string and only convert to binary chunks if we encounter binary data @@ -54,9 +57,9 @@ export function serializeEnvelope(envelope: Envelope, textEncoder?: TextEncoderI function append(next: string | Uint8Array): void { if (typeof parts === 'string') { - parts = typeof next === 'string' ? parts + next : [utf8.encode(parts), next]; + parts = typeof next === 'string' ? parts + next : [encodeUTF8(parts, textEncoder), next]; } else { - parts.push(typeof next === 'string' ? utf8.encode(next) : next); + parts.push(typeof next === 'string' ? encodeUTF8(next, textEncoder) : next); } } From 21a04d87d06165f9c87bbd97517563b1637ba327 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 15 May 2022 19:01:42 +0100 Subject: [PATCH 24/26] Lint --- packages/node/test/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index a0c18ece93cf..b12bd23fdd17 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,6 +1,6 @@ import { initAndBind, SDK_VERSION } from '@sentry/core'; import { getMainCarrier } from '@sentry/hub'; -import { Integration, EventHint } from '@sentry/types'; +import { EventHint, Integration } from '@sentry/types'; import * as domain from 'domain'; import { From be927c84e5916e6e3de1e6fbbfcce398cd7059a4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 15 May 2022 19:05:59 +0100 Subject: [PATCH 25/26] Remove erroneous changes --- packages/browser/test/unit/index.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/browser/test/unit/index.test.ts b/packages/browser/test/unit/index.test.ts index b4d02cbc18c1..37fa972512a1 100644 --- a/packages/browser/test/unit/index.test.ts +++ b/packages/browser/test/unit/index.test.ts @@ -40,7 +40,6 @@ describe('SentryBrowser', () => { beforeSend, dsn, transport: makeSimpleTransport, - // }); }); @@ -282,7 +281,6 @@ describe('SentryBrowser initialization', () => { it("shouldn't overwrite SDK data that's already there", () => { init({ dsn, - // this would normally be set by the wrapper SDK in init() _metadata: { sdk: { From c8d43eb18420f81d3908eab170f7c9e52f4dd74e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 16 May 2022 18:21:18 +0100 Subject: [PATCH 26/26] Review changes --- packages/core/src/baseclient.ts | 14 ++++----- packages/node/src/transports/http-module.ts | 2 +- packages/node/src/transports/http.ts | 33 +-------------------- packages/utils/src/envelope.ts | 4 +-- 4 files changed, 10 insertions(+), 43 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 2b2ef5177cbc..8ddc1877f62f 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -131,7 +131,7 @@ export abstract class BaseClient implements Client { this._process( this.eventFromException(exception, hint) - .then(event => this._captureEvent(event, hint || {}, scope)) + .then(event => this._captureEvent(event, hint, scope)) .then(result => { eventId = result; }), @@ -158,7 +158,7 @@ export abstract class BaseClient implements Client { this._process( promisedEvent - .then(event => this._captureEvent(event, hint || {}, scope)) + .then(event => this._captureEvent(event, hint, scope)) .then(result => { eventId = result; }), @@ -180,7 +180,7 @@ export abstract class BaseClient implements Client { let eventId: string | undefined = hint && hint.event_id; this._process( - this._captureEvent(event, hint || {}, scope).then(result => { + this._captureEvent(event, hint, scope).then(result => { eventId = result; }), ); @@ -285,11 +285,11 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public sendEvent(event: Event, hint?: EventHint): 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 || []) { + for (const attachment of hint.attachments || []) { addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment, this._options.transportOptions?.textEncoder)); } @@ -433,7 +433,7 @@ export abstract class BaseClient implements Client { // {@link Hub.addEventProcessor} gets the finished prepared event. if (finalScope) { // Collect attachments from the hint and scope - const attachments = [...(hint?.attachments || []), ...finalScope.getAttachments()]; + const attachments = [...(hint.attachments || []), ...finalScope.getAttachments()]; if (attachments.length) { hint.attachments = attachments; @@ -566,7 +566,7 @@ export abstract class BaseClient implements Client { * @param hint * @param scope */ - protected _captureEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { + protected _captureEvent(event: Event, hint: EventHint = {}, scope?: Scope): PromiseLike { return this._processEvent(event, hint, scope).then( finalEvent => { return finalEvent.event_id; diff --git a/packages/node/src/transports/http-module.ts b/packages/node/src/transports/http-module.ts index 0189d4971e4b..3d21faf2fc34 100644 --- a/packages/node/src/transports/http-module.ts +++ b/packages/node/src/transports/http-module.ts @@ -20,7 +20,7 @@ export interface HTTPModuleRequestIncomingMessage { * Some transports work in a special Javascript environment where http.IncomingMessage is not available. */ export interface HTTPModuleClientRequest { - end(chunk: string): void; + end(chunk: string | Uint8Array): void; on(event: 'error', listener: () => void): void; } diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index e9dd46bca92b..64bf52070eb9 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -8,9 +8,7 @@ import { } from '@sentry/types'; import * as http from 'http'; import * as https from 'https'; -import { Readable, Writable } from 'stream'; import { URL } from 'url'; -import { createGzip } from 'zlib'; import { HTTPModule } from './http-module'; @@ -25,22 +23,6 @@ export interface NodeTransportOptions extends BaseTransportOptions { httpModule?: HTTPModule; } -// Estimated maximum size for reasonable standalone event -const GZIP_THRESHOLD = 1024 * 32; - -/** - * Gets a stream from a Uint8Array or string - * Readable.from was added in node.js v12.3.0 and v10.17.0 - */ -function streamFromBody(body: Uint8Array | string): Readable { - return new Readable({ - read() { - this.push(body); - this.push(null); - }, - }); -} - /** * Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry. */ @@ -103,17 +85,6 @@ function createRequestExecutor( const { hostname, pathname, port, protocol, search } = new URL(options.url); return function makeRequest(request: TransportRequest): Promise { return new Promise((resolve, reject) => { - let bodyStream = streamFromBody(request.body); - - if (request.body.length > GZIP_THRESHOLD) { - options.headers = { - ...options.headers, - 'content-encoding': 'gzip', - }; - - bodyStream = bodyStream.pipe(createGzip()); - } - const req = httpModule.request( { method: 'POST', @@ -152,9 +123,7 @@ function createRequestExecutor( ); req.on('error', reject); - - // The docs say that HTTPModuleClientRequest is Writable but the types don't match exactly - bodyStream.pipe(req as unknown as Writable); + req.end(request.body); }); }; } diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 633a1146013d..1bc69915345d 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -92,9 +92,7 @@ export function createAttachmentEnvelopeItem( attachment: Attachment, textEncoder?: TextEncoderInternal, ): AttachmentItem { - const utf8 = textEncoder || new TextEncoder(); - - const buffer = typeof attachment.data === 'string' ? utf8.encode(attachment.data) : attachment.data; + const buffer = typeof attachment.data === 'string' ? encodeUTF8(attachment.data, textEncoder) : attachment.data; return [ dropUndefinedKeys({