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: Leave only valid buffer implementation #3744

Merged
merged 3 commits into from
Jun 25, 2021
Merged
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
17 changes: 3 additions & 14 deletions packages/utils/src/promisebuffer.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { SentryError } from './error';
import { isThenable } from './is';
import { SyncPromise } from './syncpromise';

type TaskProducer<T> = () => PromiseLike<T>;

/** A simple queue that holds promises. */
export class PromiseBuffer<T> {
/** Internal set of queued Promises */
Expand All @@ -21,22 +18,14 @@ export class PromiseBuffer<T> {
/**
* Add a promise to the queue.
*
* @param taskProducer A function producing any PromiseLike<T>; In previous versions this used to be `@param task: PromiseLike<T>`, however, Promises were instantly created on the call-site, making them fall through the buffer limit.
Copy link
Member

Choose a reason for hiding this comment

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

I would still keep this comment, even if we are losing the backward compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored

* @param taskProducer A function producing any PromiseLike<T>.
* @returns The original promise.
*/
public add(taskProducer: PromiseLike<T> | TaskProducer<T>): PromiseLike<T> {
// NOTE: This is necessary to preserve backwards compatibility
// It should accept _only_ `TaskProducer<T>` but we dont want to break other custom transports
// that are utilizing our `Buffer` implementation.
// see: https://github.com/getsentry/sentry-javascript/issues/3725
const normalizedTaskProducer: TaskProducer<T> = isThenable(taskProducer)
? () => taskProducer as PromiseLike<T>
: (taskProducer as TaskProducer<T>);

public add(taskProducer: () => PromiseLike<T>): PromiseLike<T> {
if (!this.isReady()) {
return SyncPromise.reject(new SentryError('Not adding Promise due to buffer limit reached.'));
}
const task = normalizedTaskProducer();
const task = taskProducer();
if (this._buffer.indexOf(task) === -1) {
this._buffer.push(task);
}
Expand Down