Skip to content

FetchTransport.sendEvent ignores buffer and send request independently to buffer. FetchTransport.close race condition. #3725

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

Closed
4 of 9 tasks
levi2ki opened this issue Jun 22, 2021 · 0 comments · Fixed by #3736
Labels
Package: browser Issues related to the Sentry Browser SDK

Comments

@levi2ki
Copy link

levi2ki commented Jun 22, 2021

Package + Version

  • @sentry/browser
  • @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other:

Version:

6.7.2

Description

Trying to limit parallel event request sending by limiting buffer size in custom transport class to 2 and checking buffer is empty, get true accidentally when doing this.close(50).then(isClosed /* can be true but it is a lie */ => super.sendEvent(event)), but call to sendEvent could return rejected SyncPromise with SentyError 'Not adding Promise due to buffer limit reached.' but event was sent to server even error was thrown.

code to reproduce behaviour:

import * as Sentry from '@sentry/browser';
import { PromiseBuffer } from '@sentry/utils/dist/promisebuffer';
import { SyncPromise } from '@sentry/utils/dist/syncpromise';
import { CaptureConsole } from '@sentry/integrations';

export class SentryBuggedTransport extends Sentry.Transports.FetchTransport {
  static bufferLimit = 2;

  readonly _buffer = new PromiseBuffer<Sentry.Response>(SentryBuggedTransport.bufferLimit);

  sendEvent(event: Sentry.Event): PromiseLike<Sentry.Response> {
    const awaiter2 = (): PromiseLike<Sentry.Response> => {
      return this.close(50).then(
        (isReady) => {
          if (isReady) {
            return super.sendEvent(event).then(
              (r) => SyncPromise.resolve(r),
              (e: Error) => {
                this.devTool(`event failed: ${e.message}`);
                return SyncPromise.reject(e);
              },
            );
          } else {
            this.devTool('THIS IS WHAT WE WANT, NOT EXCEPTION');
            return awaiter2();
          }

        },
        (_bufferRejection) => {
          console.error('Buffer poll failed. Report to support team!');
          return SyncPromise.reject(_bufferRejection);
        },
      );
    };
    return awaiter2();
  }

  // TODO: separate logger;
  private devTool(log: unknown) {
    IS_DEV ?
      // eslint-disable-next-line no-console
      console.debug(
        `[${new Date().toISOString()}]`,
        ' SentryQueueTransport ::> ',
        log,
      ) :
      null;
  }
}

try {
  Sentry.init({
    dsn: `${(SENTRY_DSN || (window.env || {}).SENTRY_DSN) as string}`,
    defaultIntegrations: false,
    maxValueLength: 2000,
    integrations: [
      new Sentry.Integrations.GlobalHandlers({
        onerror: true,
        onunhandledrejection: true,
      }),
      new Sentry.Integrations.UserAgent(),
      new Sentry.Integrations.Breadcrumbs({ console: false }),
      new CaptureConsole({ levels: ['warn', 'error', 'critical', 'fatal'] }),
    ],
    transport: SentryBuggedTransport,
  });

  const appVersion = packageVersions().host.version;
  Sentry.setExtra('appVersion', appVersion);
  Promise.all(Array(90).fill(() => Sentry.captureException('OOOPS!')).map(x => x())); /// <<<<==== here magic happen;
} catch (e) {
  // eslint-disable-next-line no-console
  console.error('An error happened during initialization of Sentry', e);
}

Fact result: Only certain requests blocks by buffer, most requests sent immidiately regardless buffer limit.

complete verbose log in attach sentry.log
sentry.log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: browser Issues related to the Sentry Browser SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants