Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions packages/browser/test/profiling/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand All @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions packages/bun/src/transports/index.ts
Original file line number Diff line number Diff line change
@@ -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 */
Expand Down Expand Up @@ -30,7 +30,7 @@ export function makeFetchTransport(options: BunTransportOptions): Transport {
});
});
} catch (e) {
return rejectedSyncPromise(e);
return Promise.reject(e);
}
}

Expand Down
74 changes: 38 additions & 36 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand Down Expand Up @@ -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> {
Comment on lines +319 to +320
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for a moment, I thought the async signature change would still be breaking for anyone currently extending Client and overriding this method (as in, them not having the async declaration). But it seems like this is fine (at least with the TS settings in our repo) and TS is smart enough to handle it. Just wanted to leave this thought here in case we can think of a TS setting or a scenario where this might be problematic.

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;
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -872,18 +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 => {
try {
return await this._transport.send(envelope);
} catch (reason) {
DEBUG_BUILD && debug.error('Error while sending envelope:', reason);
return {};
});
}
}

DEBUG_BUILD && debug.error('Transport disabled');
return resolvedSyncPromise({});
return {};
}

/* eslint-enable @typescript-eslint/unified-signatures */
Expand Down Expand Up @@ -938,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. */
Expand Down
5 changes: 2 additions & 3 deletions packages/core/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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;
}
Expand Down
18 changes: 3 additions & 15 deletions packages/core/test/lib/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -2260,11 +2252,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);
Expand Down
2 changes: 0 additions & 2 deletions packages/deno/src/integrations/deno-cron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/deno/src/transports/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/core';
import { consoleSandbox, createTransport, debug, rejectedSyncPromise, suppressTracing } from '@sentry/core';
import { consoleSandbox, createTransport, debug, suppressTracing } from '@sentry/core';

export interface DenoTransportOptions extends BaseTransportOptions {
/** Custom headers for the transport. Used by the XHRTransport and FetchTransport */
Expand Down Expand Up @@ -48,7 +48,7 @@ export function makeFetchTransport(options: DenoTransportOptions): Transport {
});
});
} catch (e) {
return rejectedSyncPromise(e);
return Promise.reject(e);
}
}

Expand Down
23 changes: 12 additions & 11 deletions packages/node-core/src/sdk/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
return tracer;
}

// Eslint ignore explanation: This is already documented in super.
// eslint-disable-next-line jsdoc/require-jsdoc
public async flush(timeout?: number): Promise<boolean> {
/** @inheritDoc */
// @ts-expect-error - PromiseLike is a subset of Promise
public async flush(timeout?: number): PromiseLike<boolean> {
await this.traceProvider?.forceFlush();

if (this.getOptions().sendClientReports) {
Expand All @@ -89,9 +89,9 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
return super.flush(timeout);
}

// Eslint ignore explanation: This is already documented in super.
// eslint-disable-next-line jsdoc/require-jsdoc
public close(timeout?: number | undefined): PromiseLike<boolean> {
/** @inheritDoc */
// @ts-expect-error - PromiseLike is a subset of Promise
public async close(timeout?: number | undefined): PromiseLike<boolean> {
if (this._clientReportInterval) {
clearInterval(this._clientReportInterval);
}
Expand All @@ -104,11 +104,12 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
process.off('beforeExit', this._logOnExitFlushListener);
}

return super
.close(timeout)
.then(allEventsSent =>
this.traceProvider ? this.traceProvider.shutdown().then(() => allEventsSent) : allEventsSent,
);
const allEventsSent = await super.close(timeout);
if (this.traceProvider) {
await this.traceProvider.shutdown();
}

return allEventsSent;
}

/**
Expand Down
4 changes: 1 addition & 3 deletions packages/node-core/test/sdk/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,7 @@ describe('NodeClient', () => {

expect(result).toBe(true);

// once call directly in close to stop client reports,
// the other in core client `_isClientDoneProcessing`
expect(clearIntervalSpy).toHaveBeenCalledTimes(2);
expect(clearIntervalSpy).toHaveBeenCalledTimes(1);

// removes `_clientReportOnExitFlushListener`
expect(processOffSpy).toHaveBeenNthCalledWith(1, 'beforeExit', expect.any(Function));
Expand Down
6 changes: 3 additions & 3 deletions packages/replay-internal/src/util/sendReplayRequest.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { RateLimits, ReplayEvent, TransportMakeRequestResponse } from '@sentry/core';
import { getClient, getCurrentScope, isRateLimited, resolvedSyncPromise, updateRateLimits } from '@sentry/core';
import { getClient, getCurrentScope, isRateLimited, updateRateLimits } from '@sentry/core';
import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants';
import { DEBUG_BUILD } from '../debug-build';
import type { SendReplayData } from '../types';
Expand Down Expand Up @@ -34,7 +34,7 @@ export async function sendReplayRequest({
const dsn = client?.getDsn();

if (!client || !transport || !dsn || !session.sampled) {
return resolvedSyncPromise({});
return Promise.resolve({});
}

const baseEvent: ReplayEvent = {
Expand All @@ -55,7 +55,7 @@ export async function sendReplayRequest({
// Taken from baseclient's `_processEvent` method, where this is handled for errors/transactions
client.recordDroppedEvent('event_processor', 'replay');
DEBUG_BUILD && debug.log('An event processor returned `null`, will not send event.');
return resolvedSyncPromise({});
return Promise.resolve({});
}

/*
Expand Down