Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref: Avoid using SentryError for PromiseBuffer control flow #15822

Merged
merged 2 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 2 additions & 2 deletions packages/cloudflare/src/transport.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 { SentryError, createTransport, suppressTracing } from '@sentry/core';
import { SENTRY_BUFFER_FULL_ERROR, createTransport, suppressTracing } from '@sentry/core';

export interface CloudflareTransportOptions extends BaseTransportOptions {
/** Fetch API init parameters. */
Expand Down Expand Up @@ -38,7 +38,7 @@ export class IsolatedPromiseBuffer {
*/
public add(taskProducer: () => PromiseLike<TransportMakeRequestResponse>): PromiseLike<TransportMakeRequestResponse> {
if (this._taskProducers.length >= this._bufferSize) {
return Promise.reject(new SentryError('Not adding Promise because buffer limit was reached.'));
return Promise.reject(SENTRY_BUFFER_FULL_ERROR);
}

this._taskProducers.push(taskProducer);
Expand Down
9 changes: 7 additions & 2 deletions packages/cloudflare/test/transport.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createEnvelope, serializeEnvelope } from '@sentry/core';
import { SENTRY_BUFFER_FULL_ERROR, createEnvelope, serializeEnvelope } from '@sentry/core';
import type { EventEnvelope, EventItem } from '@sentry/core';
import { afterAll, describe, expect, it, vi } from 'vitest';

Expand Down Expand Up @@ -140,7 +140,12 @@ describe('IsolatedPromiseBuffer', () => {
await ipb.add(task2);
await ipb.add(task3);

await expect(ipb.add(task4)).rejects.toThrowError('Not adding Promise because buffer limit was reached.');
try {
await ipb.add(task4);
throw new Error('Should not be called');
} catch (error) {
expect(error).toBe(SENTRY_BUFFER_FULL_ERROR);
}
});

it('should not throw when one of the tasks throws when drained', async () => {
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 @@ -14,9 +14,8 @@ import {
forEachEnvelopeItem,
serializeEnvelope,
} from '../utils-hoist/envelope';
import { SentryError } from '../utils-hoist/error';
import { logger } from '../utils-hoist/logger';
import { type PromiseBuffer, makePromiseBuffer } from '../utils-hoist/promisebuffer';
import { type PromiseBuffer, makePromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from '../utils-hoist/promisebuffer';
import { type RateLimits, isRateLimited, updateRateLimits } from '../utils-hoist/ratelimit';
import { resolvedSyncPromise } from '../utils-hoist/syncpromise';

Expand Down Expand Up @@ -85,7 +84,7 @@ export function createTransport(
return buffer.add(requestTask).then(
result => result,
error => {
if (error instanceof SentryError) {
if (error === SENTRY_BUFFER_FULL_ERROR) {
DEBUG_BUILD && logger.error('Skipped sending event because buffer is full.');
recordEnvelopeLoss('queue_overflow');
return resolvedSyncPromise({});
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export {
objectify,
} from './object';
export { basename, dirname, isAbsolute, join, normalizePath, relative, resolve } from './path';
export { makePromiseBuffer } from './promisebuffer';
export { makePromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from './promisebuffer';
export type { PromiseBuffer } from './promisebuffer';

export { severityLevelFromString } from './severity';
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/utils-hoist/promisebuffer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { SentryError } from './error';
import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './syncpromise';

export interface PromiseBuffer<T> {
Expand All @@ -9,6 +8,8 @@ export interface PromiseBuffer<T> {
drain(timeout?: number): PromiseLike<boolean>;
}

export const SENTRY_BUFFER_FULL_ERROR = Symbol.for('SentryBufferFullError');

/**
* Creates an new PromiseBuffer object with the specified limit
* @param limit max number of promises that can be stored in the buffer
Expand Down Expand Up @@ -42,7 +43,7 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
*/
function add(taskProducer: () => PromiseLike<T>): PromiseLike<T> {
if (!isReady()) {
return rejectedSyncPromise(new SentryError('Not adding Promise because buffer limit was reached.'));
return rejectedSyncPromise(SENTRY_BUFFER_FULL_ERROR);
}

// start the task and add its promise to the queue
Expand Down
4 changes: 2 additions & 2 deletions packages/vercel-edge/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 { SentryError, createTransport, suppressTracing } from '@sentry/core';
import { SENTRY_BUFFER_FULL_ERROR, createTransport, suppressTracing } from '@sentry/core';

export interface VercelEdgeTransportOptions extends BaseTransportOptions {
/** Fetch API init parameters. */
Expand Down Expand Up @@ -38,7 +38,7 @@ export class IsolatedPromiseBuffer {
*/
public add(taskProducer: () => PromiseLike<TransportMakeRequestResponse>): PromiseLike<TransportMakeRequestResponse> {
if (this._taskProducers.length >= this._bufferSize) {
return Promise.reject(new SentryError('Not adding Promise because buffer limit was reached.'));
return Promise.reject(SENTRY_BUFFER_FULL_ERROR);
}

this._taskProducers.push(taskProducer);
Expand Down
9 changes: 7 additions & 2 deletions packages/vercel-edge/test/transports/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterAll, describe, expect, it, vi } from 'vitest';

import { createEnvelope, serializeEnvelope } from '@sentry/core';
import { SENTRY_BUFFER_FULL_ERROR, createEnvelope, serializeEnvelope } from '@sentry/core';
import type { EventEnvelope, EventItem } from '@sentry/core';

import type { VercelEdgeTransportOptions } from '../../src/transports';
Expand Down Expand Up @@ -139,7 +139,12 @@ describe('IsolatedPromiseBuffer', () => {
await ipb.add(task2);
await ipb.add(task3);

await expect(ipb.add(task4)).rejects.toThrowError('Not adding Promise because buffer limit was reached.');
try {
await ipb.add(task4);
throw new Error('Should not be called');
} catch (error) {
expect(error).toBe(SENTRY_BUFFER_FULL_ERROR);
}
});

it('should not throw when one of the tasks throws when drained', async () => {
Expand Down
Loading