Much improved

timfish · timfish · commit 23f4b29a86d7 · 2022-05-01T05:58:15.000+01:00
diff --git 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: ['<rootDir>/test/unit/**/*.test.ts'],
 };
diff --git a/packages/browser/jest.env.js 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
@@ -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<BrowserClientOptions> {
   /**
    * @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
@@ -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<O extends ClientOptions> implements Client<O> {
   /**
    * @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<O extends ClientOptions> implements Client<O> {
    * @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<O extends ClientOptions> implements Client<O> {
   }
 
   /**
-   * 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
@@ -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<EventEnvelope>(envelopeHeaders, [eventItem, ...attachments]);
+  return createEnvelope<EventEnvelope>(envelopeHeaders, [eventItem]);
 }
diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts
@@ -69,7 +69,7 @@ export function createTransport(
     };
 
     const requestTask = (): PromiseLike<void> =>
-      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
@@ -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
@@ -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<NodeClientOptions> {
   /**
    * @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
@@ -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
@@ -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<void, [Event, AttachmentItem[]?]>;
+    let s: jest.SpyInstance<void, [Event, Attachment[]?]>;
 
     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<void, [Event, AttachmentItem[]?]>;
+    let s: jest.SpyInstance<void, [Event, Attachment[]?]>;
 
     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
@@ -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<EventEnvelope>({ 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
@@ -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<EventEnvelope>({ 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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/packages/utils/test/clientreport.test.ts b/packages/utils/test/clientreport.test.ts
diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts
diff --git a/packages/utils/test/testutils.ts b/packages/utils/test/testutils.ts
