attachments in single envelope

Author: mydea

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts

@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import type { SpanEnvelope, Event } from '@sentry/types';
+import type { Event, SpanEnvelope } from '@sentry/types';
 import { sentryTest } from '../../../../utils/fixtures';
 import type { EventAndTraceHeader } from '../../../../utils/helpers';
 import {

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts

@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import type { SpanEnvelope, Event } from '@sentry/types';
+import type { Event, SpanEnvelope } from '@sentry/types';
 import { sentryTest } from '../../../../utils/fixtures';
 import type { EventAndTraceHeader } from '../../../../utils/helpers';
 import {

packages/core/src/envelope.ts

@@ -1,6 +1,4 @@
 import type {
-  Attachment,
-  AttachmentItem,
   DsnComponents,
   DynamicSamplingContext,
   Event,
@@ -15,7 +13,6 @@ import type {
   SpanEnvelope,
 } from '@sentry/types';
 import {
-  createAttachmentEnvelopeItem,
   createEnvelope,
   createEventEnvelopeHeaders,
   dsnToString,
@@ -95,34 +92,6 @@ export function createEventEnvelope(
   return createEnvelope<EventEnvelope>(envelopeHeaders, [eventItem]);
 }
 
-/**
- * Create an Envelope from an event.
- */
-export function createAttachmentEnvelope(
-  event: Event,
-  attachments: Attachment[],
-  dsn?: DsnComponents,
-  metadata?: SdkMetadata,
-  tunnel?: string,
-): EventEnvelope {
-  const sdkInfo = getSdkMetadataForEnvelopeHeader(metadata);
-  enhanceEventWithSdkInfo(event, metadata && metadata.sdk);
-
-  const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn);
-
-  // Prevent this data (which, if it exists, was used in earlier steps in the processing pipeline) from being sent to
-  // sentry. (Note: Our use of this property comes and goes with whatever we might be debugging, whatever hacks we may
-  // have temporarily added, etc. Even if we don't happen to be using it at some point in the future, let's not get rid
-  // of this `delete`, lest we miss putting it back in the next time the property is in use.)
-  delete event.sdkProcessingMetadata;
-
-  const attachmentItems: AttachmentItem[] = [];
-  for (const attachment of attachments || []) {
-    attachmentItems.push(createAttachmentEnvelopeItem(attachment));
-  }
-  return createEnvelope<EventEnvelope>(envelopeHeaders, attachmentItems);
-}
-
 /**
  * Create envelope from Span item.
  */

packages/core/src/feedback.ts

@@ -1,20 +1,17 @@
 import type { EventHint, FeedbackEvent, SendFeedbackParams } from '@sentry/types';
 import { dropUndefinedKeys } from '@sentry/utils';
 import { getClient, getCurrentScope } from './currentScopes';
-import { createAttachmentEnvelope } from './envelope';
 
 /**
  * Send user feedback to Sentry.
  */
 export function captureFeedback(
   feedbackParams: SendFeedbackParams,
-  hint?: EventHint & { includeReplay?: boolean },
+  hint: EventHint & { includeReplay?: boolean } = {},
 ): string {
-  const { message, name, email, url, source, attachments, associatedEventId } = feedbackParams;
+  const { message, name, email, url, source, associatedEventId } = feedbackParams;
 
   const client = getClient();
-  const transport = client && client.getTransport();
-  const dsn = client && client.getDsn();
 
   const feedbackEvent: FeedbackEvent = {
     contexts: {
@@ -37,25 +34,5 @@ export function captureFeedback(
 
   const eventId = getCurrentScope().captureEvent(feedbackEvent, hint);
 
-  // For now, we have to send attachments manually in a separate envelope
-  // Because we do not support attachments in the feedback envelope
-  // Once the Sentry API properly supports this, we can get rid of this and send it through the event envelope
-  if (client && transport && dsn && attachments && attachments.length) {
-    // TODO: https://docs.sentry.io/platforms/javascript/enriching-events/attachments/
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    void transport.send(
-      createAttachmentEnvelope(
-        {
-          ...feedbackEvent,
-          event_id: eventId,
-        },
-        attachments,
-        dsn,
-        client.getOptions()._metadata,
-        client.getOptions().tunnel,
-      ),
-    );
-  }
-
   return eventId;
 }

packages/core/src/index.ts

@@ -8,7 +8,7 @@ export type { IntegrationIndex } from './integration';
 
 export * from './tracing';
 export * from './semanticAttributes';
-export { createEventEnvelope, createSessionEnvelope, createAttachmentEnvelope, createSpanEnvelope } from './envelope';
+export { createEventEnvelope, createSessionEnvelope, createSpanEnvelope } from './envelope';
 export {
   captureCheckIn,
   withMonitor,

packages/core/test/lib/feedback.test.ts

@@ -152,29 +152,33 @@ describe('captureFeedback', () => {
     const attachment1 = new Uint8Array([1, 2, 3, 4, 5]);
     const attachment2 = new Uint8Array([6, 7, 8, 9]);
 
-    const eventId = captureFeedback({
-      message: 'test',
-      attachments: [
-        {
-          data: attachment1,
-          filename: 'test-file.txt',
-        },
-        {
-          data: attachment2,
-          filename: 'test-file2.txt',
-        },
-      ],
-    });
+    const eventId = captureFeedback(
+      {
+        message: 'test',
+      },
+      {
+        attachments: [
+          {
+            data: attachment1,
+            filename: 'test-file.txt',
+          },
+          {
+            data: attachment2,
+            filename: 'test-file2.txt',
+          },
+        ],
+      },
+    );
 
     await client.flush();
 
     expect(typeof eventId).toBe('string');
 
-    expect(mockTransport).toHaveBeenCalledTimes(2);
+    expect(mockTransport).toHaveBeenCalledTimes(1);
 
-    const [feedbackEnvelope, attachmentEnvelope] = mockTransport.mock.calls;
+    const [feedbackEnvelope] = mockTransport.mock.calls;
 
-    // Feedback event is sent normally in one envelope
+    expect(feedbackEnvelope).toHaveLength(1);
     expect(feedbackEnvelope[0]).toEqual([
       {
         event_id: eventId,
@@ -206,16 +210,6 @@ describe('captureFeedback', () => {
             type: 'feedback',
           },
         ],
-      ],
-    ]);
-
-    // Attachments are sent in separate envelope
-    expect(attachmentEnvelope[0]).toEqual([
-      {
-        event_id: eventId,
-        sent_at: expect.any(String),
-      },
-      [
         [
           {
             type: 'attachment',
@@ -359,12 +353,6 @@ describe('captureFeedback', () => {
               trace: {
                 trace_id: traceId,
                 span_id: spanId,
-                data: {
-                  'sentry.origin': 'manual',
-                  'sentry.sample_rate': 1,
-                  'sentry.source': 'custom',
-                },
-                origin: 'manual',
               },
               feedback: {
                 message: 'test',

packages/feedback/src/core/sendFeedback.test.ts

@@ -174,12 +174,6 @@ describe('sendFeedback', () => {
               trace: {
                 span_id: expect.any(String),
                 trace_id: expect.any(String),
-                data: {
-                  'sentry.origin': 'manual',
-                  'sentry.sample_rate': 1,
-                  'sentry.source': 'custom',
-                },
-                origin: 'manual',
               },
               feedback: {
                 contact_email: '[email protected]',
@@ -344,32 +338,35 @@ describe('sendFeedback', () => {
     const attachment1 = new Uint8Array([1, 2, 3, 4, 5]);
     const attachment2 = new Uint8Array([6, 7, 8, 9]);
 
-    const promise = sendFeedback({
-      name: 'doe',
-      email: '[email protected]',
-      message: 'mi',
-      attachments: [
-        {
-          data: attachment1,
-          filename: 'test-file.txt',
-        },
-        {
-          data: attachment2,
-          filename: 'test-file2.txt',
-        },
-      ],
-    });
+    const promise = sendFeedback(
+      {
+        name: 'doe',
+        email: '[email protected]',
+        message: 'mi',
+      },
+      {
+        attachments: [
+          {
+            data: attachment1,
+            filename: 'test-file.txt',
+          },
+          {
+            data: attachment2,
+            filename: 'test-file2.txt',
+          },
+        ],
+      },
+    );
 
     expect(promise).toBeInstanceOf(Promise);
 
     const eventId = await promise;
 
     expect(typeof eventId).toEqual('string');
-    expect(mockTransport).toHaveBeenCalledTimes(2);
+    expect(mockTransport).toHaveBeenCalledTimes(1);
 
-    const [feedbackEnvelope, attachmentEnvelope] = mockTransport.mock.calls;
+    const [feedbackEnvelope] = mockTransport.mock.calls;
 
-    // Feedback event is sent normally in one envelope
     expect(feedbackEnvelope[0]).toEqual([
       {
         event_id: eventId,
@@ -405,16 +402,6 @@ describe('sendFeedback', () => {
             type: 'feedback',
           },
         ],
-      ],
-    ]);
-
-    // Attachments are sent in separate envelope
-    expect(attachmentEnvelope[0]).toEqual([
-      {
-        event_id: eventId,
-        sent_at: expect.any(String),
-      },
-      [
         [
           {
             type: 'attachment',

packages/feedback/src/core/sendFeedback.ts

@@ -1,6 +1,6 @@
 import { captureFeedback } from '@sentry/core';
 import { getClient } from '@sentry/core';
-import type { SendFeedback, SendFeedbackParams, TransportMakeRequestResponse } from '@sentry/types';
+import type { EventHint, SendFeedback, SendFeedbackParams, TransportMakeRequestResponse } from '@sentry/types';
 import type { Event } from '@sentry/types';
 import { getLocationHref } from '@sentry/utils';
 import { FEEDBACK_API_SOURCE } from '../constants';
@@ -10,7 +10,7 @@ import { FEEDBACK_API_SOURCE } from '../constants';
  */
 export const sendFeedback: SendFeedback = (
   options: SendFeedbackParams,
-  { includeReplay = true } = {},
+  hint: EventHint & { includeReplay?: boolean } = { includeReplay: true },
 ): Promise<string> => {
   if (!options.message) {
     throw new Error('Unable to submit feedback with empty message');
@@ -29,7 +29,7 @@ export const sendFeedback: SendFeedback = (
       url: getLocationHref(),
       ...options,
     },
-    { includeReplay },
+    hint,
   );
 
   // We want to wait for the feedback to be sent (or not)

packages/feedback/src/modal/components/Form.tsx

@@ -112,17 +112,28 @@ export function Form({
         }
         const formData = new FormData(e.target);
         const attachment = await (screenshotInput && showScreenshotInput ? screenshotInput.value() : undefined);
+
         const data: FeedbackFormData = {
           name: retrieveStringValue(formData, 'name'),
           email: retrieveStringValue(formData, 'email'),
           message: retrieveStringValue(formData, 'message'),
           attachments: attachment ? [attachment] : undefined,
         };
+
         if (!hasAllRequiredFields(data)) {
           return;
         }
+
         try {
-          await onSubmit({ ...data, source: FEEDBACK_WIDGET_SOURCE });
+          await onSubmit(
+            {
+              name: data.name,
+              email: data.email,
+              message: data.message,
+              source: FEEDBACK_WIDGET_SOURCE,
+            },
+            { attachments: data.attachments },
+          );
           onSubmitSuccess(data);
         } catch (error) {
           DEBUG_BUILD && logger.error(error);

packages/types/src/feedback/sendFeedback.ts

@@ -1,5 +1,4 @@
-import type { Attachment } from '../attachment';
-import type { Event } from '../event';
+import type { Event, EventHint } from '../event';
 import type { User } from '../user';
 
 /**
@@ -36,13 +35,12 @@ export interface SendFeedbackParams {
   message: string;
   name?: string;
   email?: string;
-  attachments?: Attachment[];
   url?: string;
   source?: string;
   associatedEventId?: string;
 }
 
-interface SendFeedbackOptions {
+interface SendFeedbackOptions extends EventHint {
   /**
    * Should include replay with the feedback?
    */