ref: Remove unneeded logic from transports (#5002)

Author: lforst

packages/browser/src/exports.ts

@@ -5,9 +5,7 @@ export type {
   SdkInfo,
   Event,
   EventHint,
-  EventStatus,
   Exception,
-  Response,
   // eslint-disable-next-line deprecation/deprecation
   Severity,
   SeverityLevel,

packages/browser/src/transports/fetch.ts

@@ -22,17 +22,12 @@ export function makeFetchTransport(
       ...options.requestOptions,
     };
 
-    return nativeFetch(options.url, requestOptions).then(response => {
-      return response.text().then(body => ({
-        body,
-        headers: {
-          'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
-          'retry-after': response.headers.get('Retry-After'),
-        },
-        reason: response.statusText,
-        statusCode: response.status,
-      }));
-    });
+    return nativeFetch(options.url, requestOptions).then(response => ({
+      headers: {
+        'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
+        'retry-after': response.headers.get('Retry-After'),
+      },
+    }));
   }
 
   return createTransport({ bufferSize: options.bufferSize }, makeRequest);

packages/browser/src/transports/xhr.ts

@@ -26,16 +26,12 @@ export function makeXHRTransport(options: XHRTransportOptions): Transport {
 
       xhr.onreadystatechange = (): void => {
         if (xhr.readyState === XHR_READYSTATE_DONE) {
-          const response = {
-            body: xhr.response,
+          resolve({
             headers: {
               'x-sentry-rate-limits': xhr.getResponseHeader('X-Sentry-Rate-Limits'),
               'retry-after': xhr.getResponseHeader('Retry-After'),
             },
-            reason: xhr.statusText,
-            statusCode: xhr.status,
-          };
-          resolve(response);
+          });
         }
       };
 

packages/browser/test/unit/helper/browser-client-options.ts

@@ -6,7 +6,7 @@ import { BrowserClientOptions } from '../../../src/client';
 export function getDefaultBrowserClientOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
   return {
     integrations: [],
-    transport: () => createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 })),
+    transport: () => createTransport({}, _ => resolvedSyncPromise({})),
     stackParser: () => [],
     ...options,
   };

packages/browser/test/unit/mocks/simpletransport.ts

@@ -2,5 +2,5 @@ import { createTransport } from '@sentry/core';
 import { resolvedSyncPromise } from '@sentry/utils';
 
 export function makeSimpleTransport() {
-  return createTransport({}, () => resolvedSyncPromise({ statusCode: 200 }));
+  return createTransport({}, () => resolvedSyncPromise({}));
 }

packages/browser/test/unit/sdk.test.ts

@@ -15,7 +15,7 @@ const PUBLIC_DSN = 'https://username@domain/123';
 function getDefaultBrowserOptions(options: Partial<BrowserOptions> = {}): BrowserOptions {
   return {
     integrations: [],
-    transport: () => createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 })),
+    transport: () => createTransport({}, _ => resolvedSyncPromise({})),
     stackParser: () => [],
     ...options,
   };

packages/browser/test/unit/transports/fetch.test.ts

