ref: Avoid some usage of SyncPromise where not needed

We make quite heavy use of it in event processing, not touching this for now, but in other places it should not be needed IMHO.

fix transport using async

refactor some more sync promise stuff

fix node client

fix some flushing stuff

fix check oops

small fixees

fix tests

fix lint

fix to promise like

small ref

avoid special casing of no fetch implementation

fetch stuff

Author: mydea

packages/browser/src/transports/fetch.ts

@@ -1,5 +1,5 @@
 import type { Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/core';
-import { createTransport, rejectedSyncPromise } from '@sentry/core';
+import { createTransport } from '@sentry/core';
 import { clearCachedImplementation, getNativeImplementation } from '@sentry-internal/browser-utils';
 import type { WINDOW } from '../helpers';
 import type { BrowserTransportOptions } from './types';
@@ -9,12 +9,12 @@ import type { BrowserTransportOptions } from './types';
  */
 export function makeFetchTransport(
   options: BrowserTransportOptions,
-  nativeFetch: typeof WINDOW.fetch | undefined = getNativeImplementation('fetch'),
+  nativeFetch: typeof WINDOW.fetch = getNativeImplementation('fetch'),
 ): Transport {
   let pendingBodySize = 0;
   let pendingCount = 0;
 
-  function makeRequest(request: TransportRequest): PromiseLike<TransportMakeRequestResponse> {
+  async function makeRequest(request: TransportRequest): Promise<TransportMakeRequestResponse> {
     const requestSize = request.body.length;
     pendingBodySize += requestSize;
     pendingCount++;
@@ -39,29 +39,23 @@ export function makeFetchTransport(
       ...options.fetchOptions,
     };
 
-    if (!nativeFetch) {
-      clearCachedImplementation('fetch');
-      return rejectedSyncPromise('No fetch implementation available');
-    }
-
     try {
-      // Note: We do not need to suppress tracing here, becasue we are using the native fetch, instead of our wrapped one.
-      return nativeFetch(options.url, requestOptions).then(response => {
-        pendingBodySize -= requestSize;
-        pendingCount--;
-        return {
-          statusCode: response.status,
-          headers: {
-            'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
-            'retry-after': response.headers.get('Retry-After'),
-          },
-        };
-      });
+      // Note: We do not need to suppress tracing here, because we are using the native fetch, instead of our wrapped one.
+      const response = await nativeFetch(options.url, requestOptions);
+
+      return {
+        statusCode: response.status,
+        headers: {
+          'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
+          'retry-after': response.headers.get('Retry-After'),
+        },
+      };
     } catch (e) {
       clearCachedImplementation('fetch');
+      throw e;
+    } finally {
       pendingBodySize -= requestSize;
       pendingCount--;
-      return rejectedSyncPromise(e);
     }
   }
 

packages/browser/test/profiling/integration.test.ts

@@ -2,7 +2,6 @@
  * @vitest-environment jsdom
  */
 
-import type { BrowserClient } from '@sentry/browser';
 import * as Sentry from '@sentry/browser';
 import { describe, expect, it, vi } from 'vitest';
 import type { JSSelfProfile } from '../../src/profiling/jsSelfProfiling';
@@ -36,7 +35,7 @@ describe('BrowserProfilingIntegration', () => {
 
     const flush = vi.fn().mockImplementation(() => Promise.resolve(true));
     const send = vi.fn().mockImplementation(() => Promise.resolve());
-    Sentry.init({
+    const client = Sentry.init({
       tracesSampleRate: 1,
       profilesSampleRate: 1,
       environment: 'test-environment',
@@ -50,13 +49,11 @@ describe('BrowserProfilingIntegration', () => {
       integrations: [Sentry.browserTracingIntegration(), Sentry.browserProfilingIntegration()],
     });
 
-    const client = Sentry.getClient<BrowserClient>();
-
     const currentTransaction = Sentry.getActiveSpan();
     expect(currentTransaction).toBeDefined();
     expect(Sentry.spanToJSON(currentTransaction!).op).toBe('pageload');
     currentTransaction?.end();
-    await client?.flush(1000);
+    await client!.flush(1000);
 
     expect(send).toHaveBeenCalledTimes(1);
 

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

@@ -1,6 +1,6 @@
 import type { EventEnvelope, EventItem } from '@sentry/core';
 import { createEnvelope, serializeEnvelope } from '@sentry/core';
-import type { Mock } from 'vitest';
+import { Mock, afterEach } from 'vitest';
 import { describe, expect, it, vi } from 'vitest';
 import { makeFetchTransport } from '../../src/transports/fetch';
 import type { BrowserTransportOptions } from '../../src/transports/types';
@@ -29,7 +29,11 @@ class Headers {
   }
 }
 
-describe('NewFetchTransport', () => {
+describe('fetchTransport', () => {
+  afterEach(() => {
+    vi.resetAllMocks();
+  });
+
   it('calls fetch with the given URL', async () => {
     const mockFetch = vi.fn(() =>
       Promise.resolve({
@@ -102,15 +106,28 @@ describe('NewFetchTransport', () => {
     });
   });
 
-  it('handles when `getNativetypeof window.fetchementation` is undefined', async () => {
+  it('handles when native fetch implementation returns undefined', async () => {
     const mockFetch = vi.fn(() => undefined) as unknown as typeof window.fetch;
     const transport = makeFetchTransport(DEFAULT_FETCH_TRANSPORT_OPTIONS, mockFetch);
 
     expect(mockFetch).toHaveBeenCalledTimes(0);
-    await expect(() => transport.send(ERROR_ENVELOPE)).not.toThrow();
+    await expect(() => transport.send(ERROR_ENVELOPE)).rejects.toThrow(
+      "Cannot read properties of undefined (reading 'status')",
+    );
     expect(mockFetch).toHaveBeenCalledTimes(1);
   });
 
+  it('handles when native fetch implementation is undefined', async () => {
+    vi.mock('@sentry-internal/browser-utils', async importOriginal => ({
+      ...(await importOriginal()),
+      getNativeImplementation: () => undefined,
+    }));
+
+    const transport = makeFetchTransport(DEFAULT_FETCH_TRANSPORT_OPTIONS);
+
+    await expect(() => transport.send(ERROR_ENVELOPE)).rejects.toThrow('nativeFetch is not a function');
+  });
+
   it('correctly sets keepalive flag', async () => {
     const mockFetch = vi.fn(() =>
       Promise.resolve({

packages/bun/src/transports/index.ts

@@ -1,5 +1,5 @@
 import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/core';
-import { createTransport, rejectedSyncPromise, suppressTracing } from '@sentry/core';
+import { createTransport, suppressTracing } from '@sentry/core';
 
 export interface BunTransportOptions extends BaseTransportOptions {
   /** Custom headers for the transport. Used by the XHRTransport and FetchTransport */
@@ -30,7 +30,7 @@ export function makeFetchTransport(options: BunTransportOptions): Transport {
         });
       });
     } catch (e) {
-      return rejectedSyncPromise(e);
+      return Promise.reject(e);
     }
   }
 

packages/core/src/client.ts

@@ -44,7 +44,7 @@ import { parseSampleRate } from './utils/parseSampleRate';
 import { prepareEvent } from './utils/prepareEvent';
 import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
 import { getActiveSpan, showSpanDropWarning, spanToTraceContext } from './utils/spanUtils';
-import { rejectedSyncPromise, resolvedSyncPromise, SyncPromise } from './utils/syncpromise';
+import { rejectedSyncPromise } from './utils/syncpromise';
 import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
 
 const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
@@ -316,16 +316,19 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
    * @returns A promise that will resolve with `true` if all events are sent before the timeout, or `false` if there are
    * still events in the queue when the timeout is reached.
    */
-  public flush(timeout?: number): PromiseLike<boolean> {
+  // @ts-expect-error - PromiseLike is a subset of Promise
+  public async flush(timeout?: number): PromiseLike<boolean> {
     const transport = this._transport;
-    if (transport) {
-      this.emit('flush');
-      return this._isClientDoneProcessing(timeout).then(clientFinished => {
-        return transport.flush(timeout).then(transportFlushed => clientFinished && transportFlushed);
-      });
-    } else {
-      return resolvedSyncPromise(true);
+    if (!transport) {
+      return Promise.resolve(true);
     }
+
+    this.emit('flush');
+
+    const clientFinished = await this._isClientDoneProcessing(timeout);
+    const transportFlushed = await transport.flush(timeout);
+
+    return clientFinished && transportFlushed;
   }
 
   /**
@@ -336,12 +339,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
    * @returns {Promise<boolean>} A promise which resolves to `true` if the flush completes successfully before the timeout, or `false` if
    * it doesn't.
    */
-  public close(timeout?: number): PromiseLike<boolean> {
-    return this.flush(timeout).then(result => {
-      this.getOptions().enabled = false;
-      this.emit('close');
-      return result;
-    });
+  // @ts-expect-error - PromiseLike is a subset of Promise
+  public async close(timeout?: number): PromiseLike<boolean> {
+    const result = await this.flush(timeout);
+    this.getOptions().enabled = false;
+    this.emit('close');
+    return result;
   }
 
   /**
@@ -415,10 +418,9 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
       env = addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment));
     }
 
-    const promise = this.sendEnvelope(env);
-    if (promise) {
-      promise.then(sendResponse => this.emit('afterSendEvent', event, sendResponse), null);
-    }
+    // sendEnvelope should not throw
+    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    this.sendEnvelope(env).then(sendResponse => this.emit('afterSendEvent', event, sendResponse));
   }
 
   /**
@@ -873,19 +875,21 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   /**
    * Send an envelope to Sentry.
    */
-  public sendEnvelope(envelope: Envelope): PromiseLike<TransportMakeRequestResponse> {
+  // @ts-expect-error - PromiseLike is a subset of Promise
+  public async sendEnvelope(envelope: Envelope): PromiseLike<TransportMakeRequestResponse> {
     this.emit('beforeEnvelope', envelope);
 
-    if (this._isEnabled() && this._transport) {
-      return this._transport.send(envelope).then(null, reason => {
-        DEBUG_BUILD && debug.error('Error while sending envelope:', reason);
-        return reason;
-      });
+    if (!this._isEnabled() || !this._transport) {
+      DEBUG_BUILD && debug.error('Transport disabled');
+      return {};
     }
 
-    DEBUG_BUILD && debug.error('Transport disabled');
-
-    return resolvedSyncPromise({});
+    try {
+      return await this._transport.send(envelope);
+    } catch (reason) {
+      DEBUG_BUILD && debug.error('Error while sending envelope:', reason);
+      return {};
+    }
   }
 
   /* eslint-enable @typescript-eslint/unified-signatures */
@@ -940,24 +944,20 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
    * @returns A promise which will resolve to `true` if processing is already done or finishes before the timeout, and
    * `false` otherwise
    */
-  protected _isClientDoneProcessing(timeout?: number): PromiseLike<boolean> {
-    return new SyncPromise(resolve => {
-      let ticked: number = 0;
-      const tick: number = 1;
+  protected async _isClientDoneProcessing(timeout?: number): Promise<boolean> {
+    let ticked = 0;
 
-      const interval = setInterval(() => {
-        if (this._numProcessing == 0) {
-          clearInterval(interval);
-          resolve(true);
-        } else {
-          ticked += tick;
-          if (timeout && ticked >= timeout) {
-            clearInterval(interval);
-            resolve(false);
-          }
-        }
-      }, tick);
-    });
+    // if no timeout is provided, we wait "forever" until everything is processed
+    while (!timeout || ticked < timeout) {
+      await new Promise(resolve => setTimeout(resolve, 1));
+
+      if (!this._numProcessing) {
+        return true;
+      }
+      ticked++;
+    }
+
+    return false;
   }
 
   /** Determines whether this SDK is enabled and a transport is present. */

packages/core/src/transports/base.ts

@@ -16,7 +16,6 @@ import {
 } from '../utils/envelope';
 import { type PromiseBuffer, makePromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from '../utils/promisebuffer';
 import { type RateLimits, isRateLimited, updateRateLimits } from '../utils/ratelimit';
-import { resolvedSyncPromise } from '../utils/syncpromise';
 
 export const DEFAULT_TRANSPORT_BUFFER_SIZE = 64;
 
@@ -51,7 +50,7 @@ export function createTransport(
 
     // Skip sending if envelope is empty after filtering out rate limited events
     if (filteredEnvelopeItems.length === 0) {
-      return resolvedSyncPromise({});
+      return Promise.resolve({});
     }
 
     const filteredEnvelope: Envelope = createEnvelope(envelope[0], filteredEnvelopeItems as (typeof envelope)[1]);
@@ -87,7 +86,7 @@ export function createTransport(
         if (error === SENTRY_BUFFER_FULL_ERROR) {
           DEBUG_BUILD && debug.error('Skipped sending event because buffer is full.');
           recordEnvelopeLoss('queue_overflow');
-          return resolvedSyncPromise({});
+          return Promise.resolve({});
         } else {
           throw error;
         }

packages/core/test/lib/client.test.ts

@@ -2200,11 +2200,7 @@ describe('Client', () => {
       client.on('afterSendEvent', callback);
 
       client.sendEvent(errorEvent);
-      vi.runAllTimers();
-      // Wait for two ticks
-      // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang
-      await undefined;
-      await undefined;
+      await vi.runAllTimersAsync();
 
       expect(mockSend).toBeCalledTimes(1);
       expect(callback).toBeCalledTimes(1);
@@ -2228,11 +2224,7 @@ describe('Client', () => {
       client.on('afterSendEvent', callback);
 
       client.sendEvent(transactionEvent);
-      vi.runAllTimers();
-      // Wait for two ticks
-      // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang
-      await undefined;
-      await undefined;
+      await vi.runAllTimersAsync();
 
       expect(mockSend).toBeCalledTimes(1);
       expect(callback).toBeCalledTimes(1);
@@ -2260,15 +2252,11 @@ describe('Client', () => {
       client.on('afterSendEvent', callback);
 
       client.sendEvent(errorEvent);
-      vi.runAllTimers();
-      // Wait for two ticks
-      // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang
-      await undefined;
-      await undefined;
+      await vi.runAllTimersAsync();
 
       expect(mockSend).toBeCalledTimes(1);
       expect(callback).toBeCalledTimes(1);
-      expect(callback).toBeCalledWith(errorEvent, 'send error');
+      expect(callback).toBeCalledWith(errorEvent, {});
     });
 
     it('passes the response to the hook', async () => {

packages/deno/src/integrations/deno-cron.ts

@@ -15,13 +15,11 @@ const _denoCronIntegration = (() => {
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      // eslint-disable-next-line deprecation/deprecation
       if (!Deno.cron) {
         // The cron API is not available in this Deno version use --unstable flag!
         return;
       }
 
-      // eslint-disable-next-line deprecation/deprecation
       Deno.cron = new Proxy(Deno.cron, {
         apply(target, thisArg, argArray: CronParams) {
           const [monitorSlug, schedule, opt1, opt2] = argArray;