@@ -34,11 +34,9 @@ describe('NewFetchTransport', () => {
     const transport = makeFetchTransport(DEFAULT_FETCH_TRANSPORT_OPTIONS, mockFetch);
 
     expect(mockFetch).toHaveBeenCalledTimes(0);
-    const res = await transport.send(ERROR_ENVELOPE);
+    await transport.send(ERROR_ENVELOPE);
     expect(mockFetch).toHaveBeenCalledTimes(1);
 
-    expect(res.status).toBe('success');
-
     expect(mockFetch).toHaveBeenLastCalledWith(DEFAULT_FETCH_TRANSPORT_OPTIONS.url, {
       body: serializeEnvelope(ERROR_ENVELOPE),
       method: 'POST',

packages/browser/test/unit/transports/xhr.test.ts

@@ -65,18 +65,6 @@ describe('NewXHRTransport', () => {
     expect(xhrMock.send).toHaveBeenCalledWith(serializeEnvelope(ERROR_ENVELOPE));
   });
 
-  it('returns the correct response', async () => {
-    const transport = makeXHRTransport(DEFAULT_XHR_TRANSPORT_OPTIONS);
-
-    const [res] = await Promise.all([
-      transport.send(ERROR_ENVELOPE),
-      (xhrMock as XMLHttpRequest).onreadystatechange!({} as Event),
-    ]);
-
-    expect(res).toBeDefined();
-    expect(res.status).toEqual('success');
-  });
-
   it('sets rate limit response headers', async () => {
     const transport = makeXHRTransport(DEFAULT_XHR_TRANSPORT_OPTIONS);
 

packages/core/src/transports/base.ts

@@ -3,19 +3,14 @@ import {
   InternalBaseTransportOptions,
   Transport,
   TransportCategory,
-  TransportRequest,
   TransportRequestExecutor,
-  TransportResponse,
 } from '@sentry/types';
 import {
-  disabledUntil,
-  eventStatusFromHttpCode,
   getEnvelopeType,
   isRateLimited,
   makePromiseBuffer,
   PromiseBuffer,
   RateLimits,
-  rejectedSyncPromise,
   resolvedSyncPromise,
   serializeEnvelope,
   updateRateLimits,
@@ -32,44 +27,26 @@ export const DEFAULT_TRANSPORT_BUFFER_SIZE = 30;
 export function createTransport(
   options: InternalBaseTransportOptions,
   makeRequest: TransportRequestExecutor,
-  buffer: PromiseBuffer<TransportResponse> = makePromiseBuffer(options.bufferSize || DEFAULT_TRANSPORT_BUFFER_SIZE),
+  buffer: PromiseBuffer<void> = makePromiseBuffer(options.bufferSize || DEFAULT_TRANSPORT_BUFFER_SIZE),
 ): Transport {
   let rateLimits: RateLimits = {};
 
   const flush = (timeout?: number): PromiseLike<boolean> => buffer.drain(timeout);
 
-  function send(envelope: Envelope): PromiseLike<TransportResponse> {
+  function send(envelope: Envelope): PromiseLike<void> {
     const envCategory = getEnvelopeType(envelope);
     const category = envCategory === 'event' ? 'error' : (envCategory as TransportCategory);
-    const request: TransportRequest = {
-      category,
-      body: serializeEnvelope(envelope),
-    };
 
     // Don't add to buffer if transport is already rate-limited
     if (isRateLimited(rateLimits, category)) {
-      return rejectedSyncPromise({
-        status: 'rate_limit',
-        reason: getRateLimitReason(rateLimits, category),
-      });
+      return resolvedSyncPromise();
     }
 
-    const requestTask = (): PromiseLike<TransportResponse> =>
-      makeRequest(request).then(({ body, headers, reason, statusCode }): PromiseLike<TransportResponse> => {
-        const status = eventStatusFromHttpCode(statusCode);
+    const requestTask = (): PromiseLike<void> =>
+      makeRequest({ body: serializeEnvelope(envelope) }).then(({ headers }): void => {
         if (headers) {
           rateLimits = updateRateLimits(rateLimits, headers);
         }
-        if (status === 'success') {
-          return resolvedSyncPromise({ status, reason });
-        }
-        return rejectedSyncPromise({
-          status,
-          reason:
-            reason ||
-            body ||
-            (status === 'rate_limit' ? getRateLimitReason(rateLimits, category) : 'Unknown transport error'),
-        });
       });
 
     return buffer.add(requestTask);
@@ -80,9 +57,3 @@ export function createTransport(
     flush,
   };
 }
-
-function getRateLimitReason(rateLimits: RateLimits, category: TransportCategory): string {
-  return `Too many ${category} requests, backing off until: ${new Date(
-    disabledUntil(rateLimits, category),
-  ).toISOString()}`;
-}

packages/core/test/lib/transports/base.test.ts

@@ -1,12 +1,8 @@
-import { EventEnvelope, EventItem, Transport, TransportMakeRequestResponse, TransportResponse } from '@sentry/types';
+import { EventEnvelope, EventItem, Transport, TransportMakeRequestResponse } from '@sentry/types';
 import { createEnvelope, PromiseBuffer, resolvedSyncPromise, serializeEnvelope } from '@sentry/utils';
 
 import { createTransport } from '../../../src/transports/base';
 
-const ERROR_TRANSPORT_CATEGORY = 'error';
-
-const TRANSACTION_TRANSPORT_CATEGORY = 'transaction';
-
 const ERROR_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
   [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
 ]);
@@ -18,12 +14,12 @@ const TRANSACTION_ENVELOPE = createEnvelope<EventEnvelope>(
 
 describe('createTransport', () => {
   it('flushes the buffer', async () => {
-    const mockBuffer: PromiseBuffer<TransportResponse> = {
+    const mockBuffer: PromiseBuffer<void> = {
       $: [],
       add: jest.fn(),
       drain: jest.fn(),
     };
-    const transport = createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 }), mockBuffer);
+    const transport = createTransport({}, _ => resolvedSyncPromise({}), mockBuffer);
     /* eslint-disable @typescript-eslint/unbound-method */
     expect(mockBuffer.drain).toHaveBeenCalledTimes(0);
     await transport.flush(1000);
@@ -34,45 +30,29 @@ describe('createTransport', () => {
 
   describe('send', () => {
     it('constructs a request to send to Sentry', async () => {
+      expect.assertions(1);
       const transport = createTransport({}, req => {
-        expect(req.category).toEqual(ERROR_TRANSPORT_CATEGORY);
         expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE));
-        return resolvedSyncPromise({ statusCode: 200, reason: 'OK' });
+        return resolvedSyncPromise({});
       });
-      const res = await transport.send(ERROR_ENVELOPE);
-      expect(res.status).toBe('success');
-      expect(res.reason).toBe('OK');
+      await transport.send(ERROR_ENVELOPE);
     });
 
-    it('returns an error if request failed', async () => {
+    it('does throw if request fails', async () => {
+      expect.assertions(2);
+
       const transport = createTransport({}, req => {
-        expect(req.category).toEqual(ERROR_TRANSPORT_CATEGORY);
         expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE));
-        return resolvedSyncPromise({ statusCode: 400, reason: 'Bad Request' });
+        throw new Error();
       });
-      try {
-        await transport.send(ERROR_ENVELOPE);
-      } catch (res) {
-        expect(res.status).toBe('invalid');
-        expect(res.reason).toBe('Bad Request');
-      }
-    });
 
-    it('returns a default reason if reason not provided and request failed', async () => {
-      const transport = createTransport({}, req => {
-        expect(req.category).toEqual(TRANSACTION_TRANSPORT_CATEGORY);
-        expect(req.body).toEqual(serializeEnvelope(TRANSACTION_ENVELOPE));
-        return resolvedSyncPromise({ statusCode: 500 });
-      });
-      try {
-        await transport.send(TRANSACTION_ENVELOPE);
-      } catch (res) {
-        expect(res.status).toBe('failed');
-        expect(res.reason).toBe('Unknown transport error');
-      }
+      expect(() => {
+        void transport.send(ERROR_ENVELOPE);
+      }).toThrow();
     });
 
-    describe('Rate-limiting', () => {
+    // TODO(v7): Add tests back in and test by using client report logic
+    describe.skip('Rate-limiting', () => {
       function setRateLimitTimes(): {
         retryAfterSeconds: number;
         beforeLimit: number;
@@ -123,7 +103,6 @@ describe('createTransport', () => {
             'x-sentry-rate-limits': null,
             'retry-after': `${retryAfterSeconds}`,
           },
-          statusCode: 429,
         });
 
         try {
@@ -133,7 +112,7 @@ describe('createTransport', () => {
           expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`);
         }
 
-        setTransportResponse({ statusCode: 200 });
+        setTransportResponse({});
 
         try {
           await transport.send(ERROR_ENVELOPE);
@@ -142,8 +121,7 @@ describe('createTransport', () => {
           expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`);
         }
 
-        const res = await transport.send(ERROR_ENVELOPE);
-        expect(res.status).toBe('success');
+        await transport.send(ERROR_ENVELOPE);
       });
 
       it('back-off using X-Sentry-Rate-Limits with single category', async () => {
@@ -169,7 +147,6 @@ describe('createTransport', () => {
             'x-sentry-rate-limits': `${retryAfterSeconds}:error:scope`,
             'retry-after': null,
           },
-          statusCode: 429,
         });
 
         try {
@@ -179,7 +156,7 @@ describe('createTransport', () => {
           expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`);
         }
 
-        setTransportResponse({ statusCode: 200 });
+        setTransportResponse({});
 
         try {
           await transport.send(TRANSACTION_ENVELOPE);
@@ -195,8 +172,7 @@ describe('createTransport', () => {
           expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`);
         }
 
-        const res = await transport.send(TRANSACTION_ENVELOPE);
-        expect(res.status).toBe('success');
+        await transport.send(TRANSACTION_ENVELOPE);
       });
 
       it('back-off using X-Sentry-Rate-Limits with multiple categories', async () => {
@@ -222,7 +198,6 @@ describe('createTransport', () => {
             'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`,
             'retry-after': null,
           },
-          statusCode: 429,
         });
 
         try {
@@ -248,13 +223,10 @@ describe('createTransport', () => {
           );
         }
 
-        setTransportResponse({ statusCode: 200 });
+        setTransportResponse({});
 
-        const eventRes = await transport.send(ERROR_ENVELOPE);
-        expect(eventRes.status).toBe('success');
-
-        const transactionRes = await transport.send(TRANSACTION_ENVELOPE);
-        expect(transactionRes.status).toBe('success');
+        await transport.send(ERROR_ENVELOPE);
+        await transport.send(TRANSACTION_ENVELOPE);
       });
 
       it('back-off using X-Sentry-Rate-Limits with missing categories should lock them all', async () => {
@@ -284,7 +256,6 @@ describe('createTransport', () => {
             'x-sentry-rate-limits': `${retryAfterSeconds}::scope`,
             'retry-after': null,
           },
-          statusCode: 429,
         });
 
         try {
@@ -310,13 +281,10 @@ describe('createTransport', () => {
           );
         }
 
-        setTransportResponse({ statusCode: 200 });
-
-        const eventRes = await transport.send(ERROR_ENVELOPE);
-        expect(eventRes.status).toBe('success');
+        setTransportResponse({});
 
-        const transactionRes = await transport.send(TRANSACTION_ENVELOPE);
-        expect(transactionRes.status).toBe('success');
+        await transport.send(ERROR_ENVELOPE);
+        await transport.send(TRANSACTION_ENVELOPE);
       });
 
       it('back-off using X-Sentry-Rate-Limits should also trigger for 200 responses', async () => {
@@ -340,7 +308,6 @@ describe('createTransport', () => {
             'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`,
             'retry-after': null,
           },
-          statusCode: 200,
         });
 
         try {

packages/core/test/mocks/client.ts

@@ -13,7 +13,7 @@ import { createTransport } from '../../src/transports/base';
 export function getDefaultTestClientOptions(options: Partial<TestClientOptions> = {}): TestClientOptions {
   return {
     integrations: [],
-    transport: () => createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 })),
+    transport: () => createTransport({}, _ => resolvedSyncPromise({})),
     stackParser: () => [],
     ...options,
   };